Hello!

On Tue, Nov 27, 2018 at 02:34:16AM -0800, Piotr Sikora via nginx-devel wrote:

> # HG changeset patch
> # User Piotr Sikora <piotrsikora@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 <piotrsikora@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);
>

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.

--
Maxim Dounin
http://mdounin.ru/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel