httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Luca Toscano <toscano.l...@gmail.com>
Subject Re: Feedback for mod_proxy_fcgi and 412 precondition failures
Date Fri, 09 Sep 2016 09:30:19 GMT
2016-09-07 22:41 GMT+02:00 Jacob Champion <champion.p@gmail.com>:

> 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

Mime
View raw message