read request body with http2

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

read request body with http2

j94305
Hello, i'm developing a custom module that needs to read the request body to
get some data from it.
To read the request body i'm using

ngx_http_read_client_request_body

and in the callback i use

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;
        }

To put the data in a char buffer

Aside from the fact that put request body in a buffer is wrong because it
can became very big...

I notice that if i enable http2 connection, the request body doesn't get
read at all.... I notice the function read_client_request_body is never
called...
Am i missing something or there are some specail way to wait/read the
request body with http2? By disabling it (remove support from nginx.conf)
the module works as it should and i can correctly read the request body.

I really hope this can be solved as i don't want to remove http2 support for
this...

Posted at Nginx Forum: https://forum.nginx.org/read.php?2,285859,285859#msg-285859

_______________________________________________
nginx mailing list
[hidden email]
http://mailman.nginx.org/mailman/listinfo/nginx
Reply | Threaded
Open this post in threaded view
|

Re: read request body with http2

Maxim Dounin
Hello!

On Sun, Oct 13, 2019 at 08:47:16PM -0400, Ansuel wrote:

> Hello, i'm developing a custom module that needs to read the request body to
> get some data from it.
> To read the request body i'm using
>
> ngx_http_read_client_request_body
>
> and in the callback i use
>
> 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;
> }
>
> To put the data in a char buffer

Note that unless you've specifically tuned client_body_buffer_size
and client_max_body_size, there can be in-file buffers in
r->request_body->bufs.

> Aside from the fact that put request body in a buffer is wrong because it
> can became very big...
>
> I notice that if i enable http2 connection, the request body doesn't get
> read at all.... I notice the function read_client_request_body is never
> called...
> Am i missing something or there are some specail way to wait/read the
> request body with http2? By disabling it (remove support from nginx.conf)
> the module works as it should and i can correctly read the request body.

There is no "read_client_request_body" function in nginx.  If
that's how you've called your handler function you pass into
ngx_http_read_client_request_body(), and it's never called -
probably there is something wrong with how you handle things.

Consider looking at the examples at the examples at the
Developer's Guide, probably you'll be able to find out what you've
did wrong, and/or will be able to reimplement it correctly from
scratch:

http://nginx.org/en/docs/dev/development_guide.html#http_request_body

> I really hope this can be solved as i don't want to remove http2 support for
> this...

Certainly reading request body works fine with HTTP/2, and it is
used in many standard modules, including proxy.

--
Maxim Dounin
http://mdounin.ru/
_______________________________________________
nginx mailing list
[hidden email]
http://mailman.nginx.org/mailman/listinfo/nginx
Reply | Threaded
Open this post in threaded view
|

Re: read request body with http2

j94305
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;
                }


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?

Posted at Nginx Forum: https://forum.nginx.org/read.php?2,285859,285870#msg-285870

_______________________________________________
nginx mailing list
[hidden email]
http://mailman.nginx.org/mailman/listinfo/nginx
Reply | Threaded
Open this post in threaded view
|

Re: 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/
_______________________________________________
nginx mailing list
[hidden email]
http://mailman.nginx.org/mailman/listinfo/nginx
Reply | Threaded
Open this post in threaded view
|

Re: read request body with http2

j94305
I don't know if this is a bug or not but... yes you were right...

All the work should be done in the ngx_http_read_client_request_body
handler, as it does duplicate the r connection to another address

What confused me is the fact that this is only done in http2 connections...

I really don't know why but all the changes i do in the read_client_request
handler reflect to the main ngx_http_request_t struct. I thought the code
was cleaner by having the body read in a function and parse the data from it
in another function.

Well this doesn't work with http2 because all the changes done in
read_client_request handler get lost and doesn't apply to the main struct...


Think this is a bug as the behavior should be similar and not change across
http versions...

Sorry for the bad explanation... Also i think this should be noted in the
wiki.

Posted at Nginx Forum: https://forum.nginx.org/read.php?2,285859,285920#msg-285920

_______________________________________________
nginx mailing list
[hidden email]
http://mailman.nginx.org/mailman/listinfo/nginx
Reply | Threaded
Open this post in threaded view
|

Re: read request body with http2

Maxim Dounin
Hello!

On Thu, Oct 17, 2019 at 04:42:54PM -0400, Ansuel wrote:

> I don't know if this is a bug or not but... yes you were right...
>
> All the work should be done in the ngx_http_read_client_request_body
> handler, as it does duplicate the r connection to another address
>
> What confused me is the fact that this is only done in http2 connections...
>
> I really don't know why but all the changes i do in the read_client_request
> handler reflect to the main ngx_http_request_t struct. I thought the code
> was cleaner by having the body read in a function and parse the data from it
> in another function.
>
> Well this doesn't work with http2 because all the changes done in
> read_client_request handler get lost and doesn't apply to the main struct...
>
>
> Think this is a bug as the behavior should be similar and not change across
> http versions...
>
> Sorry for the bad explanation... Also i think this should be noted in the
> wiki.

It looks like you don't understand how things work.  First
of all, when the ngx_http_read_client_request_body() function
returns, it doesn't mean that the request body was successfully
read, and the body handler function was already called.

You may want to start with reading more about event-driven
programming in general.  Also, as previously suggested, you may
want to read Development guide, it has enough information about
how things work and how they are expected to be handled.

Also, if you want to better understand how nginx works, consider
reading nginx source code - it is the only direct source of the
information.

--
Maxim Dounin
http://mdounin.ru/
_______________________________________________
nginx mailing list
[hidden email]
http://mailman.nginx.org/mailman/listinfo/nginx