Opened 3 weeks ago

Closed 7 days ago

#2374 closed defect (fixed)

HTTP3 sent too much body data to upstream when use POST request

Reported by: himac@… Owned by: Roman Arutyunyan
Priority: minor Milestone:
Component: http/3 Version:
Keywords: http3 quic post Cc:
uname -a: Linux pekphis107316 4.15.0-156-generic #163-Ubuntu SMP Thu Aug 19 23:31:58 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
nginx -V: nginx version: nginx/1.23.0
built by gcc 7.5.0 (Ubuntu 7.5.0-3ubuntu1~18.04)
built with OpenSSL 1.1.1 (compatible; BoringSSL) (running with BoringSSL)
TLS SNI support enabled
configure arguments: --prefix=/usr/local/nginx_http3/nginx --with-debug --with-ld-opt='-Wl,-rpath, -L/work/unitrans90/boringssl/src/build/ssl/ -L//work/boringssl/src/build/crypto' --with-cc-opt='-I//work//boringssl/src/include/ ' --with-stream --with-stream_quic_module --with-http_v2_module --with-http_v3_module --with-http_stub_status_module --with-stream --with-stream_ssl_module --with-stream_ssl_preread_module --with-http_ssl_module

Description

source code version: nginx-quic-8d0753760546

While use a client send http3 POST request to nginx proxy server, nginx would sent more body data to upstream upload server

I found in ngx_http_v3_request_body_filter function, after ngx_http_v3_parse_data parse http3 DATA frame, the cl buf maybe empty
(because real data in DATA frame would not contain at this time):

(gdb) p b
$11 = (ngx_buf_t *) 0x5555559f8740
(gdb) p *b
$12 = {pos = 0x5555559eefd8 "", last = 0x5555559eefd8 "", file_pos = 0, file_last = 0, start = 0x5555559eefd8 "", end = 0x5555559ef3a8 "www.google.com:4443", tag = 0x5555555df5a5 <ngx_http_read_client_request_body>, file = 0x0,
shadow = 0x0, temporary = 1, memory = 0, mmap = 0, recycled = 0, in_file = 0, flush = 1, sync = 0, last_buf = 0, last_in_chain = 0, last_shadow = 0, temp_file = 0, num = 0}
(gdb) p tl
$13 = (ngx_chain_t *) 0x5555559f7e28
(gdb) p *tl
$14 = {buf = 0x5555559f8740, next = 0x0}
(gdb) list
1603 b->pos = cl->buf->pos;
1604 b->last = cl->buf->last;


after ngx_http_top_request_body_filter, the empty buf chain will be linked to ngx_http_request_t->request_body->bufs:

1680 rc = ngx_http_top_request_body_filter(r, out);
(gdb)
1682 ngx_chain_update_chains(r->pool, &rb->free, &rb->busy, &out,
(gdb) p r->request_body->bufs
$15 = (ngx_chain_t *) 0x5555559f7e38
(gdb) p *r->request_body->bufs
$16 = {buf = 0x5555559f8740, next = 0x0}
(gdb) p r->request_body->free
$17 = (ngx_chain_t *) 0x0

then ngx_chain_update_chains will link the empty buf chain to ngx_http_request_t->request_body->free , because it is empty:

(gdb) n
1685 return rc;
(gdb) p *r->request_body->bufs
$18 = {buf = 0x5555559f8740, next = 0x0}
(gdb) p r->request_body->free
$19 = (ngx_chain_t *) 0x5555559f7e28
(gdb) p r->request_body->free->buf
$20 = (ngx_buf_t *) 0x5555559f8740

when the data of this HTTP3 DATA frame comes, ngx_http_top_request_body_filter called again, and will call ngx_chain_get_free_buf to get buf from ngx_http_request_t->request_body->free

1591 tl = ngx_chain_get_free_buf(r->pool, &rb->free);
(gdb)
1592 if (tl == NULL) {
(gdb) p tl
$21 = (ngx_chain_t *) 0x5555559f7e28
gdb) p tl->buf
$22 = (ngx_buf_t *) 0x5555559f8740

thus after ngx_http_top_request_body_filter, there will be two same buf in ngx_http_request_t->request_body->bufs

1680 rc = ngx_http_top_request_body_filter(r, out);
(gdb)
1682 ngx_chain_update_chains(r->pool, &rb->free, &rb->busy, &out,
(gdb) p *r->request_body->bufs
$1 = {buf = 0x5555559f8740, next = 0x5555559f87e0}
(gdb) p *r->request_body->bufs->next
$2 = {buf = 0x5555559f8740, next = 0x0}
(gdb) p *r->request_body->bufs->buf
$3 = {pos = 0x5555559f8e80 "", last = 0x5555559f8e8d "", file_pos = 0, file_last = 0, start = 0x5555559f8e80 "", end = 0x5555559fae80 "", tag = 0x5555555df5a5 <ngx_http_read_client_request_body>, file = 0x0, shadow = 0x0, temporary = 1,
memory = 0, mmap = 0, recycled = 0, in_file = 0, flush = 0, sync = 0, last_buf = 0, last_in_chain = 0, last_shadow = 0, temp_file = 0, num = 0}


thus too much data will be sent to upstream.

Attachments (1)

h3-empty-rb-buf (2.0 KB ) - added by Roman Arutyunyan 2 weeks ago.

Download all attachments as: .zip

Change History (6)

comment:1 by Roman Arutyunyan, 2 weeks ago

Owner: set to Roman Arutyunyan
Status: newassigned

by Roman Arutyunyan, 2 weeks ago

Attachment: h3-empty-rb-buf added

comment:2 by Roman Arutyunyan, 2 weeks ago

Thanks for reporting this. Please try the attached patch.

in reply to:  2 comment:3 by himac.lee@…, 13 days ago

Replying to Roman Arutyunyan:
OK, I've tested the attached patch and it works fine now.

Thanks for reporting this. Please try the attached patch.

comment:4 by Roman Arutyunyan, 7 days ago

In 8919:f9d7930d0eed/nginx-quic:

HTTP/3: skip empty request body buffers (ticket #2374).

When client DATA frame header and its content come in different QUIC packets,
it may happen that only the header is processed by the first
ngx_http_v3_request_body_filter() call. In this case an empty request body
buffer is added to r->request_body->bufs, which is later reused in a
subsequent ngx_http_v3_request_body_filter() call without being removed from
the body chain. As a result, rb->request_body->bufs ends up with two copies of
the same buffer.

The fix is to avoid adding empty request body buffers to r->request_body->bufs.

comment:5 by Roman Arutyunyan, 7 days ago

Resolution: fixed
Status: assignedclosed

Fix committed, thanks.

Note: See TracTickets for help on using tickets.