Possible incorrect handling of invalid headers with HTTP/2.0 and POST/PUT requests

[Brian Stanback]

When an invalid header is encountered, it appears that all subsequent headers are also marked as invalid (whether they are or not). I specifically noticed this happen when doing POST requests (GET requests appeared to be unaffected). Additionally, the issue does not show up with SPDY 3.1 or HTTP/1.x.

I was testing an http2 rollout with nginx v1.9.6 and noticed that when users had a particular extension enabled in Chrome (Alexa Traffic Rank), it would append a custom header containing illegal characters. Rather than ignore the individual header, nginx was stripping out valid headers including the POST body.

When setting error_log to debug, Nginx was logging the request as follows:

nginx.1    | 2015/11/11 21:45:40 [info] 21#21: *175 client sent invalid header: "alexatoolbar-alx_ns_ph" while processing HTTP/2 connection, client: 10.9.1.21, server: _, host: "_"
nginx.1    | *175 client sent invalid header: "cookie" while processing HTTP/2 connection, client: 10.9.1.21, server: _, host: "_"
nginx.1    | 2015/11/11 21:45:40 [info] 21#21: *175 client sent invalid header: "x-requested-with" while processing HTTP/2 connection, client: 10.9.1.21, server: _, host: "_"
nginx.1    | 2015/11/11 21:45:40 [warn] 21#21: *175 a client request body is buffered to a temporary file /var/cache/nginx/client_temp/0000000001, client: 10.9.1.21, server: _, request: "POST /api HTTP/2.0", host: "_"

An interesting side-effect - for some reason, It seems that when an invalid header is encountered, Nginx reports "a client request body is buffered to a temporary file" regardless of how large I set the buffer in client_body_buffer_size.)

I was able to avoid this behavior by adding the following configuration directive:

ignore_invalid_headers off;

[Brian Stanback]

For what it's worth, the issue was actually happening to me on trac.nginx.org. I wasn't able to login with a combination of Chrome and the Alexa Traffic Rank extension, disabling the extension let me in without any issue.

[Valentin V. Bartenev]

Thank you for the report. Please try the following patch:

diff -r 909b5b191f25 src/http/v2/ngx_http_v2.c
--- a/src/http/v2/ngx_http_v2.c Thu Nov 05 15:01:09 2015 +0300
+++ b/src/http/v2/ngx_http_v2.c Thu Nov 12 18:01:06 2015 +0300
@@ -1630,6 +1630,8 @@ ngx_http_v2_state_process_header(ngx_htt
     }
 
     if (r->invalid_header) {
+        r->invalid_header = 0;
+
         cscf = ngx_http_get_module_srv_conf(r, ngx_http_core_module);
 
         if (cscf->ignore_invalid_headers) {

[Valentin V. Bartenev]

The side-effect is cased by ignoring Content-Length in that case.

[Brian Stanback]

I've applied your patch to my copy of the 1.9.6 source and did some testing - I can confirm that it resolves the issue.

Thanks for addressing this so quickly! The Content-Length explanation makes sense.

[Valentin Bartenev <vbart@…>]

In 6291:932a465537ef/nginx:

HTTP/2: fixed invalid headers handling (ticket #831).

The r->invalid_header flag wasn't reset once an invalid header appeared in a
request, resulting in all subsequent headers in the request were also marked
as invalid.

[Valentin V. Bartenev]

A different version of the patch has been committed after review.
It will be released with nginx 1.9.7.

Thanks.