On 02/06/2007 04:49 PM, Jim Jagielski wrote: > > On Feb 6, 2007, at 8:10 AM, Joe Orton wrote: > >> On Mon, Feb 05, 2007 at 08:46:01PM -0000, [EMAIL PROTECTED] wrote: >> >>> Author: rpluem >>> Date: Mon Feb 5 12:46:01 2007 >>> New Revision: 503863 >>> >>> URL: http://svn.apache.org/viewvc?view=rev&rev=503863 >>> Log: >>> * Add missing Changelog entry for PR41056 / PR 19954. This was fixed >>> in r480135. >> >> >> It looks like the regression is still present in this code, an EAGAIN >> from the ap_get_brigade() to read the post-chunk CRLF is not handled and >> will be treated as an error, line 295 of http_filters.c >> >> (AFAIK the test suite does not exercise the code paths for a NONBLOCKing >> read of a chunked request body, testing this reliably requires a client >> which drip-feeds bytes to trigger the EAGAIN on the server side on every >> possible read call) >> > > Joe, can you see if the below fixes it: > > Index: http_filters.c > =================================================================== > --- http_filters.c (revision 504180) > +++ http_filters.c (working copy) > @@ -299,7 +299,8 @@ > rv = APR_SUCCESS; > } > - if (rv == APR_SUCCESS) { > + if (rv == APR_SUCCESS || > + (ctx->state == BODY_CHUNK && > (APR_STATUS_IS_EAGAIN(rv))) ) { > /* Read the real chunk line. */ > rv = ap_get_brigade(f->next, bb, AP_MODE_GETLINE, > block, 0); >
Wouldn't that do the wrong thing? Suppose we miss the the CRLF in line 295 because it is not available on the wire, but available later during our call to ap_get_brigade in line 304. In this case we would read an empty line where we expect to read the chunk size and we would bail out in line 319 with an error. Joe could you please refresh my mind what was wrong in returning an APR_EAGAIN? We actually do this explicity in line 311. If this is wrong I guess this needs to be fixed there as well. Regards Rüdiger
