httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jacob Champion <champio...@gmail.com>
Subject Re: Feedback for mod_proxy_fcgi and 412 precondition failures
Date Wed, 07 Sep 2016 20:41:35 GMT
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.

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.

(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?)

--Jacob

Mime
View raw message