Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=50453

Correctly handle multiple X-Forwarded-For headers

git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1055482 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/java/org/apache/catalina/filters/RemoteIpFilter.java b/java/org/apache/catalina/filters/RemoteIpFilter.java
index 1322847..97d6aa7 100644 (file)
--- a/java/org/apache/catalina/filters/RemoteIpFilter.java
+++ b/java/org/apache/catalina/filters/RemoteIpFilter.java
@@ -720,8 +720,17 @@ public class RemoteIpFilter implements Filter {
             String remoteIp = null;
             // In java 6, proxiesHeaderValue should be declared as a java.util.Deque
             LinkedList<String> proxiesHeaderValue = new LinkedList<String>();
+            StringBuffer concatRemoteIpHeaderValue = new StringBuffer();
             
-            String[] remoteIpHeaderValue = commaDelimitedListToStringArray(request.getHeader(remoteIpHeader));
+            for (Enumeration<String> e = request.getHeaders(remoteIpHeader); e.hasMoreElements();) {
+                if (concatRemoteIpHeaderValue.length() > 0) {
+                    concatRemoteIpHeaderValue.append(", ");
+                }
+
+                concatRemoteIpHeaderValue.append(e.nextElement());
+            }
+
+            String[] remoteIpHeaderValue = commaDelimitedListToStringArray(concatRemoteIpHeaderValue.toString());
             int idx;
             // loop on remoteIpHeaderValue to find the first trusted remote ip and to build the proxies chain
             for (idx = remoteIpHeaderValue.length - 1; idx >= 0; idx--) {
@@ -782,11 +791,11 @@ public class RemoteIpFilter implements Filter {
                 log.debug("Incoming request " + request.getRequestURI() + " with originalRemoteAddr '" + request.getRemoteAddr()
                         + "', originalRemoteHost='" + request.getRemoteHost() + "', originalSecure='" + request.isSecure()
                         + "', originalScheme='" + request.getScheme() + "', original[" + remoteIpHeader + "]='"
-                        + request.getHeader(remoteIpHeader) + ", original[" + protocolHeader + "]='"
+                        + concatRemoteIpHeaderValue + "', original[" + protocolHeader + "]='"
                         + (protocolHeader == null ? null : request.getHeader(protocolHeader)) + "' will be seen as newRemoteAddr='"
                         + xRequest.getRemoteAddr() + "', newRemoteHost='" + xRequest.getRemoteHost() + "', newScheme='"
                         + xRequest.getScheme() + "', newSecure='" + xRequest.isSecure() + "', new[" + remoteIpHeader + "]='"
-                        + xRequest.getHeader(remoteIpHeader) + ", new[" + proxiesHeader + "]='" + xRequest.getHeader(proxiesHeader) + "'");
+                        + xRequest.getHeader(remoteIpHeader) + "', new[" + proxiesHeader + "]='" + xRequest.getHeader(proxiesHeader) + "'");
             }
             chain.doFilter(xRequest, response);
         } else {

diff --git a/java/org/apache/catalina/valves/RemoteIpValve.java b/java/org/apache/catalina/valves/RemoteIpValve.java
index 9a113c4..afdcc1b 100644 (file)
--- a/java/org/apache/catalina/valves/RemoteIpValve.java
+++ b/java/org/apache/catalina/valves/RemoteIpValve.java
@@ -19,6 +19,7 @@ package org.apache.catalina.valves;
 
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Enumeration;
 import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
@@ -548,8 +549,17 @@ public class RemoteIpValve extends ValveBase {
             String remoteIp = null;
             // In java 6, proxiesHeaderValue should be declared as a java.util.Deque
             LinkedList<String> proxiesHeaderValue = new LinkedList<String>();
+            StringBuffer concatRemoteIpHeaderValue = new StringBuffer();
             
-            String[] remoteIpHeaderValue = commaDelimitedListToStringArray(request.getHeader(remoteIpHeader));
+            for (Enumeration<String> e = request.getHeaders(remoteIpHeader); e.hasMoreElements();) {
+                if (concatRemoteIpHeaderValue.length() > 0) {
+                    concatRemoteIpHeaderValue.append(", ");
+                }
+
+                concatRemoteIpHeaderValue.append(e.nextElement());
+            }
+
+            String[] remoteIpHeaderValue = commaDelimitedListToStringArray(concatRemoteIpHeaderValue.toString());
             int idx;
             // loop on remoteIpHeaderValue to find the first trusted remote ip and to build the proxies chain
             for (idx = remoteIpHeaderValue.length - 1; idx >= 0; idx--) {

diff --git a/test/org/apache/catalina/filters/TestRemoteIpFilter.java b/test/org/apache/catalina/filters/TestRemoteIpFilter.java
index 725b453..9aad43b 100644 (file)
--- a/test/org/apache/catalina/filters/TestRemoteIpFilter.java
+++ b/test/org/apache/catalina/filters/TestRemoteIpFilter.java
@@ -111,6 +111,10 @@ public class TestRemoteIpFilter extends TomcatBaseTest {
             getCoyoteRequest().getMimeHeaders().setValue(name).setString(value);
         }
 
+        public void addHeader(String name, String value) {
+            getCoyoteRequest().getMimeHeaders().addValue(name).setString(value);
+        }
+
         public void setScheme(String scheme) {
             getCoyoteRequest().scheme().setString(scheme);
         }
@@ -250,7 +254,7 @@ public class TestRemoteIpFilter extends TomcatBaseTest {
         MockHttpServletRequest request = new MockHttpServletRequest();
         request.setRemoteAddr("192.168.0.10");
         request.setRemoteHost("remote-host-original-value");
-        request.setHeader("x-forwarded-for", "140.211.11.130, 192.168.0.10, 192.168.0.11");
+        request.addHeader("x-forwarded-for", "140.211.11.130, 192.168.0.10, 192.168.0.11");
 
         // TEST
         HttpServletRequest actualRequest = testRemoteIpFilter(filterDef, request);
@@ -315,7 +319,9 @@ public class TestRemoteIpFilter extends TomcatBaseTest {
         MockHttpServletRequest request = new MockHttpServletRequest();
         request.setRemoteAddr("192.168.0.10");
         request.setRemoteHost("remote-host-original-value");
-        request.setHeader("x-forwarded-for", "140.211.11.130, proxy1, proxy2");
+        request.addHeader("x-forwarded-for", "140.211.11.130");
+        request.addHeader("x-forwarded-for", "proxy1");
+        request.addHeader("x-forwarded-for", "proxy2");
 
         // TEST
         HttpServletRequest actualRequest = testRemoteIpFilter(filterDef, request);

diff --git a/test/org/apache/catalina/valves/TestRemoteIpValve.java b/test/org/apache/catalina/valves/TestRemoteIpValve.java
index c03dbc5..5ab03f1 100644 (file)
--- a/test/org/apache/catalina/valves/TestRemoteIpValve.java
+++ b/test/org/apache/catalina/valves/TestRemoteIpValve.java
@@ -263,7 +263,9 @@ public class TestRemoteIpValve extends TestCase {
         request.setCoyoteRequest(new org.apache.coyote.Request());
         request.setRemoteAddr("192.168.0.10");
         request.setRemoteHost("remote-host-original-value");
-        request.getCoyoteRequest().getMimeHeaders().addValue("x-forwarded-for").setString("140.211.11.130, proxy1, proxy2");
+        request.getCoyoteRequest().getMimeHeaders().addValue("x-forwarded-for").setString("140.211.11.130");
+        request.getCoyoteRequest().getMimeHeaders().addValue("x-forwarded-for").setString("proxy1");
+        request.getCoyoteRequest().getMimeHeaders().addValue("x-forwarded-for").setString("proxy2");
         
         // TEST
         remoteIpValve.invoke(request, null);

diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index f655372..c304686 100644 (file)
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -168,6 +168,11 @@
         Log a warning if context.xml files define values for properties  that do
         not exist (e.g. if there is a typo in a property name). (markt)
       </fix>
+      <fix>
+        <bug>50453</bug>: Correctly handle multiple <code>X-Forwarded-For</code>
+        headers in the RemoteIpFilter and RemoteIpValve. Patch provided by Jim
+        Riggs. (markt)
+      </fix>
       <add>
         <bug>50541</bug>: Add support for setting the size limit and time limit
         for LDAP seaches when using the JNDI Realm with <code>userSearch</code>.