read request body with http2

[Maxim Dounin]

Hello!

On Mon, Oct 14, 2019 at 02:41:33PM -0400, Ansuel wrote:

> this is what i have in the module handler function
> 
> rc = ngx_http_read_client_request_body(r, ngx_http_test_read_req);
> 		if (rc != NGX_OK && rc != NGX_AGAIN) {
> 			return rc;
> 		}

The snippet provided is not enough to conclude if the handling is 
completely wrong and going to cause problems, but this at least 
differs from the proper pattern, and it is going to cause problems 
if not followed by "return NGX_DONE;", assuming the code is used 
in the content phase.

Proper pattern is outlined in the development guide,
(http://nginx.org/en/docs/dev/development_guide.html#http_request_body):

    rc = ngx_http_read_client_request_body(r, ngx_http_foo_init);

    if (rc >= NGX_HTTP_SPECIAL_RESPONSE) {
        return rc;
    }

    return NGX_DONE;

The same pattern can be seen in all nginx modules calling 
ngx_http_read_client_request_body().

> And this is what i have in 
> ngx_http_test_read_req
> 
>   char *buffer = ngx_pcalloc(r->pool, cglcf->req_len);
> 
> for (in = r->request_body->bufs; in; in = in->next) {
> 			len = ngx_buf_size(in->buf);
> 			ngx_memcpy(buffer + pos,in->buf->pos,len);
> 			pos += len;
> 	}
> 
> 
> Do you see anything wrong in how i access the request body?

Sure, see above.

Further, I already wrote that assuming buffers are in memory is 
wrong unless you've specifically tuned configuration parameters.

Note well that your code seems to assume that total request body 
size is less than cglcf->req_len, which is never checked.  This 
can easily cause buffer overflow if the request body is actually 
bigger.

-- 
Maxim Dounin
http://mdounin.ru/