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