httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ruediger Pluem <rpl...@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/
Date Tue, 03 May 2016 17:50:02 GMT


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


Mime
View raw message