Hello! >> For the Cache-Control header, the fix is to postpone pushing >> r->headers_out.cache_control until its value is completed.
Why not do the same thing for a bunch of other places like: ngx_http_auth_basic_set_realm ngx_http_range_not_satisfiable That is, for the latter, first initialize content_range, and then add it to headers. Looks like a safer strategy then rolling back a change in case of failure. Thank you! 2017-04-20 19:33 GMT+03:00 Sergey Kandaurov <pluk...@nginx.com>: > details: http://hg.nginx.org/nginx/rev/0cdee26605f3 > branches: > changeset: 6986:0cdee26605f3 > user: Sergey Kandaurov <pluk...@nginx.com> > date: Thu Apr 20 18:26:37 2017 +0300 > description: > Cleaned up r->headers_out.headers allocation error handling. > > If initialization of a header failed for some reason after ngx_list_push(), > leaving the header as is can result in uninitialized memory access by > the header filter or the log module. The fix is to clear partially > initialized headers in case of errors. > > For the Cache-Control header, the fix is to postpone pushing > r->headers_out.cache_control until its value is completed. > > diffstat: > > src/http/modules/ngx_http_auth_basic_module.c | 2 ++ > src/http/modules/ngx_http_dav_module.c | 1 + > src/http/modules/ngx_http_headers_filter_module.c | 21 > +++++++++++---------- > src/http/modules/ngx_http_range_filter_module.c | 4 ++++ > src/http/modules/ngx_http_static_module.c | 1 + > src/http/modules/perl/nginx.xs | 2 ++ > src/http/ngx_http_core_module.c | 1 + > src/http/ngx_http_upstream.c | 13 +++++++------ > 8 files changed, 29 insertions(+), 16 deletions(-) > > diffs (162 lines): > > diff -r 23ecffd5bcfe -r 0cdee26605f3 src/http/modules/ngx_http_ > auth_basic_module.c > --- a/src/http/modules/ngx_http_auth_basic_module.c Thu Apr 20 > 13:58:16 2017 +0300 > +++ b/src/http/modules/ngx_http_auth_basic_module.c Thu Apr 20 > 18:26:37 2017 +0300 > @@ -361,6 +361,8 @@ ngx_http_auth_basic_set_realm(ngx_http_r > > basic = ngx_pnalloc(r->pool, len); > if (basic == NULL) { > + r->headers_out.www_authenticate->hash = 0; > + r->headers_out.www_authenticate = NULL; > return NGX_HTTP_INTERNAL_SERVER_ERROR; > } > > diff -r 23ecffd5bcfe -r 0cdee26605f3 src/http/modules/ngx_http_dav_ > module.c > --- a/src/http/modules/ngx_http_dav_module.c Thu Apr 20 13:58:16 2017 > +0300 > +++ b/src/http/modules/ngx_http_dav_module.c Thu Apr 20 18:26:37 2017 > +0300 > @@ -1080,6 +1080,7 @@ ngx_http_dav_location(ngx_http_request_t > } else { > location = ngx_pnalloc(r->pool, r->uri.len); > if (location == NULL) { > + ngx_http_clear_location(r); > return NGX_ERROR; > } > > diff -r 23ecffd5bcfe -r 0cdee26605f3 src/http/modules/ngx_http_ > headers_filter_module.c > --- a/src/http/modules/ngx_http_headers_filter_module.c Thu Apr 20 > 13:58:16 2017 +0300 > +++ b/src/http/modules/ngx_http_headers_filter_module.c Thu Apr 20 > 18:26:37 2017 +0300 > @@ -271,11 +271,6 @@ ngx_http_set_expires(ngx_http_request_t > return NGX_ERROR; > } > > - ccp = ngx_array_push(&r->headers_out.cache_control); > - if (ccp == NULL) { > - return NGX_ERROR; > - } > - > cc = ngx_list_push(&r->headers_out.headers); > if (cc == NULL) { > return NGX_ERROR; > @@ -283,6 +278,12 @@ ngx_http_set_expires(ngx_http_request_t > > cc->hash = 1; > ngx_str_set(&cc->key, "Cache-Control"); > + > + ccp = ngx_array_push(&r->headers_out.cache_control); > + if (ccp == NULL) { > + return NGX_ERROR; > + } > + > *ccp = cc; > > } else { > @@ -470,11 +471,6 @@ ngx_http_add_cache_control(ngx_http_requ > } > } > > - ccp = ngx_array_push(&r->headers_out.cache_control); > - if (ccp == NULL) { > - return NGX_ERROR; > - } > - > cc = ngx_list_push(&r->headers_out.headers); > if (cc == NULL) { > return NGX_ERROR; > @@ -484,6 +480,11 @@ ngx_http_add_cache_control(ngx_http_requ > ngx_str_set(&cc->key, "Cache-Control"); > cc->value = *value; > > + ccp = ngx_array_push(&r->headers_out.cache_control); > + if (ccp == NULL) { > + return NGX_ERROR; > + } > + > *ccp = cc; > > return NGX_OK; > diff -r 23ecffd5bcfe -r 0cdee26605f3 src/http/modules/ngx_http_ > range_filter_module.c > --- a/src/http/modules/ngx_http_range_filter_module.c Thu Apr 20 > 13:58:16 2017 +0300 > +++ b/src/http/modules/ngx_http_range_filter_module.c Thu Apr 20 > 18:26:37 2017 +0300 > @@ -425,6 +425,8 @@ ngx_http_range_singlepart_header(ngx_htt > content_range->value.data = ngx_pnalloc(r->pool, > sizeof("bytes -/") - 1 + 3 * > NGX_OFF_T_LEN); > if (content_range->value.data == NULL) { > + content_range->hash = 0; > + r->headers_out.content_range = NULL; > return NGX_ERROR; > } > > @@ -594,6 +596,8 @@ ngx_http_range_not_satisfiable(ngx_http_ > content_range->value.data = ngx_pnalloc(r->pool, > sizeof("bytes */") - 1 + > NGX_OFF_T_LEN); > if (content_range->value.data == NULL) { > + content_range->hash = 0; > + r->headers_out.content_range = NULL; > return NGX_ERROR; > } > > diff -r 23ecffd5bcfe -r 0cdee26605f3 src/http/modules/ngx_http_ > static_module.c > --- a/src/http/modules/ngx_http_static_module.c Thu Apr 20 13:58:16 2017 > +0300 > +++ b/src/http/modules/ngx_http_static_module.c Thu Apr 20 18:26:37 2017 > +0300 > @@ -169,6 +169,7 @@ ngx_http_static_handler(ngx_http_request > > location = ngx_pnalloc(r->pool, len); > if (location == NULL) { > + ngx_http_clear_location(r); > return NGX_HTTP_INTERNAL_SERVER_ERROR; > } > > diff -r 23ecffd5bcfe -r 0cdee26605f3 src/http/modules/perl/nginx.xs > --- a/src/http/modules/perl/nginx.xs Thu Apr 20 13:58:16 2017 +0300 > +++ b/src/http/modules/perl/nginx.xs Thu Apr 20 18:26:37 2017 +0300 > @@ -510,10 +510,12 @@ header_out(r, key, value) > header->hash = 1; > > if (ngx_http_perl_sv2str(aTHX_ r, &header->key, key) != NGX_OK) { > + header->hash = 0; > XSRETURN_EMPTY; > } > > if (ngx_http_perl_sv2str(aTHX_ r, &header->value, value) != NGX_OK) { > + header->hash = 0; > XSRETURN_EMPTY; > } > > diff -r 23ecffd5bcfe -r 0cdee26605f3 src/http/ngx_http_core_module.c > --- a/src/http/ngx_http_core_module.c Thu Apr 20 13:58:16 2017 +0300 > +++ b/src/http/ngx_http_core_module.c Thu Apr 20 18:26:37 2017 +0300 > @@ -1002,6 +1002,7 @@ ngx_http_core_find_config_phase(ngx_http > p = ngx_pnalloc(r->pool, len); > > if (p == NULL) { > + ngx_http_clear_location(r); > ngx_http_finalize_request(r, NGX_HTTP_INTERNAL_SERVER_ > ERROR); > return NGX_OK; > } > diff -r 23ecffd5bcfe -r 0cdee26605f3 src/http/ngx_http_upstream.c > --- a/src/http/ngx_http_upstream.c Thu Apr 20 13:58:16 2017 +0300 > +++ b/src/http/ngx_http_upstream.c Thu Apr 20 18:26:37 2017 +0300 > @@ -4897,17 +4897,18 @@ ngx_http_upstream_copy_multi_header_line > } > } > > + ho = ngx_list_push(&r->headers_out.headers); > + if (ho == NULL) { > + return NGX_ERROR; > + } > + > + *ho = *h; > + > ph = ngx_array_push(pa); > if (ph == NULL) { > return NGX_ERROR; > } > > - ho = ngx_list_push(&r->headers_out.headers); > - if (ho == NULL) { > - return NGX_ERROR; > - } > - > - *ho = *h; > *ph = ho; > > return NGX_OK; > _______________________________________________ > nginx-devel mailing list > nginx-devel@nginx.org > http://mailman.nginx.org/mailman/listinfo/nginx-devel >
_______________________________________________ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
