On Fri, May 13, 2016 at 6:49 PM, William A Rowe Jr <wr...@rowe-clan.net> wrote: > On Fri, May 13, 2016 at 11:41 AM, Yann Ylavic <ylavic....@gmail.com> wrote: >> >> On Thu, May 12, 2016 at 4:55 PM, William A Rowe Jr <wr...@rowe-clan.net> >> wrote: >> > >> > I'd explained in another thread this week why this patch is invalid, >> > and I've gone ahead and reverted. >> > >> > We agreed there is a defect here, what about the attached fix? >> >> Looks good, if something bad happened in ap_read_request() we already >> have responded with ap_send_error_response(). >> >> What may be missing is reporting SERVER_BUSY_WRITE (with the >> partial/bad request) in ap_send_error_response(), and then we'd >> probably don't need to pass r for SERVER_BUSY_LOG in the error paths >> of ap_read_request(). > > > AIUI, at that point in the code, if there was an error response it was > already > sent, the other paths appear to be simple disconnection states with no > further > logging or socket activity, other than forcing lingering close... perhaps.
That's the case where reading the request line fails for anything but URI_TOO_LARGE, BAD_REQUEST or REQUEST_TIMEOUT. But for the above cases or an error while reading/validating the headers or running post_read_request(), we finally call ap_die() or ap_send_error_response(). I think we should report SERVER_BUSY_WRITE with r before those calls, and use NULL for SERVER_BUSY_LOG.
