Update handling of multiple headers changed in nginx 1.23.0 #136
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
On my local machiine, with changes in this pull request and Update handling of multiple headers changed in nginx 1.23.0 by hnakamur · Pull Request #2063 · openresty/lua-nginx-module, the |
|
It turned out the above failure is not caused by this pull request. I found So, when And when settings the value of those values to the variable And that yields a string like |
|
I found Re: What is the order of module execution and how can I change it? and I found the order of headers-more-nginx-module and lua-nginx-module in my configure script was not good. |
zhuizhuhaomeng
approved these changes
Jun 28, 2022
|
@zhuizhuhaomeng Thanks for your approval. Should I revert |
|
merged with the following adjustment |
|
test input.t TEST 42 seg faults! |
|
I cannot reproduce seg faults in t/input.t TEST 42. Module versions I used:
|
|
I also has PASS, but on TEST 42 core dumped prove t/input.t
t/input.t .. ok
All tests successful.
Files=1, Tests=236, 9 wallclock secs ( 0.09 usr 0.00 sys + 0.84 cusr 0.41 csys = 1.34 CPU)
Result: PASSmy config is nginx -V
nginx version: nginx/1.23.0
built by gcc 11.2.1 20220219 (Alpine 11.2.1_git20220219)
built with OpenSSL 1.1.1o 3 May 2022
TLS SNI support enabled
configure arguments: --add-dynamic-module='modules/ngx_devel_kit modules/echo-nginx-module modules/encrypted-session-nginx-module modules/form-input-nginx-module modules/iconv-nginx-module modules/nginx_csrf_prevent modules/nginx-push-stream-module modules/nginx-upload-module modules/nginx-upstream-fair modules/nginx-uuid4-module modules/ngx_brotli/filter modules/ngx_brotli/static modules/ngx_http_auth_basic_ldap_module modules/ngx_http_captcha_module modules/ngx_http_headers_module modules/ngx_http_htmldoc_module modules/ngx_http_json_module modules/ngx_http_mustach_module modules/ngx_http_sign_module modules/ngx_http_substitutions_filter_module modules/ngx_http_zip_var_module modules/set-misc-nginx-module modules/ngx_http_auth_pam_module modules/ngx_http_evaluate_module modules/ngx_http_response_body_module modules/headers-more-nginx-module modules/ngx_http_remote_passwd modules/ngx_http_json_var_module modules/ngx_http_time_var_module modules/ngx_http_error_page_inherit_module modules/ngx_http_include_server_module modules/nginx-ejwt-module modules/array-var-nginx-module modules/ngx_pq_module modules/ngx_upstream_jdomain' --conf-path=/etc/nginx/nginx.conf --error-log-path=/var/log/nginx/error.log --group=nginx --http-client-body-temp-path=/var/tmp/nginx/client_body_temp --http-fastcgi-temp-path=/var/tmp/nginx/fastcgi_temp --http-log-path=/var/log/nginx/access.log --http-proxy-temp-path=/var/tmp/nginx/proxy_temp --http-scgi-temp-path=/var/tmp/nginx/scgi_temp --http-uwsgi-temp-path=/var/tmp/nginx/uwsgi_temp --lock-path=/run/nginx/nginx.lock --modules-path=/usr/local/lib/nginx --pid-path=/run/nginx/nginx.pid --prefix=/etc/nginx --sbin-path=/usr/local/bin/nginx --user=nginx --with-cc-opt='-Wextra -Wwrite-strings -Wmissing-prototypes -Werror -Wno-discarded-qualifiers' --with-compat --with-debug --with-file-aio --with-http_addition_module --with-http_auth_request_module --with-http_dav_module --with-http_degradation_module --with-http_flv_module --with-http_geoip_module=dynamic --with-http_gunzip_module --with-http_gzip_static_module --with-http_image_filter_module=dynamic --with-http_mp4_module --with-http_random_index_module --with-http_realip_module --with-http_secure_link_module --with-http_slice_module --with-http_ssl_module --with-http_stub_status_module --with-http_sub_module --with-http_v2_module --with-http_xslt_module=dynamic --with-pcre --with-pcre-jit --with-poll_module --with-select_module --with-stream=dynamic --with-stream_geoip_module=dynamic --with-stream_realip_module --with-stream_ssl_module --with-stream_ssl_preread_module --with-threads |
|
and bad test is # vi:filetype=
use lib 'lib';
use Test::Nginx::Socket; # 'no_plan';
repeat_each(2);
plan tests => repeat_each() * 3;
no_long_string();
#no_diff;
run_tests();
__DATA__
=== TEST 42: clear all and re-insert
--- main_config
load_module /etc/nginx/modules/ngx_http_echo_module.so;
load_module /etc/nginx/modules/ngx_http_headers_more_filter_module.so;
--- config
location = /t {
more_clear_input_headers Host Connection Cache-Control Accept
User-Agent Accept-Encoding Accept-Language
Cookie;
more_set_input_headers "Host: a" "Connection: b" "Cache-Control: c"
"Accept: d" "User-Agent: e" "Accept-Encoding: f"
"Accept-Language: g" "Cookie: h";
more_clear_input_headers Host Connection Cache-Control Accept
User-Agent Accept-Encoding Accept-Language
Cookie;
more_set_input_headers "Host: a" "Connection: b" "Cache-Control: c"
"Accept: d" "User-Agent: e" "Accept-Encoding: f"
"Accept-Language: g" "Cookie: h";
echo ok;
}
--- raw_request eval
"GET /t HTTP/1.1\r
Host: localhost\r
Connection: close\r
Cache-Control: max-age=0\r
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8\r
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/30.0.1599.101 Safari/537.36\r
Accept-Encoding: gzip,deflate,sdch\r
Accept-Language: en-US,en;q=0.8\r
Cookie: test=cookie;\r
\r
"
--- response_body
ok
--- no_error_log
[error]
|
|
I tried again with change to dynamic modules and without debug, but I got still no core dumped. I confirmed a core dumped if I inject an intentional bug like below: |
|
I have seg fault NOT IN this module: Program terminated with signal SIGSEGV, Segmentation fault.
#0 ngx_http_variable_headers_internal (r=0x7efc786202b0, v=0x7efc78620ff0, data=<optimized out>, sep=sep@entry=44 ',') at src/http/ngx_http_variables.c:834
834 if (th->hash == 0) {
(gdb) bt
#0 ngx_http_variable_headers_internal (r=0x7efc786202b0, v=0x7efc78620ff0, data=<optimized out>, sep=sep@entry=44 ',') at src/http/ngx_http_variables.c:834
#1 0x0000555cfa2f95fa in ngx_http_variable_header (r=<optimized out>, v=<optimized out>, data=<optimized out>) at src/http/ngx_http_variables.c:808
#2 0x0000555cfa2faf12 in ngx_http_get_indexed_variable (r=0x7efc786202b0, index=<optimized out>) at src/http/ngx_http_variables.c:631
#3 0x0000555cfa2f5746 in ngx_http_log_variable_getlen (r=<optimized out>, data=<optimized out>) at src/http/modules/ngx_http_log_module.c:956
#4 0x0000555cfa2f618f in ngx_http_log_handler (r=0x7efc786202b0) at src/http/modules/ngx_http_log_module.c:305
#5 0x0000555cfa2ea00e in ngx_http_log_request (r=r@entry=0x7efc786202b0) at src/http/ngx_http_request.c:3727
#6 0x0000555cfa2ebfbd in ngx_http_free_request (r=r@entry=0x7efc786202b0, rc=rc@entry=0) at src/http/ngx_http_request.c:3673
#7 0x0000555cfa2ec148 in ngx_http_close_request (r=r@entry=0x7efc786202b0, rc=rc@entry=0) at src/http/ngx_http_request.c:3619
#8 0x0000555cfa2ed257 in ngx_http_finalize_connection (r=r@entry=0x7efc786202b0) at src/http/ngx_http_request.c:2762
#9 0x0000555cfa2ee120 in ngx_http_finalize_request (r=r@entry=0x7efc786202b0, rc=0) at src/http/ngx_http_request.c:2635
#10 0x0000555cfa2e7cc5 in ngx_http_core_content_phase (r=0x7efc786202b0, ph=<optimized out>) at src/http/ngx_http_core_module.c:1261
#11 0x0000555cfa2e1afd in ngx_http_core_run_phases (r=r@entry=0x7efc786202b0) at src/http/ngx_http_core_module.c:875
#12 0x0000555cfa2e1ba0 in ngx_http_handler (r=r@entry=0x7efc786202b0) at src/http/ngx_http_core_module.c:858
#13 0x0000555cfa2eef13 in ngx_http_process_request (r=r@entry=0x7efc786202b0) at src/http/ngx_http_request.c:2094
#14 0x0000555cfa2ef6d5 in ngx_http_process_request_headers (rev=rev@entry=0x7efc78640370) at src/http/ngx_http_request.c:1496
#15 0x0000555cfa2efb71 in ngx_http_process_request_line (rev=rev@entry=0x7efc78640370) at src/http/ngx_http_request.c:1163
#16 0x0000555cfa2efd8b in ngx_http_wait_request_handler (rev=0x7efc78640370) at src/http/ngx_http_request.c:501
#17 0x0000555cfa2cded1 in ngx_epoll_process_events (cycle=0x7efc78648250, timer=<optimized out>, flags=<optimized out>) at src/event/modules/ngx_epoll_module.c:901
#18 0x0000555cfa2c002e in ngx_process_events_and_timers (cycle=cycle@entry=0x7efc78648250) at src/event/ngx_event.c:248
#19 0x0000555cfa2ccc18 in ngx_single_process_cycle (cycle=cycle@entry=0x7efc78648250) at src/os/unix/ngx_process_cycle.c:300
#20 0x0000555cfa29c309 in main (argc=<optimized out>, argv=<optimized out>) at src/core/nginx.c:380 |
|
but WITH this test file #136 (comment) |
|
I don't know how, but this fix solve segfault diff --git a/src/ngx_http_headers_more_headers_in.c b/src/ngx_http_headers_more_headers_in.c
index 7c28b23..e5e9182 100644
--- a/src/ngx_http_headers_more_headers_in.c
+++ b/src/ngx_http_headers_more_headers_in.c
@@ -335,6 +335,7 @@ matched:
h->key = hv->key;
h->value = *value;
+ h->next = NULL;
h->lowcase_key = ngx_pnalloc(r->pool, h->key.len);
if (h->lowcase_key == NULL) {
@@ -392,6 +393,7 @@ ngx_http_set_builtin_header(ngx_http_request_t *r,
h->hash = hv->hash;
h->value = *value;
+ h->next = NULL;
return NGX_OK;
} |
|
Hi @RekGRpth, I can't reproduce your segfault either, just to confirm a few more conditions with you:
|
musl
it prints letter
ldd $(which nginx)
/lib/ld-musl-x86_64.so.1 (0x7fc97e7c7000)
libpcre2-8.so.0 => /usr/lib/libpcre2-8.so.0 (0x7fc97e5af000)
libssl.so.1.1 => /lib/libssl.so.1.1 (0x7fc97e52e000)
libcrypto.so.1.1 => /lib/libcrypto.so.1.1 (0x7fc97e2ac000)
libz.so.1 => /lib/libz.so.1 (0x7fc97e292000)
libc.musl-x86_64.so.1 => /lib/ld-musl-x86_64.so.1 (0x7fc97e7c7000) |
|
Hi all, Compiling from master against Nginx 1.23.0 works fine for me. If there are no pending changes related to that, could you please create a tag? Thanks in advance, |
How about including tests? |
|
@RekGRpth |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes headers-more-nginx-module not compatible with nginx 1.23.0 · Issue #132 · openresty/headers-more-nginx-module.