You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cxf.apache.org by re...@apache.org on 2020/08/30 20:20:57 UTC

[cxf] branch master updated: CXF-8331: Reverse proxy url is not reflected when use-x-forwarded-headers is true

This is an automated email from the ASF dual-hosted git repository.

reta pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cxf.git


The following commit(s) were added to refs/heads/master by this push:
     new 50f6d7f  CXF-8331: Reverse proxy url is not reflected when use-x-forwarded-headers is true
50f6d7f is described below

commit 50f6d7fc063b8728cd3903a9a7775c0462860a83
Author: reta <dr...@gmail.com>
AuthorDate: Sun Aug 30 16:12:43 2020 -0400

    CXF-8331: Reverse proxy url is not reflected when use-x-forwarded-headers is true
---
 rt/transports/http/pom.xml                         |   6 +
 .../cxf/transport/servlet/AbstractHTTPServlet.java |   7 +-
 .../HttpServletRequestXForwardedFilterTest.java    | 207 +++++++++++++++++++++
 3 files changed, 219 insertions(+), 1 deletion(-)

diff --git a/rt/transports/http/pom.xml b/rt/transports/http/pom.xml
index 8534869..0bd6d7f 100644
--- a/rt/transports/http/pom.xml
+++ b/rt/transports/http/pom.xml
@@ -69,6 +69,12 @@
             <scope>test</scope>
         </dependency>
         <dependency>
+            <groupId>org.mockito</groupId>
+            <artifactId>mockito-core</artifactId>
+            <version>${cxf.mockito.version}</version>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
             <groupId>org.osgi</groupId>
             <artifactId>org.osgi.core</artifactId>
             <scope>provided</scope>
diff --git a/rt/transports/http/src/main/java/org/apache/cxf/transport/servlet/AbstractHTTPServlet.java b/rt/transports/http/src/main/java/org/apache/cxf/transport/servlet/AbstractHTTPServlet.java
index b53a1ab..a6b5cbb 100644
--- a/rt/transports/http/src/main/java/org/apache/cxf/transport/servlet/AbstractHTTPServlet.java
+++ b/rt/transports/http/src/main/java/org/apache/cxf/transport/servlet/AbstractHTTPServlet.java
@@ -28,10 +28,12 @@ import java.util.Enumeration;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Properties;
 import java.util.ResourceBundle;
 import java.util.logging.Logger;
 import java.util.regex.Pattern;
+import java.util.stream.Stream;
 
 import javax.servlet.Filter;
 import javax.servlet.FilterConfig;
@@ -303,7 +305,10 @@ public abstract class AbstractHTTPServlet extends HttpServlet implements Filter
             String originalPrefix = request.getHeader(X_FORWARDED_PREFIX_HEADER);
             String originalHost = request.getHeader(X_FORWARDED_HOST_HEADER);
             String originalPort = request.getHeader(X_FORWARDED_PORT_HEADER);
-            if (originalProtocol != null || originalRemoteAddr != null) {
+            
+            // If at least one of the X-Forwarded-Xxx headers is set, try to use them
+            if (Stream.of(originalProtocol, originalRemoteAddr, originalPrefix, 
+                    originalHost, originalPort).anyMatch(Objects::nonNull)) {
                 return new HttpServletRequestXForwardedFilter(request, 
                                                               originalProtocol, 
                                                               originalRemoteAddr,
diff --git a/rt/transports/http/src/test/java/org/apache/cxf/transport/servlet/HttpServletRequestXForwardedFilterTest.java b/rt/transports/http/src/test/java/org/apache/cxf/transport/servlet/HttpServletRequestXForwardedFilterTest.java
new file mode 100644
index 0000000..4561ac1
--- /dev/null
+++ b/rt/transports/http/src/test/java/org/apache/cxf/transport/servlet/HttpServletRequestXForwardedFilterTest.java
@@ -0,0 +1,207 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.cxf.transport.servlet;
+
+import java.io.IOException;
+import java.util.function.BiConsumer;
+
+import javax.servlet.FilterChain;
+import javax.servlet.ServletConfig;
+import javax.servlet.ServletException;
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import org.apache.cxf.Bus;
+import org.apache.cxf.BusFactory;
+import org.junit.Before;
+import org.junit.Test;
+
+import static org.hamcrest.CoreMatchers.equalTo;
+import static org.hamcrest.CoreMatchers.nullValue;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+public class HttpServletRequestXForwardedFilterTest {
+    private AbstractHTTPServlet servlet;
+    private HttpServletRequest request;
+    private HttpServletResponse response;
+    private ServletConfig config;
+    
+    @Before
+    public void setUp() throws ServletException {
+        request = mock(HttpServletRequest.class);
+        response = mock(HttpServletResponse.class);
+        
+        when(request.getRequestURI()).thenReturn("/test");
+        when(request.getRequestURL()).thenReturn(new StringBuffer("http://localhost/api/test"));
+        when(request.getContextPath()).thenReturn("/api");
+        when(request.getServletPath()).thenReturn("");
+        
+        config = mock(ServletConfig.class);
+        when(config.getInitParameter(eq("use-x-forwarded-headers"))).thenReturn("true");
+    }
+
+    @Test
+    public void testNoXForwardedHeadersSpecified() throws Exception {
+        servlet = servlet((req, resp) -> {
+            assertThat(req.getRequestURI(), equalTo("/test"));
+            assertThat(req.getContextPath(), equalTo("/api"));
+            assertThat(req.getRequestURL().toString(), equalTo("http://localhost/api/test"));
+            assertThat(req.getRemoteAddr(), nullValue());
+            assertThat(req.isSecure(), equalTo(false));
+        });
+        
+        servlet.init(config);
+        servlet.service(request, response);
+    }
+
+    @Test
+    public void testAllXForwardedHeadersSpecified() throws Exception {
+        servlet = servlet((req, resp) -> {
+            assertThat(req.getRequestURI(), equalTo("/forwarded/test"));
+            assertThat(req.getContextPath(), equalTo("/forwarded"));
+            assertThat(req.getServletPath(), equalTo("/api"));
+            assertThat(req.getRequestURL().toString(), equalTo("https://abc:50000/forwarded/api/test"));
+            assertThat(req.getRemoteAddr(), equalTo("203.0.113.195"));
+            assertThat(req.isSecure(), equalTo(true));
+        });
+        
+        when(request.getHeader(eq("X-Forwarded-Proto"))).thenReturn("https");
+        when(request.getHeader(eq("X-Forwarded-For"))).thenReturn("203.0.113.195, 70.41.3.18, 150.172.238.178");
+        when(request.getHeader(eq("X-Forwarded-Prefix"))).thenReturn("/forwarded");
+        when(request.getHeader(eq("X-Forwarded-Host"))).thenReturn("abc");
+        when(request.getHeader(eq("X-Forwarded-Port"))).thenReturn("50000");
+
+        servlet.init(config);
+        servlet.service(request, response);
+    }
+
+    @Test
+    public void testXForwardedProtoHeaderSpecified() throws Exception {
+        servlet = servlet((req, resp) -> {
+            assertThat(req.getRequestURI(), equalTo("/test"));
+            assertThat(req.getContextPath(), equalTo("/api"));
+            assertThat(req.getServletPath(), equalTo(""));
+            assertThat(req.getRequestURL().toString(), equalTo("https://localhost/api/test"));
+            assertThat(req.getRemoteAddr(), nullValue());
+            assertThat(req.isSecure(), equalTo(true));
+        });
+        
+        when(request.getHeader(eq("X-Forwarded-Proto"))).thenReturn("https");
+        
+        servlet.init(config);
+        servlet.service(request, response);
+    }
+    
+    @Test
+    public void testXForwardedForHeaderSpecified() throws Exception {
+        servlet = servlet((req, resp) -> {
+            assertThat(req.getRequestURI(), equalTo("/test"));
+            assertThat(req.getContextPath(), equalTo("/api"));
+            assertThat(req.getServletPath(), equalTo(""));
+            assertThat(req.getRequestURL().toString(), equalTo("http://localhost/api/test"));
+            assertThat(req.getRemoteAddr(), equalTo("203.0.113.195"));
+            assertThat(req.isSecure(), equalTo(false));
+        });
+        
+        when(request.getHeader(eq("X-Forwarded-For"))).thenReturn("203.0.113.195, 70.41.3.18, 150.172.238.178");
+        
+        servlet.init(config);
+        servlet.service(request, response);
+    }
+
+    @Test
+    public void testXForwardedPrefixHeaderSpecified() throws Exception {
+        servlet = servlet((req, resp) -> {
+            assertThat(req.getRequestURI(), equalTo("/forwarded/test"));
+            assertThat(req.getContextPath(), equalTo("/forwarded"));
+            assertThat(req.getServletPath(), equalTo("/api"));
+            assertThat(req.getRequestURL().toString(), equalTo("http://localhost/forwarded/api/test"));
+            assertThat(req.getRemoteAddr(), nullValue());
+            assertThat(req.isSecure(), equalTo(false));
+        });
+        
+        when(request.getHeader(eq("X-Forwarded-Prefix"))).thenReturn("/forwarded");
+
+        servlet.init(config);
+        servlet.service(request, response);
+    }
+    
+    @Test
+    public void testXForwardedHostHeaderSpecified() throws Exception {
+        servlet = servlet((req, resp) -> {
+            assertThat(req.getRequestURI(), equalTo("/test"));
+            assertThat(req.getContextPath(), equalTo("/api"));
+            assertThat(req.getServletPath(), equalTo(""));
+            assertThat(req.getRequestURL().toString(), equalTo("http://abc/api/test"));
+            assertThat(req.getRemoteAddr(), nullValue());
+            assertThat(req.isSecure(), equalTo(false));
+        });
+        
+        when(request.getHeader(eq("X-Forwarded-Host"))).thenReturn("abc");
+
+        servlet.init(config);
+        servlet.service(request, response);
+    }
+    
+    @Test
+    public void testXForwardedPortHeaderSpecified() throws Exception {
+        servlet = servlet((req, resp) -> {
+            assertThat(req.getRequestURI(), equalTo("/test"));
+            assertThat(req.getContextPath(), equalTo("/api"));
+            assertThat(req.getServletPath(), equalTo(""));
+            assertThat(req.getRequestURL().toString(), equalTo("http://localhost:50000/api/test"));
+            assertThat(req.getRemoteAddr(), nullValue());
+            assertThat(req.isSecure(), equalTo(false));
+        });
+        
+        when(request.getHeader(eq("X-Forwarded-Port"))).thenReturn("50000");
+
+        servlet.init(config);
+        servlet.service(request, response);
+    }
+    
+    private AbstractHTTPServlet servlet(BiConsumer<HttpServletRequest, HttpServletResponse> assertions) {
+        return new AbstractHTTPServlet() {
+            private static final long serialVersionUID = -3870709934037062681L;
+
+            @Override
+            public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) 
+                    throws IOException, ServletException {
+            }
+            
+            @Override
+            protected void invoke(HttpServletRequest request, HttpServletResponse response) 
+                    throws ServletException {
+                assertions.accept(request, response);
+            }
+            
+            @Override
+            protected Bus getBus() {
+                return BusFactory.getDefaultBus();
+            }
+        };
+    }
+    
+}