Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Subject: Re: svn commit: r1741310 - in /httpd/httpd/trunk: modules/http2/
 server/ server/mpm/event/ server/mpm/motorz/ server/mpm/simple/
 server/mpm/winnt/ server/mpm/worker/
To: dev@httpd.apache.org
References: <20160427184150.5A41B3A0734@svn01-us-west.apache.org>
 <5721C780.1010809@apache.org> <57270B41.8050608@apache.org>
 <CACsi252WTrZ=pYTabpEK8Py4v26nSrEYpy6fQZJK1ZSz_eq63w@mail.gmail.com>
From: Ruediger Pluem <rpluem@apache.org>
Message-ID: <5728E4CA.80901@apache.org>
Date: Tue, 3 May 2016 19:50:02 +0200
User-Agent: Mozilla/5.0 (X11; Linux i686 on x86_64; rv:43.0) Gecko/20100101
 Firefox/43.0 SeaMonkey/2.40
MIME-Version: 1.0
In-Reply-To: <CACsi252WTrZ=pYTabpEK8Py4v26nSrEYpy6fQZJK1ZSz_eq63w@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
archived-at: Tue, 03 May 2016 17:49:17 -0000


On 05/03/2016 05:51 PM, William A Rowe Jr wrote:
> On Mon, May 2, 2016 at 3:09 AM, Ruediger Pluem <rpluem@apache.org <mailto:rpluem@apache.org>> wrote:
> 
> 
>     On 04/28/2016 10:19 AM, Ruediger Pluem wrote:
>     >
>     > On 04/27/2016 08:41 PM, wrowe@apache.org <mailto:wrowe@apache.org> wrote:
>     >> Author: wrowe
>     >> Date: Wed Apr 27 18:41:49 2016
>     >> New Revision: 1741310
>     >>
>     >> URL: http://svn.apache.org/viewvc?rev=1741310&view=rev
>     >> Log:
>     >>
>     >>   Ensure the useragent_ip is only used in the case
>     >>   where it has been initialized, fall back on the
>     >>   connection's remote_ip if the status is accidently
>     >>   updated from an uninitialized request_rec.
>     >>
>     >> --- httpd/httpd/trunk/server/scoreboard.c (original)
>     >> +++ httpd/httpd/trunk/server/scoreboard.c Wed Apr 27 18:41:49 2016
>     >> @@ -501,7 +501,7 @@ static int update_child_status_internal(
>     >>              copy_request(ws->request, sizeof(ws->request), r);
>     >>          }
>     >>
>     >> -        if (r) {
>     >> +        if (r && r->useragent_ip) {
>     >>              if (!(val = ap_get_useragent_host(r, REMOTE_NOLOOKUP, NULL)))
>     >>                  apr_cpystrn(ws->client, r->useragent_ip, sizeof(ws->client));
>     >
>     > Hm, wouldn't it be better to just encapsulate the above line in an if (r->useragent_ip)
>     > or do we already know that ap_get_useragent_host(r, REMOTE_NOLOOKUP, NULL) cannot deliver
>     > something meaningful if r->useragent_ip == NULL?
> 
> 
> It *could* deliver; but if r->useragent_ip is unset, this is 
> a premature call to set up the score entry, the request
> read hook hasn't completed and will fail to deliver.  But
> I don't know what you mean by "the above line" - you
> mean the apr_cpystrn?  As you point out, the call is
> fairly useless without a completed request rec and 
> known useragent_ip in the first place (you want to
> look up the hostname of null address - but then 
> guard against copying a NULL string once it has
> failed ? :-)
> 
> By falling back on the conn-based lookup, in the code you 
> snipped... we are populating the conn-based hostname, 
> which may be useful datum elsewhere...

Good point. I missed the else if (c) stuff below. Thanks for pointing out.

Regards

Rüdiger