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
