Hi Alex,

On Mon, Jan 15, 2018 at 9:59 PM, tokers <zchao1995@gmail.com> wrote:
> # HG changeset patch
> # User Alex Zhang <zchao1995@gmail.com>
> # Date 1516079440 -28800
> # Tue Jan 16 13:10:40 2018 +0800
> # Node ID 9ca5af970d2296a02acefb3070237c5f52119708
> # Parent 93abb5a855d6534f0356882f45be49f8c6a95a8b
> yield 499 while reading client body and client prematurely closed
> connection.
>
> The function ngx_http_do_read_client_request_body returns
> NGX_HTTP_BAD_REQUEST (client prematurely closed connection),
> while the 400 status code cannot reflect that client closed connection
> prematurely. It should return code 499(NGX_HTTP_CLIENT_CLOSED_REQUEST)
> and it is helpful to troubleshoot some relevant problems.
>
> Signed-off-by: Alex Zhang <zchao1995@gmail.com>
>
> diff -r 93abb5a855d6 -r 9ca5af970d22 src/http/ngx_http_request_body.c
> --- a/src/http/ngx_http_request_body.c Thu Jan 11 21:43:49 2018 +0300
> +++ b/src/http/ngx_http_request_body.c Tue Jan 16 13:10:40 2018 +0800
> @@ -342,14 +342,17 @@
> break;
> }
>
> - if (n == 0) {
> + if (n == 0 || n == NGX_ERROR) {
> + c->error = 1;
> +
> + if (n == 0) {
> + return NGX_HTTP_BAD_REQUEST;
> + }
> +
> ngx_log_error(NGX_LOG_INFO, c->log, 0,
> "client prematurely closed connection");
> - }
>
> - if (n == 0 || n == NGX_ERROR) {
> - c->error = 1;
> - return NGX_HTTP_BAD_REQUEST;
> + return NGX_HTTP_CLIENT_CLOSED_REQUEST;
> }
>
> rb->buf->last += n;

I agree with this change (in fact, I have similar code in my local
tree), but something like this is probably more readable:

diff -r 93abb5a855d6 src/http/ngx_http_request_body.c
--- a/src/http/ngx_http_request_body.c
+++ b/src/http/ngx_http_request_body.c
@@ -345,9 +345,11 @@ ngx_http_do_read_client_request_body(ngx
if (n == 0) {
ngx_log_error(NGX_LOG_INFO, c->log, 0,
"client prematurely closed connection");
+ c->error = 1;
+ return NGX_HTTP_CLIENT_CLOSED_REQUEST;
}

- if (n == 0 || n == NGX_ERROR) {
+ if (n == NGX_ERROR) {
c->error = 1;
return NGX_HTTP_BAD_REQUEST;
}

Having said that, handing of client errors before request body is
fully received is pretty inconsistent in NGINX, especially between
HTTP/1.1 and HTTP/2, so this is only partial fix.

Best regards,
Piotr Sikora
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel