[PATCH 02 of 14] HTTP/3: fixed handling of request body larger than Content-Length

[Maxim Dounin]

Hello!

On Sat, Apr 20, 2024 at 03:59:50AM +0300, Maxim Dounin wrote:

> # HG changeset patch
> # User Maxim Dounin <mdounin at mdounin.ru>
> # Date 1713574628 -10800
> #      Sat Apr 20 03:57:08 2024 +0300
> # Node ID 0946ec7c4a6a26fda6ed03cb1f1f877984ec5614
> # Parent  3c408152180f04a54c44d482cf1c9c52b63480d9
> HTTP/3: fixed handling of request body larger than Content-Length.
> 
> Previously, 413 (Request entity too large) was returned, and incorrect
> "client intended to send too large body" error message was logged.
> Fix is to return 400 (Bad request) and log the "client intended to send
> body data larger than declared" error message, similarly to what HTTP/2
> code does.
> 
> Additionally, previously "client_max_body_size 0;" was incorrectly handled
> by the HTTP/3 code, resulting in 413 instead of no limit.  This is also
> fixed by the correct checks added.
> 
> diff --git a/src/http/v3/ngx_http_v3_request.c b/src/http/v3/ngx_http_v3_request.c
> --- a/src/http/v3/ngx_http_v3_request.c
> +++ b/src/http/v3/ngx_http_v3_request.c
> @@ -1482,7 +1482,6 @@ ngx_http_v3_do_read_client_request_body(
>  static ngx_int_t
>  ngx_http_v3_request_body_filter(ngx_http_request_t *r, ngx_chain_t *in)
>  {
> -    off_t                      max;
>      size_t                     size;
>      u_char                    *p;
>      ngx_int_t                  rc;
> @@ -1510,14 +1509,6 @@ ngx_http_v3_request_body_filter(ngx_http
>          rb->rest = cscf->large_client_header_buffers.size;
>      }
>  
> -    clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);
> -
> -    max = r->headers_in.content_length_n;
> -
> -    if (max == -1 && clcf->client_max_body_size) {
> -        max = clcf->client_max_body_size;
> -    }
> -
>      out = NULL;
>      ll = &out;
>      last = 0;
> @@ -1575,7 +1566,12 @@ ngx_http_v3_request_body_filter(ngx_http
>  
>                  /* rc == NGX_OK */
>  
> -                if (max != -1 && (uint64_t) (max - rb->received) < st->length) {
> +                clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);
> +
> +                if (clcf->client_max_body_size
> +                    && (uint64_t) (clcf->client_max_body_size - rb->received)
> +                       < st->length)
> +                {
>                      ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
>                                    "client intended to send too large "
>                                    "body: %O+%ui bytes",
> @@ -1584,6 +1580,16 @@ ngx_http_v3_request_body_filter(ngx_http
>                      return NGX_HTTP_REQUEST_ENTITY_TOO_LARGE;
>                  }
>  
> +                if (r->headers_in.content_length_n != -1
> +                    && rb->received > r->headers_in.content_length_n)
> +                {

Err, similarly to the above check, this should be:

                 if (r->headers_in.content_length_n != -1
-                    && rb->received > r->headers_in.content_length_n)
+                    && (uint64_t) (r->headers_in.content_length_n
+                                   - rb->received)
+                       < st->length)
                 {
                     ngx_log_error(NGX_LOG_INFO, r->connection->log, 0,
                                   "client intended to send body data "

Since, in contrast to the HTTP/2 code, this is checked before 
rb->received is updated.

> +                    ngx_log_error(NGX_LOG_INFO, r->connection->log, 0,
> +                                  "client intended to send body data "
> +                                  "larger than declared");
> +
> +                    return NGX_HTTP_BAD_REQUEST;
> +                }
> +
>                  continue;
>              }
>  

-- 
Maxim Dounin
http://mdounin.ru/