On Monday 28 January 2013 15:30:35 Maxim Dounin wrote:
> Hello!
>
> On Sun, Jan 27, 2013 at 12:07:37AM -0800, agentzh wrote:
> > Hello!
> >
> > I've noticed that for nginx 1.3.9+, "client_body_in_file_only on" no
> > longer always sets r->request_body->bufs, which makes upstream modules
> > like ngx_proxy send empty request bodies to the backend server.
> >
> > Here's a minimal example that demonstrates the issue:
> > location = /t {
> >
> > client_body_in_file_only on;
> > proxy_pass http://127.0.0.1:1234;
> >
> > }
> >
> > And run nc to listen on the local port 1234:
> > $ nc -l 1234
> >
> > Then issue a simple POST request to location = /t:
> > $ curl -d 'hello world' localhost/t
> >
> > When using nginx 1.3.9+, we get the raw HTTP request sent by ngx_proxy:
> > $ nc -l 1234
> > POST /t HTTP/1.0
> > Host: 127.0.0.1:1234
> > Connection: close
> > Content-Length: 11
> > User-Agent: curl/7.24.0 (x86_64-redhat-linux-gnu) libcurl/7.24.0 ...
> > Accept: */*
> > Content-Type: application/x-www-form-urlencoded
> >
> > That is, when the request body is completely preread into the client
> > header buffer, the request body will only be hooked into
> > r->request_body->temp_file but not r->request_body->bufs.
> >
> > But when the request body is big enough that it is not completely
> > preread into the client header buffer, then the a in-file buf will
> > still be properly inserted into r->request_body->bufs and we can get
> > the expected request body sent from ngx_proxy.
>
> Thnx, the test for client_body_in_file_only actually checked file,
> so it wasn't noticed. :)
>
> The following patch should fix this:
>
> # HG changeset patch
> # User Maxim Dounin <mdounin@mdounin.ru>
> # Date 1359372279 -14400
> # Node ID 1f6b73a7b7c9992d2e6853413a8f2c599c1e8ea8
> # Parent 153d131f0b7aa28fde12a94fd6a7e78a20743a3a
> Request body: fixed client_body_in_file_only.
>
> Broken while introducing chunked request body reading support in 1.3.9.
>

IMHO it's worth to specify that it was broken by r4931.

> diff --git a/src/http/ngx_http_request_body.c
> b/src/http/ngx_http_request_body.c --- a/src/http/ngx_http_request_body.c
> +++ b/src/http/ngx_http_request_body.c
> @@ -35,7 +35,8 @@ ngx_http_read_client_request_body(ngx_ht
> size_t preread;
> ssize_t size;
> ngx_int_t rc;
> - ngx_chain_t out;
> + ngx_buf_t *b;
> + ngx_chain_t out, *cl;
> ngx_http_request_body_t *rb;
> ngx_http_core_loc_conf_t *clcf;
>
> @@ -128,6 +129,21 @@ ngx_http_read_client_request_body(ngx_ht
> rc = NGX_HTTP_INTERNAL_SERVER_ERROR;
> goto done;
> }
> +
> + cl = ngx_chain_get_free_buf(r->pool, &rb->free);
> + if (cl == NULL) {
> + return NGX_HTTP_INTERNAL_SERVER_ERROR;
> + }
> +
> + b = cl->buf;
> +
> + ngx_memzero(b, sizeof(ngx_buf_t));
> +
> + b->in_file = 1;
> + b->file_last = rb->temp_file->file.offset;
> + b->file = &rb->temp_file->file;
> +
> + rb->bufs = cl;
> }
>
> post_handler(r);

Looks good and works fine (according to your test).

wbr, Valentin V. Bartenev

--
http://nginx.com/support.html
http://nginx.org/en/donation.html

_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel