Thanks Maxim.

This is new patch, I think this may be better.

I think ngx_handle_read_event can't be avoid(in
ngx_http_discard_request_body), because it doesn't known whether or not need
to add read event in ngx_http_discard_request_body. So there need
ngx_handle_read_event to judge.

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,11 +476,13 @@

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;
+ rc = ngx_http_read_discarded_request_body(r);
+
+ if (rc == NGX_ERROR) {
+ return NGX_HTTP_CLIENT_CLOSED_REQUEST;
}

- if (ngx_http_read_discarded_request_body(r) == NGX_OK) {
+ if (rc == NGX_OK) {
r->lingering_close = 0;

} else {
@@ -488,6 +490,10 @@
r->discard_body = 1;
}

+ if (ngx_handle_read_event(rev, 0) != NGX_OK) {
+ return NGX_HTTP_INTERNAL_SERVER_ERROR;
+ }
+
return NGX_OK;
}

@@ -527,6 +533,11 @@

rc = ngx_http_read_discarded_request_body(r);

+ if (rc == NGX_ERROR) {
+ ngx_http_finalize_request(r, NGX_HTTP_CLIENT_CLOSED_REQUEST);
+ return;
+ }
+
if (rc == NGX_OK) {
r->discard_body = 0;
r->lingering_close = 0;
@@ -583,19 +594,15 @@

n = r->connection->recv(r->connection, buffer, size);

- if (n == NGX_ERROR) {
+ if (n == 0 || n == NGX_ERROR) {
r->connection->error = 1;
- return NGX_OK;
+ return NGX_ERROR;
}

if (n == NGX_AGAIN) {
return NGX_AGAIN;
}

- if (n == 0) {
- return NGX_OK;
- }
-
r->headers_in.content_length_n -= n;
}
}


On Mon, Sep 26, 2011 at 11:50 PM, Maxim Dounin <mdounin@mdounin.ru> wrote:

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



--
douban:www.douban.com/people/mustang/

blog: www.pagefault.info

twitter: www.twitter.com/minibobo

weibo: www.weibo.com/diaoliang
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel