#1384 closed defect (fixed)
request body may be corrupted when content-length is not set in headers using http2
Reported by: | Iven Xu | Owned by: | |
---|---|---|---|
Priority: | minor | Milestone: | |
Component: | nginx-module | Version: | 1.13.x |
Keywords: | http2 request body corrupted content-length | Cc: | |
uname -a: | Linux 3.10.106-1-0044 #1 SMP Mon Jul 17 15:20:12 CST 2017 x86_64 x86_64 x86_64 GNU/Linux | ||
nginx -V: | nginx version: nginx/1.13.5 |
Description
Hi,
Recently I use nginx as proxy server with a problem. If a client send a post request without Content-Length set in the request header using http2(proxy_request_buffering is on), the request body may be corrupted when nginx send request body to upstream in ngx_http_upstream_send_request_body. If content-length is set in the header, the request body will be ok. In addition, it will be ok if proxy_request_buffering is off.
I want to find the answer in the code. In ngx_http_v2_read_request_body, if Content-Length is not set in the headers, r->headers_in.content_length_n == -1, will enter below branch.
} else {
if (stream->preread) {
/* enforce writing preread buffer to file */
r->request_body_in_file_only = 1;
}
rb->buf = ngx_calloc_buf(r->pool);
if (rb->buf != NULL) {
rb->buf->sync = 1;
}
}
As rb->buf->sync is set with 1, in ngx_http_v2_process_request_body, will not copy request body from recv_buffer of http2 module to buf->last.
if (size) {
if (buf->sync) {
buf->pos = buf->start = pos;
buf->last = buf->end = pos + size;
} else {
if (size > (size_t) (buf->end - buf->last)) {
ngx_log_error(NGX_LOG_INFO, fc->log, 0,
"client intended to send body data "
"larger than declared");
return NGX_HTTP_BAD_REQUEST;
}
buf->last = ngx_cpymem(buf->last, pos, size);
}
}
But recv_buffer is shared memory for all http2 request, the request body may be corrputed when call ngx_http_upstream_send_request_body.
I also try to find whether it is necessary in the rfc7540. My understanding is that, Content-Length in the header is optional. If Conntent-Length is not set, nginx should read request body from DATA frame. I saw a description like this in the rfc7540:
An HTTP POST request that includes request header fields and payload data is transmitted as one HEADERS frame, followed by zero or more CONTINUATION frames containing the request header fields, followed by one or more DATA frames, with the last CONTINUATION (or HEADERS) frame having the END_HEADERS flag set and the final DATA frame having the END_STREAM flag set.
So, in my opinion nginx should handle the request body corretly even if the Content-Length is not set. Am I right?
Change History (7)
comment:1 by , 7 years ago
Status: | new → accepted |
---|
comment:2 by , 7 years ago
comment:3 by , 7 years ago
Resolution: | → fixed |
---|---|
Status: | accepted → closed |
Fixed by b6dc472299da (will be part of nginx 1.13.6).