yield 499 while reading client body and client prematurely closed connection

[Piotr Sikora]

Hi Alex,

On Mon, Jan 15, 2018 at 9:59 PM, tokers <zchao1995 at gmail.com> wrote:
> # HG changeset patch
> # User Alex Zhang <zchao1995 at 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 at 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