[nginx] Request body: error_page 413 handling with HTTP/2 and HT...

[Maxim Dounin]

details:   http://freenginx.org/hg/nginx/rev/f798ecafec05
branches:  
changeset: 9261:f798ecafec05
user:      Maxim Dounin <mdounin at mdounin.ru>
date:      Sat Apr 27 18:22:38 2024 +0300
description:
Request body: error_page 413 handling with HTTP/2 and HTTP/3.

When the client_max_body_size limit in ngx_http_core_find_config_phase()
is hit, nginx calls the ngx_http_discard_request_body() function, which
normally sets the r->discard_body flag while discarding the body, and then
reduces the r->headers_in.content_length_n field to 0 when the body is
completely discarded.  As such, the client_max_body_size check is skipped
if the request is redirected to an error page, and this makes it possible
to use "error_page 413" without additional settings.

This only works with HTTP/1.x though.  The HTTP/2 and HTTP/3 request body
discarding code paths failed to set r->discard_body or reset
r->headers_in.content_length_n, so configuring "error_page 413" did notwork
without additionally clearing the client_max_body_size limit in the
location with error page.

Fix is to set r->headers_in.content_length_n to 0 in the HTTP/2 and HTTP/3
request body discarding code paths (if there is a body).  This is essentially
what happens with HTTP/1.x when the body is completely discarded, and makes
it possible to use "error_page 413" with HTTP/2 and HTTP/3 without additional
settings.

Additionally, r->discard_body flag is also set.  For HTTP/2, it is not needed,
but serves as an optimization.  For HTTP/3, it ensures that the request body
cannot be read after it was discarded, thus bypassing the client_max_body_size
limit.

Further, the r->discard_body flag is now always set after the request body
is discarded (and not cleared once it is fully discarded).  While the body
is being discarded, the new r->discarding_body flag is now used.  This
slightly optimizes existing code paths in ngx_http_read_client_request_body()
and ngx_http_discard_request_body(), and also makes it easier to only set
ngx_http_discarded_request_body_handler() for HTTP/1.x.

diffstat:

 src/http/ngx_http_core_module.c  |   1 +
 src/http/ngx_http_request.c      |   4 ++--
 src/http/ngx_http_request.h      |   1 +
 src/http/ngx_http_request_body.c |  20 +++++++++++++++++---
 4 files changed, 21 insertions(+), 5 deletions(-)

diffs (107 lines):

diff --git a/src/http/ngx_http_core_module.c b/src/http/ngx_http_core_module.c
--- a/src/http/ngx_http_core_module.c
+++ b/src/http/ngx_http_core_module.c
@@ -2456,6 +2456,7 @@ ngx_http_subrequest(ngx_http_request_t *
     sr->internal = 1;
 
     sr->discard_body = r->discard_body;
+    sr->discarding_body = r->discarding_body;
     sr->expect_tested = 1;
     sr->main_filter_need_in_memory = r->main_filter_need_in_memory;
 
diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c
--- a/src/http/ngx_http_request.c
+++ b/src/http/ngx_http_request.c
@@ -2762,7 +2762,7 @@ ngx_http_finalize_connection(ngx_http_re
 
     if (r->main->count != 1) {
 
-        if (r->discard_body) {
+        if (r->discarding_body) {
             r->read_event_handler = ngx_http_discarded_request_body_handler;
             ngx_add_timer(r->connection->read, clcf->lingering_timeout);
 
@@ -2820,7 +2820,7 @@ ngx_http_set_write_handler(ngx_http_requ
 
     r->http_state = NGX_HTTP_WRITING_REQUEST_STATE;
 
-    r->read_event_handler = r->discard_body ?
+    r->read_event_handler = r->discarding_body ?
                                 ngx_http_discarded_request_body_handler:
                                 ngx_http_test_reading;
     r->write_event_handler = ngx_http_writer;
diff --git a/src/http/ngx_http_request.h b/src/http/ngx_http_request.h
--- a/src/http/ngx_http_request.h
+++ b/src/http/ngx_http_request.h
@@ -538,6 +538,7 @@ struct ngx_http_request_s {
     unsigned                          keepalive:1;
     unsigned                          lingering_close:1;
     unsigned                          discard_body:1;
+    unsigned                          discarding_body:1;
     unsigned                          reading_body:1;
     unsigned                          internal:1;
     unsigned                          error_page:1;
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
@@ -682,12 +682,24 @@ ngx_http_discard_request_body(ngx_http_r
 #if (NGX_HTTP_V2)
     if (r->stream) {
         r->stream->skip_data = 1;
+
+        if (r->headers_in.content_length_n > 0 || r->headers_in.chunked) {
+            r->headers_in.content_length_n = 0;
+            r->discard_body = 1;
+        }
+
         return NGX_OK;
     }
 #endif
 
 #if (NGX_HTTP_V3)
     if (r->http_version == NGX_HTTP_VERSION_30) {
+
+        if (r->headers_in.content_length_n > 0 || r->headers_in.chunked) {
+            r->headers_in.content_length_n = 0;
+            r->discard_body = 1;
+        }
+
         return NGX_OK;
     }
 #endif
@@ -706,6 +718,8 @@ ngx_http_discard_request_body(ngx_http_r
         return NGX_OK;
     }
 
+    r->discard_body = 1;
+
     size = r->header_in->last - r->header_in->pos;
 
     if (size || r->headers_in.chunked) {
@@ -740,7 +754,7 @@ ngx_http_discard_request_body(ngx_http_r
     }
 
     r->count++;
-    r->discard_body = 1;
+    r->discarding_body = 1;
 
     return NGX_OK;
 }
@@ -769,7 +783,7 @@ ngx_http_discarded_request_body_handler(
         timer = (ngx_msec_t) r->lingering_time - (ngx_msec_t) ngx_time();
 
         if ((ngx_msec_int_t) timer <= 0) {
-            r->discard_body = 0;
+            r->discarding_body = 0;
             r->lingering_close = 0;
             ngx_http_finalize_request(r, NGX_ERROR);
             return;
@@ -782,7 +796,7 @@ ngx_http_discarded_request_body_handler(
     rc = ngx_http_read_discarded_request_body(r);
 
     if (rc == NGX_OK) {
-        r->discard_body = 0;
+        r->discarding_body = 0;
         r->lingering_close = 0;
         r->lingering_time = 0;
         ngx_http_finalize_request(r, NGX_DONE);