Hi Maxim, > On 6 Mar 2024, at 12:28 AM, Maxim Dounin <mdou...@mdounin.ru> wrote: > > Hello! > > On Mon, Mar 04, 2024 at 06:46:23PM +0400, Roman Arutyunyan wrote: > >> # HG changeset patch >> # User Roman Arutyunyan <a...@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
Thanks for your kind comment, I read that before. The patch fixes exactly what it fixes. Accessing the request body after it was discarded (for example from a 413 custom handler) is a sign of misconfiguration. Indeed for a misconfigured nginx there is a problem with a HTTP/1 body currently being discarded as well as other inconsistencies. This may or may not be fixed in the future, but anyway it's a separate issue, which does not exist in a properly configured nginx. > 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-de...@freenginx.org list for proper review. We understand your hard feelings about leaving the project. Hope you'll be ok. > -- > Maxim Dounin > http://mdounin.ru/ > _______________________________________________ > nginx-devel mailing list > nginx-devel@nginx.org > https://mailman.nginx.org/mailman/listinfo/nginx-devel ---- Roman Arutyunyan a...@nginx.com
_______________________________________________ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel