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
nginx@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx
