httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ruediger Pluem <rpl...@apache.org>
Subject Re: [PATCH] proxy/balancer: fix PR 45434 regression
Date Wed, 25 Jul 2012 01:46:27 GMT


Rainer Jung wrote:
> On 24.07.2012 19:40, Joe Orton wrote:
>> The test case for PR 45434 seems to have regressed across 2.2->2.4.
>>
>> https://issues.apache.org/bugzilla/show_bug.cgi?id=45434
>>
>> I have not tried to understand the mechanics here, but a dumb
>> side-by-side analysis found a missing piece, below.  2.2 hardcodes this
>> as "real + 11" but 2.4 uses the constant elsewhere.  Any reason why this
>> would be wrong?  It fixes the test case I added to t/modules/proxy.t.
>>
>> 2.2.x code for reference:
>>
>> http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c?annotate=1058624#l1090
>>
>> Index: modules/proxy/proxy_util.c
>> ===================================================================
>> --- modules/proxy/proxy_util.c    (revision 1365029)
>> +++ modules/proxy/proxy_util.c    (working copy)
>> @@ -860,7 +860,7 @@
>>               (balancer = ap_proxy_get_balancer(r->pool, sconf, real, 1))) {
>>               int n, l3 = 0;
>>               proxy_worker **worker = (proxy_worker **)balancer->workers->elts;
>> -            const char *urlpart = ap_strchr_c(real, '/');
>> +            const char *urlpart = ap_strchr_c(real + sizeof(BALANCER_PREFIX) - 1,
'/');
>>               if (urlpart) {
>>                   if (!urlpart[1])
>>                       urlpart = NULL;
>>
> 
> The code including the offset was introduced in trunk in
> 
> http://svn.apache.org/viewvc?view=revision&amp;revision=771587
> 
> and then ported back to 2.2 in
> 
> http://svn.apache.org/viewvc?view=revision&revision=777191
> 
> Later in first the offset got replaced by using a separate function call first which
returned a new variable "bname"
> which was already advanced by the correct offset and then put that one into ap_strchr_c:
> 
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?r1=1058621&r2=1058622&diff_format=h
> 
> A bit later the function was rearranged to not return a char * but a boolean and the
new variable was removed again. The
> old variable "real" was now used without the previous offset. That seems to be the culprit
and a bug. There's no
> indication, that this was intentional.
> 
> So: Your change seems good to me.

+1 to this analysis. Patch looks good to me.

Regards

RĂ¼diger


Mime
View raw message