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

[Maxim Dounin]

Hello!

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

> # HG changeset patch
> # User Piotr Sikora <piotrsikora at 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 at 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/