ngx_http_discard_request_body may read incomplete data?

[Maxim Dounin]

Hello!

On Mon, Sep 26, 2011 at 11:34:42PM +0800, Simon Liu wrote:

> Thanks .
> 
> This is patch:
> 
> Index: src/http/ngx_http_request_body.c
> ===================================================================
> --- src/http/ngx_http_request_body.c    (revision 4146)
> +++ src/http/ngx_http_request_body.c    (working copy)
> @@ -476,10 +476,6 @@
> 
>      r->read_event_handler = ngx_http_discarded_request_body_handler;
> 
> -    if (ngx_handle_read_event(rev, 0) != NGX_OK) {
> -        return NGX_HTTP_INTERNAL_SERVER_ERROR;
> -    }
> -
>      if (ngx_http_read_discarded_request_body(r) == NGX_OK) {
>          r->lingering_close = 0;
> 
> @@ -488,6 +484,10 @@
>          r->discard_body = 1;
>      }
> 
> +    if (ngx_handle_read_event(rev, 0) != NGX_OK) {
> +        return NGX_HTTP_INTERNAL_SERVER_ERROR;
> +    }
> +
>      return NGX_OK;
>  }

It's better to avoid ngx_handle_read_event() altogether in 
case ngx_http_read_discarded_request_body() returns NGX_OK (i.e. 
everything is read, no futher actions required).

It would be a bit more readable and slightly more optimal.

Maxim Dounin

> 
> 
> On Mon, Sep 26, 2011 at 11:08 PM, Maxim Dounin <mdounin at mdounin.ru> wrote:
> 
> > Hello!
> >
> > On Mon, Sep 26, 2011 at 10:42:20PM +0800, Simon Liu wrote:
> >
> > > Hi all.
> > >
> > > My nginx version is svn trunk.
> > >
> > > Nginx will call ngx_handle_read_event, and then
> > > call ngx_http_read_discarded_request_body In
> > ngx_http_discard_request_body .
> > >  So this will cause client's body read  incomplete data , because
> > > ngx_handle_read_event
> > > may delete read event when use level event.
> > >
> > > ngx_handle_read_event:
> > >
> > > else if (ngx_event_flags & NGX_USE_LEVEL_EVENT) {
> > >
> > >         /* select, poll, /dev/poll */
> > > .................................................................
> > >
> > >         if (rev->active && (rev->ready || (flags & NGX_CLOSE_EVENT))) {
> > >             if (ngx_del_event(rev, NGX_READ_EVENT, NGX_LEVEL_EVENT |
> > flags)
> > >                 == NGX_ERROR)
> > >             {
> > >                 return NGX_ERROR;
> > >             }
> > >
> > >             return NGX_OK;
> > >         }
> > >
> > >
> > > ngx_http_discard_request_body:
> > >
> > >     r->read_event_handler = ngx_http_discarded_request_body_handler;
> > >
> > >     if (ngx_handle_read_event(rev, 0) != NGX_OK) {
> > >         return NGX_HTTP_INTERNAL_SERVER_ERROR;
> > >     }
> > >
> > >     if (ngx_http_read_discarded_request_body(r) == NGX_OK) {
> > >         r->lingering_close = 0;
> > >
> > >
> > > thanks!
> >
> > Ack, ngx_handle_read_event() should be after
> > ngx_http_read_discarded_request_body() call, like it's done in
> > ngx_http_discarded_request_body_handler().
> >
> > Care to provide patch?
> >
> > Maxim Dounin
> >
> > _______________________________________________
> > nginx-devel mailing list
> > nginx-devel at nginx.org
> > http://mailman.nginx.org/mailman/listinfo/nginx-devel
> >

> Index: src/http/ngx_http_request_body.c
> ===================================================================
> --- src/http/ngx_http_request_body.c	(revision 4146)
> +++ src/http/ngx_http_request_body.c	(working copy)
> @@ -476,10 +476,6 @@
>  
>      r->read_event_handler = ngx_http_discarded_request_body_handler;
>  
> -    if (ngx_handle_read_event(rev, 0) != NGX_OK) {
> -        return NGX_HTTP_INTERNAL_SERVER_ERROR;
> -    }
> -
>      if (ngx_http_read_discarded_request_body(r) == NGX_OK) {
>          r->lingering_close = 0;
>  
> @@ -488,6 +484,10 @@
>          r->discard_body = 1;
>      }
>  
> +    if (ngx_handle_read_event(rev, 0) != NGX_OK) {
> +        return NGX_HTTP_INTERNAL_SERVER_ERROR;
> +    }
> +
>      return NGX_OK;
>  }
>  

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