On 10/12/2010 13:54, Konstantin Kolinko wrote:
> 2010/12/10 Brett Delle Grazie <brett.dellegra...@gmail.com>:
>> (...)
>>
>> Everything works fine except if the client has an X-Forwarded-For header
>> _already_ in the request (perhaps due to Squid in forward proxy on client
>> side).
>>
>> Thus offending request looks like:
>>
>> Headers (fake IP addresses used):
>> X-Forwarded-For: 192.168.0.4  (client side added)
>> ... (some other headers) ...
>> X-Forwarded-For: 224.212.128.2 (added by HAproxy - this is the actual IP of
>> the client's squid proxy).
>> ... (some other headers) ...
>>
>> Now Tomcat's RemoteIP valve doesn't appear to handle this situation
>> correctly - it returns 192.168.0.4 instead of the expected 224.212.128.2
>>
> 
> Looks like a bug,
> 
> Please add it to bugzilla, as Mark suggested.
> 
> BTW, I think that the following change can fix it:
> (for current tc6.0.x, not tested!)

I don't think so. I think the problem is further up on line 558:
String[] remoteIpHeaderValue =
commaDelimitedListToStringArray(request.getHeader(remoteIpHeader));

I'm pretty sure that needs to be using request.getHeaders() but I
haven't tested it either ;)

Fortunately, the contribution of this valve and the matching filter came
with an extensive set of unit tests. It should be easy to extend the
tests to cover multiple headers.

Mark

> 
> Index: java/org/apache/catalina/valves/RemoteIpValve.java
> ===================================================================
> --- java/org/apache/catalina/valves/RemoteIpValve.java        (revision 
> 1044342)
> +++ java/org/apache/catalina/valves/RemoteIpValve.java        (working copy)
> @@ -564,12 +564,12 @@
>              // loop on remoteIpHeaderValue to find the first trusted
> remote ip and to build the proxies chain
>              for (idx = remoteIpHeaderValue.length - 1; idx >= 0; idx--) {
>                  String currentRemoteIp = remoteIpHeaderValue[idx];
> -                remoteIp = currentRemoteIp;
>                  if (matchesOne(currentRemoteIp, internalProxies)) {
>                      // do nothing, internalProxies IPs are not appended to 
> the
>                  } else if (matchesOne(currentRemoteIp, trustedProxies)) {
>                      proxiesHeaderValue.addFirst(currentRemoteIp);
>                  } else {
> +                    remoteIp = currentRemoteIp;
>                      idx--; // decrement idx because break statement
> doesn't do it
>                      break;
>                  }
> 
> Best regards,
> Konstantin Kolinko
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: users-h...@tomcat.apache.org
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org

Reply via email to