[PATCH] Fixed 413 custom error page for HTTP/2 and HTTP/3 (ticket #2609)

[Maxim Dounin]

Hello!

On Mon, Mar 04, 2024 at 06:46:23PM +0400, Roman Arutyunyan wrote:

> # HG changeset patch
> # User Roman Arutyunyan <arut at nginx.com>
> # Date 1709563405 -14400
> #      Mon Mar 04 18:43:25 2024 +0400
> # Node ID 3b0be477ab7246caba4c5152286b8be520ee0418
> # Parent  44da04c2d4db94ad4eefa84b299e07c5fa4a00b9
> Fixed 413 custom error page for HTTP/2 and HTTP/3 (ticket #2609).
> 
> Previously an attempt to return a custom 413 error page for these protocols
> resulted in the standard 413 page (if recursive_error_pages was off) or
> otherwise internal redirection cycle followed by the 500 error.
> 
> Discarding request body for HTTP/1 starts by setting r->discard_body which
> indicates the body is currently being discarded.  If and when the entire body
> is read and discarded, the flag is cleared and r->headers_in.content_length_n
> is set to zero.  Both r->discard_body and r->headers_in.content_length_n
> prevent nginx from re-generating 413 error after internal redirect in
> ngx_http_core_find_config_phase().
> 
> However the above does not work for HTTP/2 and HTTP/3.  Discarding request
> body for these protocols does not affect the above mentioned fields, which is
> why there's no protection against re-generating the 413 error.  The fix is to
> assign zero to r->headers_in.content_length_n much like in HTTP/1 case after
> the body is entirely read and discarded, except for these protocols no active
> discard is needed.
> 
> diff --git a/src/http/ngx_http_request_body.c b/src/http/ngx_http_request_body.c
> --- a/src/http/ngx_http_request_body.c
> +++ b/src/http/ngx_http_request_body.c
> @@ -640,12 +640,14 @@ ngx_http_discard_request_body(ngx_http_r
>  #if (NGX_HTTP_V2)
>      if (r->stream) {
>          r->stream->skip_data = 1;
> +        r->headers_in.content_length_n = 0;
>          return NGX_OK;
>      }
>  #endif
>  
>  #if (NGX_HTTP_V3)
>      if (r->http_version == NGX_HTTP_VERSION_30) {
> +        r->headers_in.content_length_n = 0;
>          return NGX_OK;
>      }
>  #endif

The patch is wrong, see here:

https://trac.nginx.org/nginx/ticket/1152#comment:6 

The issue is in my TODO list.  Once properly fixed, you'll be able 
to merge the fix from freenginx.

Alternatively, consider submitting patches to 
the nginx-devel at freenginx.org list for proper review.

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