Hello! On Tue, Jun 13, 2017 at 05:19:48AM -0700, Piotr Sikora via nginx-devel wrote:
> # HG changeset patch > # User Piotr Sikora <piotrsik...@google.com> > # Date 1490516709 25200 > # Sun Mar 26 01:25:09 2017 -0700 > # Node ID e2abc3bc3fc12b788d2631d3c47215acdc4ebbe6 > # Parent 6263d68cb96042d8f8974a4a3945226227ce13b9 > HTTP/2: reject HTTP/2 requests with connection-specific headers. > > Signed-off-by: Piotr Sikora <piotrsik...@google.com> > > diff -r 6263d68cb960 -r e2abc3bc3fc1 src/http/ngx_http_request.c > --- a/src/http/ngx_http_request.c > +++ b/src/http/ngx_http_request.c > @@ -19,6 +19,8 @@ static ngx_int_t ngx_http_alloc_large_he > > static ngx_int_t ngx_http_process_header_line(ngx_http_request_t *r, > ngx_table_elt_t *h, ngx_uint_t offset); > +static ngx_int_t ngx_http_process_http1_header_line(ngx_http_request_t *r, > + ngx_table_elt_t *h, ngx_uint_t offset); > static ngx_int_t ngx_http_process_unique_header_line(ngx_http_request_t *r, > ngx_table_elt_t *h, ngx_uint_t offset); > static ngx_int_t ngx_http_process_multi_header_lines(ngx_http_request_t *r, > @@ -146,7 +148,7 @@ ngx_http_header_t ngx_http_headers_in[] > > { ngx_string("Upgrade"), > offsetof(ngx_http_headers_in_t, upgrade), > - ngx_http_process_header_line }, > + ngx_http_process_http1_header_line }, > > #if (NGX_HTTP_GZIP) > { ngx_string("Accept-Encoding"), > @@ -161,8 +163,13 @@ ngx_http_header_t ngx_http_headers_in[] > offsetof(ngx_http_headers_in_t, authorization), > ngx_http_process_unique_header_line }, > > - { ngx_string("Keep-Alive"), offsetof(ngx_http_headers_in_t, keep_alive), > - ngx_http_process_header_line }, > + { ngx_string("Keep-Alive"), > + offsetof(ngx_http_headers_in_t, keep_alive), > + ngx_http_process_http1_header_line }, > + > + { ngx_string("Proxy-Connection"), > + offsetof(ngx_http_headers_in_t, proxy_connection), > + ngx_http_process_http1_header_line }, I'm highly sceptical about the whole series in general, and this patch specifically. In particular, the "Proxy-Connection" header is not something even defined by any standard, and even in its non-standard [broken] meaning never expected to be used in connections to nginx. Not to mention that Proxy-Authorization, a standard-defined hop-by-hop (connection-specific in terms of HTTP/2) header, is not checked anywhere. Additionally, I really think that disabling upgrades is one of the big mistakes of HTTP/2. It would be much more logical to interpret a HTTP/2 stream as a connection to upgrade, and allow to multiplex arbitrary protocols via a single HTTP/2 connection. Unless there are practical reasons for these changes, I would rather reject the series. -- Maxim Dounin http://nginx.org/ _______________________________________________ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel