Use a return code for ngx_http_close_request()

jason_sam · June 27, 2014, 3:26pm

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:

crespin · June 27, 2014, 9:42pm

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/

crespin · June 30, 2014, 9:00pm

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/

crespin · June 28, 2014, 12:54pm

Maxim D. Wrote:

must be

Maxim D.
http://nginx.org/

nginx mailing list
[email protected]
nginx Info Page

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: