httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Luca Toscano <>
Subject Re: mod_proxy_fcgi, 304 responses and bogus error messages
Date Mon, 11 Jul 2016 21:33:10 GMT
2016-07-11 19:44 GMT+02:00 Jacob Champion <>:

> On 07/11/2016 08:53 AM, Luca Toscano wrote:
>> I am looking for some feedback about the patch proposed to figure out if
>> I am on the right track or not. Does it make sense to read all the data
>> returned by a FCGI backend even on error conditions to avoid subsequent
>> spurious reads or is there a smarter method?
> (following up from IRC)
> Regarding your patch: I think that, rather than duplicate the jump back to
> recv_again, the error handling logic around the call to
> ap_scan_script_header_* should be improved. That function is documented to
> return binary success/failure (OK/INTERNAL_SERVER_ERROR), but in reality it
> can return other things too -- definitely NOT_MODIFIED (which is handled),
> perhaps PRECONDITION_FAILED (which is not handled), and maybe others?
> It looks like mod_proxy_fcgi used to take the binary approach ("anything
> that's not OK is an error; break out and run away") and when the exception
> was added for NOT_MODIFIED, that "error break" was left in. Perhaps we
> should just remove that break in the non-fatal-error cases (including,

I followed your suggestion and avoid the break in the HTTP_NOT_MODIFIED use
case, it works nicely! Updated the patch on the bugzilla PR. If anybody has
other suggestions please let us know..

> I don't know the correct answer to your "should we drain on error
> conditions too" question, since I don't know if the connection is torn down
> on errors. (If it's not torn down, then yeah, we should be draining the
> content and padding, but IMO tearing down the connection is probably the
> right thing to do.)

As we were discussing on IRC, the break were originally developed probably
for real error conditions and the HTTP_NOT_MODIFIED use case has been added
later on as a special use case. So I would be inclined to fix this
important use case without increasing the scope of the patch.

> And of course we need regression tests for all these fixes... does anyone
> have time to review
> as a possible FCGI regression test template?
+1 for a good test suite around FCGI handlers, I'll try to extend/review it
during the next days.



View raw message