New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dropped requests when more than 20 client headers #6

Closed
crdumoul opened this Issue Dec 29, 2011 · 4 comments

Comments

Projects
None yet
2 participants
@crdumoul

crdumoul commented Dec 29, 2011

When more than 20 client headers are included in a response, the ngx_list_t for the headers will contain multiple parts. The function ngx_http_headers_more_rm_header in ngx_http_headers_more_util.c doesn't properly traverse the different parts of the list, which results in an error. The request is then dropped.

This situation can be reproduced by sending a request through nginx that contains more than 20 client headers, where the Host header is the last header.

The call stack to get to the relevant code looks like this:

(gdb) bt
#0  ngx_http_headers_more_rm_header (l=0x8f87124, h=0x8ff2748) at /opt/agentzh-headers-more-nginx-module-2cbbc15/src/ngx_http_headers_more_util.c:257
#1  0x080b2b99 in ngx_http_set_builtin_header (r=0x8f870f0, hv=0x8ff8358, value=0xbfd9a9f8) at /opt/agentzh-headers-more-nginx-module-2cbbc15/src/ngx_http_headers_more_headers_in.c:297
#2  0x080b2c96 in ngx_http_set_host_header (r=0x8f870f0, hv=0x8ff8358, value=0xbfd9a9f8) at /opt/agentzh-headers-more-nginx-module-2cbbc15/src/ngx_http_headers_more_headers_in.c:322
#3  0x080b2df7 in ngx_http_headers_more_exec_input_cmd (r=0x8f870f0, cmd=0x9115a50) at /opt/agentzh-headers-more-nginx-module-2cbbc15/src/ngx_http_headers_more_headers_in.c:151
#4  0x080b1de1 in ngx_http_headers_more_handler (r=0x8f870f0) at /opt/agentzh-headers-more-nginx-module-2cbbc15/src/ngx_http_headers_more_filter_module.c:285
#5  0x08074fe1 in ngx_http_core_rewrite_phase (r=0x8f870f0, ph=0x9117864) at src/http/ngx_http_core_module.c:915
#6  0x08071833 in ngx_http_core_run_phases (r=0x8f870f0) at src/http/ngx_http_core_module.c:861
#7  0x08076d13 in ngx_http_named_location (r=0x8f870f0, name=0xbfd9ab8c) at src/http/ngx_http_core_module.c:2555
#8  0x08077119 in ngx_http_core_try_files_phase (r=0x8f870f0, ph=0x91178b8) at src/http/ngx_http_core_module.c:1278
#9  0x08071833 in ngx_http_core_run_phases (r=0x8f870f0) at src/http/ngx_http_core_module.c:861
#10 0x08071932 in ngx_http_handler (r=0x8f870f0) at src/http/ngx_http_core_module.c:844
#11 0x0807be2f in ngx_http_process_request (r=0x8f870f0) at src/http/ngx_http_request.c:1667
#12 0x0807c4b9 in ngx_http_process_request_headers (rev=0xb718f0a4) at src/http/ngx_http_request.c:1113
#13 0x0807ca55 in ngx_http_process_request_line (rev=0xb718f0a4) at src/http/ngx_http_request.c:913
#14 0x08078e8b in ngx_http_init_request (rev=0xb718f0a4) at src/http/ngx_http_request.c:518
#15 0x0806aa71 in ngx_epoll_process_events (cycle=0x8f7e828, timer=60000, flags=1) at src/event/modules/ngx_epoll_module.c:678
#16 0x08061c06 in ngx_process_events_and_timers (cycle=0x8f7e828) at src/event/ngx_event.c:245
#17 0x08069074 in ngx_worker_process_cycle (cycle=0x8f7e828, data=0x0) at src/os/unix/ngx_process_cycle.c:801
#18 0x08067731 in ngx_spawn_process (cycle=0x8f7e828, proc=0x8068f98 , data=0x0, name=0x80b77af "worker process", respawn=-3) at src/os/unix/ngx_process.c:196
#19 0x0806856f in ngx_start_worker_processes (cycle=0x8f7e828, n=1, type=-3) at src/os/unix/ngx_process_cycle.c:360
#20 0x08069990 in ngx_master_process_cycle (cycle=0x8f7e828) at src/os/unix/ngx_process_cycle.c:136
#21 0x0804cd8b in main (argc=1, argv=0xbfd9b154) at src/core/nginx.c:411

The problem is when the "part" variable is re-assigned, the "data" variable is not also re-assigned.

A patch for the fix looks like this:

--- ngx_http_headers_more_util.c.orig   2011-12-29 11:42:59.089757781 -0500
+++ ngx_http_headers_more_util.c    2011-12-29 11:43:47.885757202 -0500
@@ -250,6 +250,7 @@
             }
 
             part = part->next;
+            data = part->elts;
             h = part->elts;
             i = 0;
         }

I was using version 0.14 of the code (https://github.com/agentzh/headers-more-nginx-module/tarball/v0.14), but it looks like the same problem is still in the latest version.

@agentzh

This comment has been minimized.

Member

agentzh commented Dec 30, 2011

thanks for the report! i'll look into this :)

agentzh added a commit that referenced this issue Dec 30, 2011

bugfix: removing builtin headers in huge request headers with 20+ ent…
…ries could result in data loss. thanks Chris Dumoulin for the patch in github issue #6.
@agentzh

This comment has been minimized.

Member

agentzh commented Dec 30, 2011

I've applied your patch in git master. Please confirm that it works for you :)

Thanks a lot!

@crdumoul

This comment has been minimized.

crdumoul commented Jan 2, 2012

I've confirmed that the problem is fixed in the master branch

@agentzh

This comment has been minimized.

Member

agentzh commented Jan 2, 2012

Thanks! I'm closing this issue :)

@agentzh agentzh closed this Jan 2, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment