On 10/12/2010 13:54, Konstantin Kolinko wrote:
> 2010/12/10 Brett Delle Grazie <[email protected]>:
>> (...)
>>
>> 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: [email protected]
> For additional commands, e-mail: [email protected]
>
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
