[PATCH] nginx does not close the connection for 412 responses under extreme conditions

Maxim Dounin mdounin at mdounin.ru
Thu May 17 10:24:54 UTC 2012


On Sat, May 12, 2012 at 09:12:46PM +0400, Maxim Dounin wrote:


> I'll take a closer look at the patch later to make sure it doesn't 
> break various image filter use cases (likely no, but as I already 
> said filter finalization is at best fragile) and to see if it's 
> also possible to avoid response truncation in case of error_page 
> used after filter finalization.

Production use of 1.3.0 revealed that it indeed breaks filter 
finalization in config like this

        location /image/ {
            error_page 415 = /zero;
            image_filter crop $arg_w $arg_h;
            proxy_store /tmp/store/$uri;

        location /zero {
            return 204;

In case of 415 from image_filter request is finalized in rewrite 
module, and

1) due to filter_finalize check r->write_event_handler 
remains set to ngx_http_core_run_phases();

2) due to proxy_store NGX_ERROR is ignored by upstream, and 
reading from upstream continues.

Then on write event reported for a client 
ngx_http_core_run_phases() is triggered again, 
ngx_http_finalize_request() is called (incorrecly) causing another 
ngx_http_finalize_connection() due to filter_finalize set.  This 
eventually results in segfault once request is finalized from 
upstream module.

Michael Monashov, who reported the issue, currently testing the 
following patch (against 1.3.0):

--- a/src/http/ngx_http_request.c
+++ b/src/http/ngx_http_request.c
@@ -1933,8 +1933,6 @@ ngx_http_finalize_request(ngx_http_reque
     if (rc == NGX_OK && r->filter_finalize) {
         c->error = 1;
-        ngx_http_finalize_connection(r);
-        return;
     if (rc == NGX_DECLINED) {

It should resolve the original problem with socket leak but let 
things handle correctly the above scenario as well.  Could you 
please test if it works for you?

Maxim Dounin

More information about the nginx-devel mailing list