[PATCH] HTTP/2: fixed handling of fully preread request bodies

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora <***@google.com>
# Date 1540435636 25200
# Wed Oct 24 19:47:16 2018 -0700
# Node ID 466c154c5c53b956660211df96331b3c25669485
# Parent be5cb9c67c05ccaf22dab7abba78aa4c1545a8ee
HTTP/2: fixed handling of fully preread request bodies.

Previously, fully preread request body of a request without the
"Content-Length" header was always written to a temporary file.

Reported by Wayne Zhang.

Signed-off-by: Piotr Sikora <***@google.com>

diff -r be5cb9c67c05 -r 466c154c5c53 src/http/v2/ngx_http_v2.c
--- a/src/http/v2/ngx_http_v2.c
+++ b/src/http/v2/ngx_http_v2.c
@@ -3863,6 +3863,12 @@ ngx_http_v2_read_request_body(ngx_http_r
{
rb->buf = ngx_create_temp_buf(r->pool, (size_t) len);

+ } else if (len < 0 && stream->in_closed && stream->preread
+ && !r->request_body_in_file_only)
+ {
+ rb->buf = ngx_create_temp_buf(r->pool,
+ (size_t) ngx_buf_size(stream->preread));
+
} else {
rb->buf = ngx_calloc_buf(r->pool);

[Maxim Dounin]

Hello!

Post by Piotr Sikora via nginx-devel
# HG changeset patch
# Date 1540435636 25200
# Wed Oct 24 19:47:16 2018 -0700
# Node ID 466c154c5c53b956660211df96331b3c25669485
# Parent be5cb9c67c05ccaf22dab7abba78aa4c1545a8ee
HTTP/2: fixed handling of fully preread request bodies.
Previously, fully preread request body of a request without the
"Content-Length" header was always written to a temporary file.
Reported by Wayne Zhang.
diff -r be5cb9c67c05 -r 466c154c5c53 src/http/v2/ngx_http_v2.c
--- a/src/http/v2/ngx_http_v2.c
+++ b/src/http/v2/ngx_http_v2.c
@@ -3863,6 +3863,12 @@ ngx_http_v2_read_request_body(ngx_http_r
{
rb->buf = ngx_create_temp_buf(r->pool, (size_t) len);
+ } else if (len < 0 && stream->in_closed && stream->preread
+ && !r->request_body_in_file_only)
+ {
+ rb->buf = ngx_create_temp_buf(r->pool,
+ (size_t) ngx_buf_size(stream->preread));
+
} else {
rb->buf = ngx_calloc_buf(r->pool);

I believe the problem is more than that. HTTP/2 code in general
does not work well with request bodies if Content-Length is not
known, and always writes them to disk.

In particular, I've seen this happening when working on gRPC
proxying - as gRPC clients does not seem to indicate
Content-Length even if it is known in advance. (In case of gRPC
proxying this is not currently important though, as it does not
happen when using unbuffered proxying.)

I think that if we want to address this, this needs to be
addressed for all cases, not only a particular case of fully
preread request body.