Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
MIME-Version: 1.0
In-Reply-To: <CACsi251PyON2noYmE+OEW3+QXL8ciJ3YsrJsdCfN-fs=tSPbQA@mail.gmail.com>
References: <20160508135338.9C0BD3A0336@svn01-us-west.apache.org>
	<CACsi251nAozCH6mQ_iNh_tAEE+ANf9OmVmnc0gQd4qWE14JTJA@mail.gmail.com>
	<CAKQ1sVNL+nFA3r=Uc52Gt3vdTy+c5Oq5E=yy-Becryy3FHqoSw@mail.gmail.com>
	<CACsi251PyON2noYmE+OEW3+QXL8ciJ3YsrJsdCfN-fs=tSPbQA@mail.gmail.com>
Date: Fri, 13 May 2016 18:57:49 +0200
Message-ID: <CAKQ1sVPyh-bBja6x1UOFXZ1zg-PW+8pgFH=n3wuOSXjZ5FUy9Q@mail.gmail.com>
Subject: Re: svn commit: r1742822 - /httpd/httpd/trunk/modules/http/http_core.c
From: Yann Ylavic <ylavic.dev@gmail.com>
To: httpd-dev <dev@httpd.apache.org>
Content-Type: text/plain; charset=UTF-8
archived-at: Fri, 13 May 2016 16:57:55 -0000

On Fri, May 13, 2016 at 6:49 PM, William A Rowe Jr <wrowe@rowe-clan.net> wrote:
> On Fri, May 13, 2016 at 11:41 AM, Yann Ylavic <ylavic.dev@gmail.com> wrote:
>>
>> On Thu, May 12, 2016 at 4:55 PM, William A Rowe Jr <wrowe@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.