Hello! On Sat, Jun 03, 2017 at 08:03:57PM -0700, Piotr Sikora via nginx-devel wrote:
> # HG changeset patch > # User Piotr Sikora <[email protected]> > # Date 1489618489 25200 > # Wed Mar 15 15:54:49 2017 -0700 > # Node ID e472b23fdc387943ea90fb2f0ae415d9d104edc7 > # Parent 716852cce9136d977b81a2d1b8b6f9fbca0dce49 > Proxy: always emit "Host" header first. > > Signed-off-by: Piotr Sikora <[email protected]> > > diff -r 716852cce913 -r e472b23fdc38 src/http/modules/ngx_http_proxy_module.c > --- a/src/http/modules/ngx_http_proxy_module.c > +++ b/src/http/modules/ngx_http_proxy_module.c > @@ -3412,7 +3412,7 @@ ngx_http_proxy_init_headers(ngx_conf_t * > uintptr_t *code; > ngx_uint_t i; > ngx_array_t headers_names, headers_merged; > - ngx_keyval_t *src, *s, *h; > + ngx_keyval_t *host, *src, *s, *h; > ngx_hash_key_t *hk; > ngx_hash_init_t hash; > ngx_http_script_compile_t sc; > @@ -3444,11 +3444,33 @@ ngx_http_proxy_init_headers(ngx_conf_t * > return NGX_ERROR; > } > > + h = default_headers; > + > + if (h->key.len != sizeof("Host") - 1 > + || ngx_strcasecmp(h->key.data, (u_char *) "Host") != 0) > + { > + return NGX_ERROR; > + } > + > + host = ngx_array_push(&headers_merged); > + if (host == NULL) { > + return NGX_ERROR; > + } > + > + *host = *h++; > + > if (conf->headers_source) { > > src = conf->headers_source->elts; > for (i = 0; i < conf->headers_source->nelts; i++) { > > + if (src[i].key.len == sizeof("Host") - 1 > + && ngx_strcasecmp(src[i].key.data, (u_char *) "Host") == 0) > + { > + *host = src[i]; > + continue; > + } > + > s = ngx_array_push(&headers_merged); > if (s == NULL) { > return NGX_ERROR; > @@ -3458,8 +3480,6 @@ ngx_http_proxy_init_headers(ngx_conf_t * > } > } > > - h = default_headers; > - > while (h->key.len) { > > src = headers_merged.elts; I don't think I like this. Trying to prioritize headers based on some mostly external knowledge in a function which is intended to merge headers user-supplied and default headers doesn't look wise. Not to mention that it depends on the "Host" header being first in the default headers lists. Instead, consider the following variants: - emitting default headers first, than user-supplied ones; - emitting the Host header separately from other headers, via a dedicated code in ngx_http_proxy_create_request(). Note well that I'm not really sure that any of the above variants is in fact better than the current behaviour. The current behaviour is to follow header order specified in the configuration, and it makes it possible to redefine order of standard headers if needed. I suspect this and other patches you've send at the same time are in fact parts of your work to introduce HTTP/2 support to upstream servers. Please consider submitting a patch series with the whole feature instead. Individual patches which doesn't make much sense by its own are hard to review, and are likely to be rejected. -- Maxim Dounin http://nginx.org/ _______________________________________________ nginx-devel mailing list [email protected] http://mailman.nginx.org/mailman/listinfo/nginx-devel
