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, maybe, PRECONDITION_FAILED?).
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.)
And of course we need regression tests for all these fixes... does
anyone have time to review
https://bz.apache.org/bugzilla/attachment.cgi?id=33867
as a possible FCGI regression test template?
--Jacob
