Opened 7 years ago

Last modified 9 months ago

#1152 reopened defect

Custom error_page doesn't work for HTTP error 413

Reported by: splewako@… Owned by:
Priority: minor Milestone:
Component: other Version: 1.11.x
Keywords: Cc:
uname -a:
nginx -V: nginx version: nginx/1.11.6
built by gcc 4.8.5 20150623 (Red Hat 4.8.5-4) (GCC)
built with OpenSSL 1.0.1e-fips 11 Feb 2013
TLS SNI support enabled
configure arguments: --prefix=/etc/nginx --sbin-path=/usr/sbin/nginx --modules-path=/usr/lib64/nginx/modules --conf-path=/etc/nginx/nginx.conf --error-log-path=/var/log/nginx/error.log --http-log-path=/var/log/nginx/access.log --pid-path=/var/run/nginx.pid --lock-path=/var/run/nginx.lock --http-client-body-temp-path=/var/cache/nginx/client_temp --http-proxy-temp-path=/var/cache/nginx/proxy_temp --http-fastcgi-temp-path=/var/cache/nginx/fastcgi_temp --http-uwsgi-temp-path=/var/cache/nginx/uwsgi_temp --http-scgi-temp-path=/var/cache/nginx/scgi_temp --user=nginx --group=nginx --with-compat --with-file-aio --with-threads --with-http_addition_module --with-http_auth_request_module --with-http_dav_module --with-http_flv_module --with-http_gunzip_module --with-http_gzip_static_module --with-http_mp4_module --with-http_random_index_module --with-http_realip_module --with-http_secure_link_module --with-http_slice_module --with-http_ssl_module --with-http_stub_status_module --with-http_sub_module --with-http_v2_module --with-mail --with-mail_ssl_module --with-stream --with-stream_realip_module --with-stream_ssl_module --with-stream_ssl_preread_module --with-cc-opt='-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m64 -mtune=generic'

Description

Native Nginx error page instead of custom is shown for 413 errors (404 works fine).


http {

uwsgi_intercept_errors on;
error_page 404 /error.404.html;
error_page 413 /error.413.html;


server {

listen 443 ssl http2;
listen [::]:443 ssl http2;
root /www;


location /error {

internal;

}


location / {

uwsgi_pass unix:/run/uwsgi.sock

}

}

}

Change History (6)

comment:1 by Maxim Dounin, 7 years ago

Resolution: worksforme
Status: newclosed

Works fine here.

Please make sure that:

  • there are no other error_page directives on lower levels, quoting docs:

    These directives are inherited from the previous level if and only if there are no error_page directives defined on the current level.

  • the page in question actually exists (try to request it directly with internal removed).

Full test configuration:

http {
    access_log /dev/stderr combined;

    server {
        listen      8080;

        proxy_intercept_errors on;
        error_page 413 /error413;

        client_max_body_size 1g;

        location / {
            proxy_pass http://127.0.0.1:8081;
        }

        location = /error413 {
            internal;
            return 200 "location /error413\n";
        }
    }

    server {
        listen      8081;
        client_max_body_size 1;
    }
}

Test, which shows that custom error page is returned:

> telnet localhost 8080
Trying ::1...
telnet: connect to address ::1: Connection refused
Trying 127.0.0.1...
Connected to localhost.
Escape character is '^]'.
GET / HTTP/1.0
Content-Length: 10

1234567890
HTTP/1.1 413 Request Entity Too Large
Server: nginx/1.11.7
Date: Mon, 12 Dec 2016 14:11:05 GMT
Content-Type: text/plain
Content-Length: 19
Connection: close

location /error413
Connection closed by foreign host.

If you still see the problem, consider reopening this ticket and provide a full configuration to reproduce the problem you are seeing.

comment:2 by albgus@…, 12 months ago

For what it's worth I've encountered this issue as well and it seems to come down to some unintuitive processing of the upload file size limit. It appears the limit is processed for the error page location as well. I've found that adding client_max_body_size 0; to the error location makes it work.

Very basic example below

Given

location /api/ {
    error_page 413 /_error/413.json;
}

The following works

location /_error/ {
    internal;

    client_max_body_size 0;
    alias /etc/nginx/error_pages/;
}

While this causes the nginx default 413 HTML to be returned.

location /_error/ {
    internal;

    alias /etc/nginx/error_pages/;
}

comment:3 by Maxim Dounin, 12 months ago

Resolution: worksforme
Status: closedreopened

For what it's worth I've encountered this issue as well and it seems to come down to some unintuitive processing of the upload file size limit. It appears the limit is processed for the error page location as well.

Thanks for reviving this. Looking again through the code, I was able to reproduce this. It looks like a HTTP/2-specific bug.

When detects that client_max_body_size is reached, it discards the request body. With HTTP/1.x, this will set the r->discard_body body flag when discarding the request body, and will set r->headers_in.content_length_n to 0 when the body is completely discarded. As such, the relevant check in ngx_http_core_find_config_phase() is skipped and not triggered again:

    if (r->headers_in.content_length_n != -1
        && !r->discard_body
        && clcf->client_max_body_size
        && clcf->client_max_body_size < r->headers_in.content_length_n)
    {
        ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
                      "client intended to send too large body: %O bytes",
                      r->headers_in.content_length_n);

        r->expect_tested = 1;
        (void) ngx_http_discard_request_body(r);
        ngx_http_finalize_request(r, NGX_HTTP_REQUEST_ENTITY_TOO_LARGE);
        return NGX_OK;
    }

With HTTP/2, however, neither the r->discard_body flag nor the r->headers_in.content_length_n field is set, and this triggers the check again, so the error page does not work without removing the client_max_body_size limit in the location with the error page.

For example, in the following configuration:

    server {
        listen 8443 ssl http2;

        ssl_certificate mdounin.ru.crt;
        ssl_certificate_key mdounin.ru.key;

        error_page 413 /error413;

        client_max_body_size 1;

        location / {
        }

        location = /error413 {
            internal;
            return 200 "location /error413\n";
        }
    }

Requests over HTTP/1.x properly return the error page configured:

$ curl -k --data foo --http1.1 https://127.0.0.1:8443/foo
location /error413

But requests over HTTP/2 return the default error page, as the 413 is triggered again when trying to reach the error page:

$ curl -k --data foo --http2 https://127.0.0.1:8443/foo
<html>
<head><title>413 Request Entity Too Large</title></head>
<body>
<center><h1>413 Request Entity Too Large</h1></center>
<hr><center>nginx/1.25.0</center>
</body>
</html>

Please try the following patch, it should make things work with both HTTP/1.x and HTTP/2:

# HG changeset patch
# User Maxim Dounin <mdounin@mdounin.ru>
# Date 1682044645 -10800
#      Fri Apr 21 05:37:25 2023 +0300
# Node ID 364c5c513fd3298a311103f91075162dd71cbdf8
# Parent  77d5c662f3d9d9b90425128109d3369c30ef5f07
HTTP/2: fixed "error_page 413" handling (ticket #1152).

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 either 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 request body discarding
code failed to set r->discard_body or reset r->headers_in.content_length_n,
so configuring "error_page 413" did not work 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 request
body discarding code.  This is essentially what happens with HTTP/1.x when
the body is completely discarded, and makes it possible to use
"error_page 413" without additional settings.

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
@@ -621,6 +621,7 @@ 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

in reply to:  3 comment:4 by Sergey Kandaurov, 9 months ago

Replying to Maxim Dounin:

Please try the following patch, it should make things work with both HTTP/1.x and HTTP/2:

For the record, same problem exists in HTTP/3, so would require similar adjustment:

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

comment:5 by Maxim Dounin, 9 months ago

See also #2408.

comment:6 by Maxim Dounin, 9 months ago

Just for the record, from my internal notes to ensure it's not forgotten.

The patch needs to account for "requests with body" vs. "requests without body distinction". Simply assigning r->headers_in.content_length_n = 0; might result in incorrect further processing of requests without body after the ngx_http_discard_request_body() call: for example, during proxying there will be an empty body.

Also, as mentioned in #2408, further processing of requests with discarded body might be incorrect: there will be no body, yet existing headers will indicate that there is a body. And there are multiple distinct cases which might work differently: such as HTTP proxying which uses r->headers_in.content_length_n (which won't be 0 in case of HTTP/1.x if body isn't fully discarded yet), and fastcgi proxying which uses $content_length (which will simply reflect the Content-Length header if it's present). This probably needs further cleanup.

Note: See TracTickets for help on using tickets.