Use a return code for ngx_http_close_request()

Hello,

here is another path still on ngx_http_request.c.
In function ngx_http_close_request(), the second parameter is an error
code.

This error code is used in ngx_http_free_request() to set the HTTP
status
code if it’s not present or if no bytes are already sent.

Use NGX_OK instead of zero seems - for me - valid.

When ngx_http_close_request() is called after an error, I guess it’s
must be
NGX_HTTP_INTERNAL_SERVER_ERROR.

Perhaps, it’s better to do two patch one for zero to NGX_OK and another
for
NGX_HTTP_INTERNAL_SERVER_ERROR.

Thanks for your comments,

Best regards,

yves

Subject: [PATCH] use an error code for ngx_http_close_request()


1.0/source/nginx-1.7.2/src/http/ngx_http_request.c | 38
+++++++++±---------
1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/1.0/source/nginx-1.7.2/src/http/ngx_http_request.c
b/1.0/source/nginx-1.7.2/src/http/ngx_http_request.c
index 8ddfc60…1979229 100644
— a/1.0/source/nginx-1.7.2/src/http/ngx_http_request.c
+++ b/1.0/source/nginx-1.7.2/src/http/ngx_http_request.c
@@ -2429,7 +2429,7 @@ ngx_http_finalize_request(ngx_http_request_t *r,
ngx_int_t rc)
}

 if (c->read->eof) {
  •    ngx_http_close_request(r, 0);
    
  •    ngx_http_close_request(r, NGX_OK);
       return;
    
    }

@@ -2493,7 +2493,7 @@ ngx_http_terminate_handler(ngx_http_request_t *r)

 r->count = 1;
  • ngx_http_close_request(r, 0);
  • ngx_http_close_request(r, NGX_OK);
    }

@@ -2504,7 +2504,7 @@ ngx_http_finalize_connection(ngx_http_request_t
*r)

#if (NGX_HTTP_SPDY)
if (r->spdy_stream) {

  •    ngx_http_close_request(r, 0);
    
  •    ngx_http_close_request(r, NGX_OK);
       return;
    
    }
    #endif
    @@ -2523,7 +2523,7 @@ ngx_http_finalize_connection(ngx_http_request_t
    *r)
    }
    }
  •    ngx_http_close_request(r, 0);
    
  •    ngx_http_close_request(r, NGX_OK);
       return;
    
    }

@@ -2546,7 +2546,7 @@ ngx_http_finalize_connection(ngx_http_request_t
*r)
return;
}

  • ngx_http_close_request(r, 0);
  • ngx_http_close_request(r, NGX_OK);
    }

@@ -2581,7 +2581,7 @@ ngx_http_set_write_handler(ngx_http_request_t *r)
}

 if (ngx_handle_write_event(wev, clcf->send_lowat) != NGX_OK) {
  •    ngx_http_close_request(r, 0);
    
  •    ngx_http_close_request(r, NGX_HTTP_INTERNAL_SERVER_ERROR);
       return NGX_ERROR;
    
    }

@@ -2622,7 +2622,7 @@ ngx_http_writer(ngx_http_request_t *r)
ngx_add_timer(wev, clcf->send_timeout);

         if (ngx_handle_write_event(wev, clcf->send_lowat) != 

NGX_OK) {

  •            ngx_http_close_request(r, 0);
    
  •            ngx_http_close_request(r, 
    

NGX_HTTP_INTERNAL_SERVER_ERROR);
}

         return;

@@ -2635,7 +2635,7 @@ ngx_http_writer(ngx_http_request_t *r)
“http writer delayed”);

     if (ngx_handle_write_event(wev, clcf->send_lowat) != NGX_OK) {
  •        ngx_http_close_request(r, 0);
    
  •        ngx_http_close_request(r, NGX_HTTP_INTERNAL_SERVER_ERROR);
       }
    
       return;
    

@@ -2659,7 +2659,7 @@ ngx_http_writer(ngx_http_request_t *r)
}

     if (ngx_handle_write_event(wev, clcf->send_lowat) != NGX_OK) {
  •        ngx_http_close_request(r, 0);
    
  •        ngx_http_close_request(r, NGX_HTTP_INTERNAL_SERVER_ERROR);
       }
    
       return;
    

@@ -2696,7 +2696,7 @@ ngx_http_block_reading(ngx_http_request_t *r)
&& r->connection->read->active)
{
if (ngx_del_event(r->connection->read, NGX_READ_EVENT, 0) !=
NGX_OK) {

  •        ngx_http_close_request(r, 0);
    
  •        ngx_http_close_request(r, NGX_HTTP_INTERNAL_SERVER_ERROR);
       }
    

    }
    }
    @@ -2798,7 +2798,7 @@ ngx_http_test_reading(ngx_http_request_t *r)
    if ((ngx_event_flags & NGX_USE_LEVEL_EVENT) && rev->active) {

       if (ngx_del_event(rev, NGX_READ_EVENT, 0) != NGX_OK) {
    
  •        ngx_http_close_request(r, 0);
    
  •        ngx_http_close_request(r, NGX_HTTP_INTERNAL_SERVER_ERROR);
       }
    
    }

@@ -2869,7 +2869,7 @@ ngx_http_set_keepalive(ngx_http_request_t *r)
cscf->large_client_header_buffers.num *
sizeof(ngx_buf_t
*));

             if (hc->free == NULL) {
  •                ngx_http_close_request(r, 0);
    
  •                ngx_http_close_request(r,
    

NGX_HTTP_INTERNAL_SERVER_ERROR);
return;
}
}
@@ -3198,7 +3198,7 @@ ngx_http_set_lingering_close(ngx_http_request_t
*r)
ngx_add_timer(rev, clcf->lingering_timeout);

 if (ngx_handle_read_event(rev, 0) != NGX_OK) {
  •    ngx_http_close_request(r, 0);
    
  •    ngx_http_close_request(r,
    

NGX_HTTP_INTERNAL_SERVER_ERRORNGX_ERROR);
return;
}

@@ -3207,7 +3207,7 @@ ngx_http_set_lingering_close(ngx_http_request_t
*r)

 if (wev->active && (ngx_event_flags & NGX_USE_LEVEL_EVENT)) {
     if (ngx_del_event(wev, NGX_WRITE_EVENT, 0) != NGX_OK) {
  •        ngx_http_close_request(r, 0);
    
  •        ngx_http_close_request(r, NGX_HTTP_INTERNAL_SERVER_ERROR);
           return;
       }
    
    }
    @@ -3215,7 +3215,7 @@ ngx_http_set_lingering_close(ngx_http_request_t
    *r)
    if (ngx_shutdown_socket(c->fd, NGX_WRITE_SHUTDOWN) == -1) {
    ngx_connection_error(c, ngx_socket_errno,
    ngx_shutdown_socket_n " failed");
  •    ngx_http_close_request(r, 0);
    
  •    ngx_http_close_request(r, NGX_HTTP_INTERNAL_SERVER_ERROR);
       return;
    
    }

@@ -3242,13 +3242,13 @@ ngx_http_lingering_close_handler(ngx_event_t
*rev)
“http lingering close handler”);

 if (rev->timedout) {
  •    ngx_http_close_request(r, 0);
    
  •    ngx_http_close_request(r, NGX_OK);
       return;
    

    }

    timer = (ngx_msec_t) r->lingering_time - (ngx_msec_t) ngx_time();
    if ((ngx_msec_int_t) timer <= 0) {

  •    ngx_http_close_request(r, 0);
    
  •    ngx_http_close_request(r, NGX_OK);
       return;
    
    }

@@ -3258,14 +3258,14 @@ ngx_http_lingering_close_handler(ngx_event_t
*rev)
ngx_log_debug1(NGX_LOG_DEBUG_HTTP, c->log, 0, “lingering read:
%d”,
n);

     if (n == NGX_ERROR || n == 0) {
  •        ngx_http_close_request(r, 0);
    
  •        ngx_http_close_request(r, (n == 0) ? NGX_OK : NGX_ERROR);
           return;
       }
    

    } while (rev->ready);

    if (ngx_handle_read_event(rev, 0) != NGX_OK) {

  •    ngx_http_close_request(r, 0);
    
  •    ngx_http_close_request(r, NGX_HTTP_INTERNAL_SERVER_ERROR);
       return;
    
    }


1.7.10.4

Posted at Nginx Forum:
http://forum.nginx.org/read.php?2,251228,251228#msg-251228

Hello!

On Fri, Jun 27, 2014 at 09:26:23AM -0400, crespin wrote:

Hello,

here is another path still on ngx_http_request.c.
In function ngx_http_close_request(), the second parameter is an error
code.

This error code is used in ngx_http_free_request() to set the HTTP status
code if it’s not present or if no bytes are already sent.

Use NGX_OK instead of zero seems - for me - valid.

The same logic applies as in the previous answer about
ngx_http_terminate_request().

When ngx_http_close_request() is called after an error, I guess it’s must be
NGX_HTTP_INTERNAL_SERVER_ERROR.

Perhaps, it’s better to do two patch one for zero to NGX_OK and another for
NGX_HTTP_INTERNAL_SERVER_ERROR.

Use of NGX_HTTP_INTERNAL_SERVER_ERROR is wrong, as there are no
chances that this response code will be ever actually sent.
Response headers are either already sent, or won’t be sent at all.


Maxim D.
http://nginx.org/

Hello!

On Sat, Jun 28, 2014 at 06:54:14AM -0400, crespin wrote:

ngx_hhtp_close_request() call ngx_http_free_request()
Reading the code with your comments, rc is only set when a timeout is
raised.
Is it right ?
So the parameter can be a flag ngx_flag_t is_timeout ?

I don’t understand your question, sorry.


Maxim D.
http://nginx.org/

Maxim D. Wrote:

must be

Maxim D.
http://nginx.org/


nginx mailing list
[email protected]
http://mailman.nginx.org/mailman/listinfo/nginx

Hello Maxim,

Thanks for your response.

ngx_hhtp_close_request() call ngx_http_free_request()
Reading the code with your comments, rc is only set when a timeout is
raised.
Is it right ?
So the parameter can be a flag ngx_flag_t is_timeout ?

yves

Posted at Nginx Forum:
http://forum.nginx.org/read.php?2,251228,251281#msg-251281

This forum is not affiliated to the Ruby language, Ruby on Rails framework, nor any Ruby applications discussed here.

| Privacy Policy | Terms of Service | Remote Ruby Jobs