max_body_size check failed in nginx when a handler has read request body in access phase

[mcdullbloom]

Description

the action of set max_body_size is invalid when a handler in access phase has read request body, which is running before access_by_lua.And we can't just put access_by_lua handler before that handler, since access_by_lua is the major phase of apisix.

client-control.lua

        -- then check it when reading the body
        local ok, err = apisix_ngx_client.set_client_max_body_size(conf.max_body_size)
        if not ok then
            core.log.error("failed to set client max body size: ", err)
            return 503
        end

ngx_int_t
ngx_http_read_client_request_body(ngx_http_request_t *r,
    ngx_http_client_body_handler_pt post_handler){
……
    if (r != r->main || r->request_body || r->discard_body) {  // request body will be read only once totallly
        r->request_body_no_buffering = 0;
        post_handler(r);
        return NGX_OK;
    }

#if (NGX_HTTP_APISIX)
    if (ngx_http_apisix_delay_client_max_body_check(r)) {
        off_t max_body_size = ngx_http_apisix_client_max_body_size(r); // max_body_size has not been set by plugin when the handler of third-party module is called

        if (r->headers_in.content_length_n != -1
            && max_body_size
            && 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;
            rc = NGX_HTTP_REQUEST_ENTITY_TOO_LARGE;
            goto done;
        }
    }
……

Environment

• APISIX version (run apisix version):
• Operating system (run uname -a):
• OpenResty / Nginx version (run openresty -V or nginx -V): 1.19.3
• etcd version, if relevant (run curl http://127.0.0.1:9090/v1/server_info):
• APISIX Dashboard version, if relevant:
• Plugin runner version, for issues related to plugin runners:
• LuaRocks version, for installation issues (run luarocks --version):

[Revolyssup]

@mcdullbloom Can you share some error logs or the case in which this resulted in error? Also can you update the description to say in exactly which case this will cause issues as the tests are passing for this plugin.

[mcdullbloom]

There're not any error logs.Just set max_body_size failed.

plugin:

"plugins": { "client-control":{ "max_body_size": 100 } }

(gdb) p max_body_size $1 = 0

Detail debug info：

(gdb)
(gdb) b ngx_http_read_client_request_body
Breakpoint 1 at 0x47d5b0: file src/http/ngx_http_request_body.c, line 37.
(gdb) c
Continuing.

Breakpoint 1, ngx_http_read_client_request_body (r=r@entry=0x150bd60, post_handler=post_handler@entry=0x7f1eaefb2520 <ngx_http_t1k_req_request_body_handler>) at src/http/ngx_http_request_body.c:37
37      {
(gdb) l
32
33
34      ngx_int_t
35      ngx_http_read_client_request_body(ngx_http_request_t *r,
36          ngx_http_client_body_handler_pt post_handler)
37      {
38          size_t                     preread;
39          ssize_t                    size;
40          ngx_int_t                  rc;
41          ngx_buf_t                 *b;
(gdb) n
46          r->main->count++;
(gdb)
48          if (r != r->main || r->request_body || r->discard_body) {
(gdb)
55          if (ngx_http_apisix_delay_client_max_body_check(r)) {
(gdb)
56              off_t max_body_size = ngx_http_apisix_client_max_body_size(r);
(gdb)
58              if (r->headers_in.content_length_n != -1
(gdb)
60                  && max_body_size < r->headers_in.content_length_n)
(gdb) p max_body_size 
$1 = 0
(gdb) n
59                  && max_body_size
(gdb)
60                  && max_body_size < r->headers_in.content_length_n)
(gdb)
73          if (ngx_http_test_expect(r) != NGX_OK) {
(gdb)
78          rb = ngx_pcalloc(r->pool, sizeof(ngx_http_request_body_t));
(gdb)
79          if (rb == NULL) {
(gdb)
78          rb = ngx_pcalloc(r->pool, sizeof(ngx_http_request_body_t));
(gdb)
79          if (rb == NULL) {
(gdb)
99          if (r->headers_in.content_length_n < 0 && !r->headers_in.chunked) {
(gdb)
94          rb->rest = -1;
(gdb)
95          rb->post_handler = post_handler;
(gdb)
97          r->request_body = rb;
(gdb)
99          if (r->headers_in.content_length_n < 0 && !r->headers_in.chunked) {
(gdb)
106         if (r->stream) {
(gdb)
112         preread = r->header_in->last - r->header_in->pos;
(gdb)
114         if (preread) {
(gdb)
118             ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
(gdb)
124             rc = ngx_http_request_body_filter(r, &out);
(gdb)
121             out.buf = r->header_in;
(gdb)
122             out.next = NULL;
(gdb)
124             rc = ngx_http_request_body_filter(r, &out);
(gdb)
126             if (rc != NGX_OK) {
(gdb)
130             r->request_length += preread - (r->header_in->last - r->header_in->pos);
(gdb)
132             if (!r->headers_in.chunked
(gdb)
133                 && rb->rest > 0
(gdb)
168         if (rb->rest == 0) {
(gdb)
170             r->request_body_no_buffering = 0;
(gdb)
171             post_handler(r);
(gdb)
172             return NGX_OK;
(gdb)
233     }
(gdb)
ngx_http_t1k_req_module_handler (r=0x150bd60) at /opt/openresty-1.19.3.1-build/openresty-1.19.3.1/modules/ngx-http-t1k-module-t1k-3.0.4-5a1ed627/ngx_http_t1k_req.c:182
182         if (rc == NGX_ERROR || rc >= NGX_HTTP_SPECIAL_RESPONSE) {
(gdb)

[monkeyDluffy6017]

@mcdullbloom have you solved your problem?

[mcdullbloom]

not yet.

@mcdullbloom have you solved your problem?

[Abhijeetmishr]

Hi @mcdullbloom are you talking about this only ? correct me if I have misunderstood it.

[mcdullbloom]

Hi @mcdullbloom are you talking about this only ? correct me if I have misunderstood it.

@Abhijeetmishr It seems that your requet header is bad and apisix returns 400.Our problems are not the same.

I doubt that a third nginx module which run in access phase ahead of access_by_lua_module may make the value set of max_body_size by client_control plugin not work.