2016-09-07 22:41 GMT+02:00 Jacob Champion <[email protected]>: > On 09/07/2016 11:25 AM, Luca Toscano wrote: > > Index: modules/proxy/mod_proxy_fcgi.c > > =================================================================== > > --- modules/proxy/mod_proxy_fcgi.c (revision 1759650) > > +++ modules/proxy/mod_proxy_fcgi.c (working copy) > > @@ -670,7 +670,7 @@ > > * bogus reads. */ > > ignore_body = 1; > > } > > - else { > > + else if (status != > HTTP_PRECONDITION_FAILED) { > > ap_log_rerror(APLOG_MARK, > APLOG_ERR, 0, r, APLOGNO(01070) > > "Error parsing > script headers"); > > rv = APR_EINVAL; > > > > I can't find the last conversation we had over this in my outbox and my > memory has holes; sorry if I repeat anything that was already covered a > while back. >
Thanks for following up :) > > I'd prefer not to patch this with another special case, but fix the > architecture instead. The ap_scan_script_header_xxx functions are > documented to be pass/fail, but they're actually tri-state: pass, hard > failure (5xx), or conditional "failure". It seems like the recent bugs > associated with this code are due to interactions with this third state. > > Ideally, we wouldn't be doing conditional logic in the function that is > documented to be a dumb header parser, but if we can't change that due to > compatibility concerns, we should at least adjust calling code to > understand the tri-state. In other words: OK = pass, 5xx = fail, anything > else is potentially a conditional failure that we might need to handle > specially. > > 1) Do we need to return a message-body in this use case? Maybe handling >> HTTP_PRECONDITION_FAILURE before the EOS bucket is sent? >> > > Since this "optimization" only happens for GET/HEAD, I think we should > scrap the response body. > > 2) Should the HTTP_PRECONDITION_FAILED case be coupled with >> HTTP_NOT_MODIFIED (so leveraging the ignore_body = 1) ? IIUC it >> shouldn't change much.. >> > > Makes sense to me. If *all* the conditional failure codes (are there any > others?) can be treated in the same way, that would be a good way to handle > the "third state" logic. > +1 > > (Tangent for a separate conversation: how do we feel about the fact that > this optimization still makes the backend do the same amount of work to > generate the response? Would it be better for us to abort the FCGI stream > once we figure out we're not going to use it?) My two cents: it could be an option, some code for a similar issue (client disconnects) has been written in https://bz.apache.org/bugzilla/show_bug.cgi?id=56188 and could be taken as example. It would be a very delicate change though, so not sure if it is worth the risk of introducing unexpected bugs. Luca
