<div dir="ltr">Hello!<div><br></div><div><span style="font-size:12.8px">>> For the Cache-Control header, the fix is to postpone pushing</span><br style="font-size:12.8px;outline:none"><span style="font-size:12.8px">>> r->headers_out.cache_control until its value is completed.</span><br></div><br>Why not do the same thing for a bunch of other places like:<br>ngx_http_auth_basic_set_realm<div>ngx_http_range_not_satisfiable</div><br>That is, for the latter, first initialize content_range, and then add it to headers.<br>Looks like a safer strategy then rolling back a change in case of failure.<div><br><div>Thank you!<br><div><span style="font-size:12.8px"><br></span></div></div></div></div><div class="gmail_extra"><br><div class="gmail_quote">2017-04-20 19:33 GMT+03:00 Sergey Kandaurov <span dir="ltr"><<a href="mailto:pluknet@nginx.com" target="_blank">pluknet@nginx.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">details: <a href="http://hg.nginx.org/nginx/rev/0cdee26605f3" rel="noreferrer" target="_blank">http://hg.nginx.org/nginx/rev/<wbr>0cdee26605f3</a><br>
branches:<br>
changeset: 6986:0cdee26605f3<br>
user: Sergey Kandaurov <<a href="mailto:pluknet@nginx.com">pluknet@nginx.com</a>><br>
date: Thu Apr 20 18:26:37 2017 +0300<br>
description:<br>
Cleaned up r->headers_out.headers allocation error handling.<br>
<br>
If initialization of a header failed for some reason after ngx_list_push(),<br>
leaving the header as is can result in uninitialized memory access by<br>
the header filter or the log module. The fix is to clear partially<br>
initialized headers in case of errors.<br>
<br>
For the Cache-Control header, the fix is to postpone pushing<br>
r->headers_out.cache_control until its value is completed.<br>
<br>
diffstat:<br>
<br>
src/http/modules/ngx_http_<wbr>auth_basic_module.c | 2 ++<br>
src/http/modules/ngx_http_dav_<wbr>module.c | 1 +<br>
src/http/modules/ngx_http_<wbr>headers_filter_module.c | 21 +++++++++++----------<br>
src/http/modules/ngx_http_<wbr>range_filter_module.c | 4 ++++<br>
src/http/modules/ngx_http_<wbr>static_module.c | 1 +<br>
src/http/modules/perl/nginx.xs | 2 ++<br>
src/http/ngx_http_core_module.<wbr>c | 1 +<br>
src/http/ngx_http_upstream.c | 13 +++++++------<br>
8 files changed, 29 insertions(+), 16 deletions(-)<br>
<br>
diffs (162 lines):<br>
<br>
diff -r 23ecffd5bcfe -r 0cdee26605f3 src/http/modules/ngx_http_<wbr>auth_basic_module.c<br>
--- a/src/http/modules/ngx_http_<wbr>auth_basic_module.c Thu Apr 20 13:58:16 2017 +0300<br>
+++ b/src/http/modules/ngx_http_<wbr>auth_basic_module.c Thu Apr 20 18:26:37 2017 +0300<br>
@@ -361,6 +361,8 @@ ngx_http_auth_basic_set_realm(<wbr>ngx_http_r<br>
<br>
basic = ngx_pnalloc(r->pool, len);<br>
if (basic == NULL) {<br>
+ r->headers_out.www_<wbr>authenticate->hash = 0;<br>
+ r->headers_out.www_<wbr>authenticate = NULL;<br>
return NGX_HTTP_INTERNAL_SERVER_<wbr>ERROR;<br>
}<br>
<br>
diff -r 23ecffd5bcfe -r 0cdee26605f3 src/http/modules/ngx_http_dav_<wbr>module.c<br>
--- a/src/http/modules/ngx_http_<wbr>dav_module.c Thu Apr 20 13:58:16 2017 +0300<br>
+++ b/src/http/modules/ngx_http_<wbr>dav_module.c Thu Apr 20 18:26:37 2017 +0300<br>
@@ -1080,6 +1080,7 @@ ngx_http_dav_location(ngx_<wbr>http_request_t<br>
} else {<br>
location = ngx_pnalloc(r->pool, r->uri.len);<br>
if (location == NULL) {<br>
+ ngx_http_clear_location(r);<br>
return NGX_ERROR;<br>
}<br>
<br>
diff -r 23ecffd5bcfe -r 0cdee26605f3 src/http/modules/ngx_http_<wbr>headers_filter_module.c<br>
--- a/src/http/modules/ngx_http_<wbr>headers_filter_module.c Thu Apr 20 13:58:16 2017 +0300<br>
+++ b/src/http/modules/ngx_http_<wbr>headers_filter_module.c Thu Apr 20 18:26:37 2017 +0300<br>
@@ -271,11 +271,6 @@ ngx_http_set_expires(ngx_http_<wbr>request_t<br>
return NGX_ERROR;<br>
}<br>
<br>
- ccp = ngx_array_push(&r->headers_<wbr>out.cache_control);<br>
- if (ccp == NULL) {<br>
- return NGX_ERROR;<br>
- }<br>
-<br>
cc = ngx_list_push(&r->headers_out.<wbr>headers);<br>
if (cc == NULL) {<br>
return NGX_ERROR;<br>
@@ -283,6 +278,12 @@ ngx_http_set_expires(ngx_http_<wbr>request_t<br>
<br>
cc->hash = 1;<br>
ngx_str_set(&cc->key, "Cache-Control");<br>
+<br>
+ ccp = ngx_array_push(&r->headers_<wbr>out.cache_control);<br>
+ if (ccp == NULL) {<br>
+ return NGX_ERROR;<br>
+ }<br>
+<br>
*ccp = cc;<br>
<br>
} else {<br>
@@ -470,11 +471,6 @@ ngx_http_add_cache_control(<wbr>ngx_http_requ<br>
}<br>
}<br>
<br>
- ccp = ngx_array_push(&r->headers_<wbr>out.cache_control);<br>
- if (ccp == NULL) {<br>
- return NGX_ERROR;<br>
- }<br>
-<br>
cc = ngx_list_push(&r->headers_out.<wbr>headers);<br>
if (cc == NULL) {<br>
return NGX_ERROR;<br>
@@ -484,6 +480,11 @@ ngx_http_add_cache_control(<wbr>ngx_http_requ<br>
ngx_str_set(&cc->key, "Cache-Control");<br>
cc->value = *value;<br>
<br>
+ ccp = ngx_array_push(&r->headers_<wbr>out.cache_control);<br>
+ if (ccp == NULL) {<br>
+ return NGX_ERROR;<br>
+ }<br>
+<br>
*ccp = cc;<br>
<br>
return NGX_OK;<br>
diff -r 23ecffd5bcfe -r 0cdee26605f3 src/http/modules/ngx_http_<wbr>range_filter_module.c<br>
--- a/src/http/modules/ngx_http_<wbr>range_filter_module.c Thu Apr 20 13:58:16 2017 +0300<br>
+++ b/src/http/modules/ngx_http_<wbr>range_filter_module.c Thu Apr 20 18:26:37 2017 +0300<br>
@@ -425,6 +425,8 @@ ngx_http_range_singlepart_<wbr>header(ngx_htt<br>
content_range->value.data = ngx_pnalloc(r->pool,<br>
sizeof("bytes -/") - 1 + 3 * NGX_OFF_T_LEN);<br>
if (content_range->value.data == NULL) {<br>
+ content_range->hash = 0;<br>
+ r->headers_out.content_range = NULL;<br>
return NGX_ERROR;<br>
}<br>
<br>
@@ -594,6 +596,8 @@ ngx_http_range_not_<wbr>satisfiable(ngx_http_<br>
content_range->value.data = ngx_pnalloc(r->pool,<br>
sizeof("bytes */") - 1 + NGX_OFF_T_LEN);<br>
if (content_range->value.data == NULL) {<br>
+ content_range->hash = 0;<br>
+ r->headers_out.content_range = NULL;<br>
return NGX_ERROR;<br>
}<br>
<br>
diff -r 23ecffd5bcfe -r 0cdee26605f3 src/http/modules/ngx_http_<wbr>static_module.c<br>
--- a/src/http/modules/ngx_http_<wbr>static_module.c Thu Apr 20 13:58:16 2017 +0300<br>
+++ b/src/http/modules/ngx_http_<wbr>static_module.c Thu Apr 20 18:26:37 2017 +0300<br>
@@ -169,6 +169,7 @@ ngx_http_static_handler(ngx_<wbr>http_request<br>
<br>
location = ngx_pnalloc(r->pool, len);<br>
if (location == NULL) {<br>
+ ngx_http_clear_location(r);<br>
return NGX_HTTP_INTERNAL_SERVER_<wbr>ERROR;<br>
}<br>
<br>
diff -r 23ecffd5bcfe -r 0cdee26605f3 src/http/modules/perl/nginx.xs<br>
--- a/src/http/modules/perl/nginx.<wbr>xs Thu Apr 20 13:58:16 2017 +0300<br>
+++ b/src/http/modules/perl/nginx.<wbr>xs Thu Apr 20 18:26:37 2017 +0300<br>
@@ -510,10 +510,12 @@ header_out(r, key, value)<br>
header->hash = 1;<br>
<br>
if (ngx_http_perl_sv2str(aTHX_ r, &header->key, key) != NGX_OK) {<br>
+ header->hash = 0;<br>
XSRETURN_EMPTY;<br>
}<br>
<br>
if (ngx_http_perl_sv2str(aTHX_ r, &header->value, value) != NGX_OK) {<br>
+ header->hash = 0;<br>
XSRETURN_EMPTY;<br>
}<br>
<br>
diff -r 23ecffd5bcfe -r 0cdee26605f3 src/http/ngx_http_core_module.<wbr>c<br>
--- a/src/http/ngx_http_core_<wbr>module.c Thu Apr 20 13:58:16 2017 +0300<br>
+++ b/src/http/ngx_http_core_<wbr>module.c Thu Apr 20 18:26:37 2017 +0300<br>
@@ -1002,6 +1002,7 @@ ngx_http_core_find_config_<wbr>phase(ngx_http<br>
p = ngx_pnalloc(r->pool, len);<br>
<br>
if (p == NULL) {<br>
+ ngx_http_clear_location(r);<br>
ngx_http_finalize_request(r, NGX_HTTP_INTERNAL_SERVER_<wbr>ERROR);<br>
return NGX_OK;<br>
}<br>
diff -r 23ecffd5bcfe -r 0cdee26605f3 src/http/ngx_http_upstream.c<br>
--- a/src/http/ngx_http_upstream.c Thu Apr 20 13:58:16 2017 +0300<br>
+++ b/src/http/ngx_http_upstream.c Thu Apr 20 18:26:37 2017 +0300<br>
@@ -4897,17 +4897,18 @@ ngx_http_upstream_copy_multi_<wbr>header_line<br>
}<br>
}<br>
<br>
+ ho = ngx_list_push(&r->headers_out.<wbr>headers);<br>
+ if (ho == NULL) {<br>
+ return NGX_ERROR;<br>
+ }<br>
+<br>
+ *ho = *h;<br>
+<br>
ph = ngx_array_push(pa);<br>
if (ph == NULL) {<br>
return NGX_ERROR;<br>
}<br>
<br>
- ho = ngx_list_push(&r->headers_out.<wbr>headers);<br>
- if (ho == NULL) {<br>
- return NGX_ERROR;<br>
- }<br>
-<br>
- *ho = *h;<br>
*ph = ho;<br>
<br>
return NGX_OK;<br>
______________________________<wbr>_________________<br>
nginx-devel mailing list<br>
<a href="mailto:nginx-devel@nginx.org">nginx-devel@nginx.org</a><br>
<a href="http://mailman.nginx.org/mailman/listinfo/nginx-devel" rel="noreferrer" target="_blank">http://mailman.nginx.org/<wbr>mailman/listinfo/nginx-devel</a><br>
</blockquote></div><br></div>