Author: markt
Date: Mon Apr 27 20:13:05 2015
New Revision: 1676364
URL: http://svn.apache.org/r1676364
Log:
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=57856
Ensure that any scheme/port changes implemented by the RemoteIpFilter also
affect HttpServletResponse.sendRedirect()
Modified:
tomcat/trunk/java/org/apache/catalina/filters/LocalStrings.properties
tomcat/trunk/java/org/apache/catalina/filters/RemoteIpFilter.java
tomcat/trunk/test/org/apache/catalina/connector/TesterResponse.java
tomcat/trunk/test/org/apache/catalina/filters/TestRemoteIpFilter.java
Modified: tomcat/trunk/java/org/apache/catalina/filters/LocalStrings.properties
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/filters/LocalStrings.properties?rev=1676364&r1=1676363&r2=1676364&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/filters/LocalStrings.properties
(original)
+++ tomcat/trunk/java/org/apache/catalina/filters/LocalStrings.properties Mon
Apr 27 20:13:05 2015
@@ -40,3 +40,4 @@ expiresFilter.filterInitialized=Filter i
expiresFilter.expirationHeaderAlreadyDefined=Request "{0}" with response
status "{1}" content-type "{2}", expiration header already defined
expiresFilter.skippedStatusCode=Request "{0}" with response status "{1}"
content-type "{1}", skip expiration header generation for given status
+remoteIpFilter.invalidLocation=Failed to modify the rewrite location [{0}] to
use scheme [{1}] and port [{2}]
\ No newline at end of file
Modified: tomcat/trunk/java/org/apache/catalina/filters/RemoteIpFilter.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/filters/RemoteIpFilter.java?rev=1676364&r1=1676363&r2=1676364&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/filters/RemoteIpFilter.java (original)
+++ tomcat/trunk/java/org/apache/catalina/filters/RemoteIpFilter.java Mon Apr
27 20:13:05 2015
@@ -17,6 +17,8 @@
package org.apache.catalina.filters;
import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
import java.text.DateFormat;
import java.text.SimpleDateFormat;
import java.util.Arrays;
@@ -40,11 +42,13 @@ import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletRequestWrapper;
import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.HttpServletResponseWrapper;
import org.apache.catalina.AccessLog;
import org.apache.catalina.Globals;
import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
+import org.apache.tomcat.util.res.StringManager;
/**
* <p>
@@ -617,6 +621,49 @@ public class RemoteIpFilter implements F
}
}
+ public static class XForwardedResponse extends HttpServletResponseWrapper {
+
+ private final String scheme;
+ private final int port;
+
+ public XForwardedResponse(HttpServletResponse response, String scheme,
int port) {
+ super(response);
+ this.scheme = scheme;
+ if ("http".equals(scheme) && port == 80 || "https".equals(scheme)
&& port == 443) {
+ this.port = -1;
+ } else {
+ this.port = port;
+ }
+ }
+
+ @Override
+ public void sendRedirect(String location) throws IOException {
+ /*
+ * This isn't particularly pretty but, given that:
+ * a) there is no setRequest() method on ServletResponse (even if
+ * there were the response could still access this information
+ * via some internal structure for speed); and
+ * b) that this is meant to work on any Servlet container;
+ * this was the cleanest way I could come up with for doing this.
+ */
+ super.sendRedirect(location);
+ String redirect = getHeader("location");
+ URI newRedirectURI;
+ try {
+ URI redirectURI = new URI(redirect);
+ newRedirectURI = new URI(scheme, redirectURI.getUserInfo(),
+ redirectURI.getHost(), port, redirectURI.getPath(),
+ redirectURI.getQuery(), redirectURI.getFragment());
+ } catch (URISyntaxException e) {
+ log.warn(sm.getString("remoteIpFilter.invalidLocation",
+ location, scheme, Integer.toString(port)));
+ return;
+ }
+ reset();
+ super.sendRedirect(newRedirectURI.toString());
+ }
+ }
+
/**
* {@link Pattern} for a comma delimited string that support whitespace
characters
*/
@@ -632,6 +679,7 @@ public class RemoteIpFilter implements F
* Logger
*/
private static final Log log = LogFactory.getLog(RemoteIpFilter.class);
+ private static final StringManager sm =
StringManager.getManager(RemoteIpFilter.class);
protected static final String PROTOCOL_HEADER_PARAMETER = "protocolHeader";
@@ -810,6 +858,14 @@ public class RemoteIpFilter implements F
}
}
+ HttpServletResponse xResponse;
+ if (xRequest.getScheme() != request.getScheme() ||
+ xRequest.getServerPort() != request.getServerPort()) {
+ xResponse = new XForwardedResponse(response,
xRequest.getScheme(), xRequest.getServerPort());
+ } else {
+ xResponse = response;
+ }
+
if (log.isDebugEnabled()) {
log.debug("Incoming request " + request.getRequestURI() + "
with originalRemoteAddr '" + request.getRemoteAddr()
+ "', originalRemoteHost='" + request.getRemoteHost()
+ "', originalSecure='" + request.isSecure()
@@ -832,7 +888,7 @@ public class RemoteIpFilter implements F
request.setAttribute(AccessLog.SERVER_PORT_ATTRIBUTE,
Integer.valueOf(xRequest.getServerPort()));
}
- chain.doFilter(xRequest, response);
+ chain.doFilter(xRequest, xResponse);
} else {
if (log.isDebugEnabled()) {
log.debug("Skip RemoteIpFilter for request " +
request.getRequestURI() + " with originalRemoteAddr '"
Modified: tomcat/trunk/test/org/apache/catalina/connector/TesterResponse.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/connector/TesterResponse.java?rev=1676364&r1=1676363&r2=1676364&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/connector/TesterResponse.java
(original)
+++ tomcat/trunk/test/org/apache/catalina/connector/TesterResponse.java Mon Apr
27 20:13:05 2015
@@ -60,4 +60,12 @@ public class TesterResponse extends Resp
// There is no buffer created for this test object since no test
depends
// on one being present or on this method suspending it.
}
+
+ @Override
+ public void reset() {
+ // Minimal implementation for tests that avoids using OutputBuffer
+ if (super.getCoyoteResponse() != null) {
+ super.getCoyoteResponse().reset();
+ }
+ }
}
Modified: tomcat/trunk/test/org/apache/catalina/filters/TestRemoteIpFilter.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/filters/TestRemoteIpFilter.java?rev=1676364&r1=1676363&r2=1676364&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/filters/TestRemoteIpFilter.java
(original)
+++ tomcat/trunk/test/org/apache/catalina/filters/TestRemoteIpFilter.java Mon
Apr 27 20:13:05 2015
@@ -49,7 +49,8 @@ import org.apache.catalina.Context;
import org.apache.catalina.LifecycleException;
import org.apache.catalina.connector.Connector;
import org.apache.catalina.connector.Request;
-import org.apache.catalina.connector.Response;
+import org.apache.catalina.connector.TesterResponse;
+import org.apache.catalina.core.TesterContext;
import org.apache.catalina.startup.Tomcat;
import org.apache.catalina.startup.TomcatBaseTest;
import org.apache.tomcat.util.descriptor.web.FilterDef;
@@ -59,19 +60,25 @@ public class TestRemoteIpFilter extends
/**
* Mock {@link FilterChain} to keep a handle on the passed
- * {@link ServletRequest}.
+ * {@link ServletRequest} and (@link ServletResponse}.
*/
public static class MockFilterChain implements FilterChain {
private HttpServletRequest request;
+ private HttpServletResponse response;
@Override
public void doFilter(ServletRequest request, ServletResponse response)
throws IOException, ServletException {
this.request = (HttpServletRequest) request;
+ this.response = (HttpServletResponse) response;
}
public HttpServletRequest getRequest() {
return request;
}
+
+ public HttpServletResponse getResponse() {
+ return response;
+ }
}
public static class MockHttpServlet extends HttpServlet {
@@ -134,6 +141,20 @@ public class TestRemoteIpFilter extends
public Object getAttribute(String name) {
return getCoyoteRequest().getAttributes().get(name);
}
+
+ @Override
+ public String getServerName() {
+ return "localhost";
+ }
+
+ @Override
+ public Context getContext() {
+ // Lazt init
+ if (super.getContext() == null) {
+ getMappingData().context = new TesterContext();
+ }
+ return super.getContext();
+ }
}
public static final String TEMP_DIR = System.getProperty("java.io.tmpdir");
@@ -184,7 +205,7 @@ public class TestRemoteIpFilter extends
request.setHeader("x-forwarded-proto", "http");
// TEST
- HttpServletRequest actualRequest = testRemoteIpFilter(filterDef,
request);
+ HttpServletRequest actualRequest = testRemoteIpFilter(filterDef,
request).getRequest();
// VERIFY
boolean actualSecure = actualRequest.isSecure();
@@ -217,7 +238,7 @@ public class TestRemoteIpFilter extends
request.setHeader("x-forwarded-proto", "http");
// TEST
- HttpServletRequest actualRequest = testRemoteIpFilter(filterDef,
request);
+ HttpServletRequest actualRequest = testRemoteIpFilter(filterDef,
request).getRequest();
// VERIFY
boolean actualSecure = actualRequest.isSecure();
@@ -248,7 +269,7 @@ public class TestRemoteIpFilter extends
request.setRemoteHost("remote-host-original-value");
// TEST
- HttpServletRequest actualRequest = testRemoteIpFilter(filterDef,
request);
+ HttpServletRequest actualRequest = testRemoteIpFilter(filterDef,
request).getRequest();
// VERIFY
String actualXForwardedFor = request.getHeader("x-forwarded-for");
@@ -262,7 +283,6 @@ public class TestRemoteIpFilter extends
String actualRemoteHost = actualRequest.getRemoteHost();
assertEquals("remoteHost", "remote-host-original-value",
actualRemoteHost);
-
}
@Test
@@ -281,7 +301,7 @@ public class TestRemoteIpFilter extends
request.addHeader("x-forwarded-for", "140.211.11.130, 192.168.0.10,
192.168.0.11");
// TEST
- HttpServletRequest actualRequest = testRemoteIpFilter(filterDef,
request);
+ HttpServletRequest actualRequest = testRemoteIpFilter(filterDef,
request).getRequest();
// VERIFY
String actualXForwardedFor =
actualRequest.getHeader("x-forwarded-for");
@@ -316,7 +336,7 @@ public class TestRemoteIpFilter extends
request.setHeader("x-forwarded-for", "140.211.11.130, proxy1, proxy2");
// TEST
- HttpServletRequest actualRequest = testRemoteIpFilter(filterDef,
request);
+ HttpServletRequest actualRequest = testRemoteIpFilter(filterDef,
request).getRequest();
// VERIFY
String actualXForwardedFor =
actualRequest.getHeader("x-forwarded-for");
@@ -350,7 +370,7 @@ public class TestRemoteIpFilter extends
request.addHeader("x-forwarded-for", "proxy2");
// TEST
- HttpServletRequest actualRequest = testRemoteIpFilter(filterDef,
request);
+ HttpServletRequest actualRequest = testRemoteIpFilter(filterDef,
request).getRequest();
// VERIFY
String actualXForwardedFor =
actualRequest.getHeader("x-forwarded-for");
@@ -383,7 +403,7 @@ public class TestRemoteIpFilter extends
request.setHeader("x-forwarded-for", "140.211.11.130, proxy1, proxy2,
192.168.0.10, 192.168.0.11");
// TEST
- HttpServletRequest actualRequest = testRemoteIpFilter(filterDef,
request);
+ HttpServletRequest actualRequest = testRemoteIpFilter(filterDef,
request).getRequest();
// VERIFY
String actualXForwardedFor =
actualRequest.getHeader("x-forwarded-for");
@@ -415,7 +435,7 @@ public class TestRemoteIpFilter extends
request.setHeader("x-forwarded-for", "140.211.11.130, proxy1, proxy2");
// TEST
- HttpServletRequest actualRequest = testRemoteIpFilter(filterDef,
request);
+ HttpServletRequest actualRequest = testRemoteIpFilter(filterDef,
request).getRequest();
// VERIFY
String actualXForwardedFor =
actualRequest.getHeader("x-forwarded-for");
@@ -447,7 +467,7 @@ public class TestRemoteIpFilter extends
request.setHeader("x-forwarded-for", "140.211.11.130, proxy1,
untrusted-proxy, proxy2");
// TEST
- HttpServletRequest actualRequest = testRemoteIpFilter(filterDef,
request);
+ HttpServletRequest actualRequest = testRemoteIpFilter(filterDef,
request).getRequest();
// VERIFY
String actualXForwardedFor =
actualRequest.getHeader("x-forwarded-for");
@@ -483,8 +503,8 @@ public class TestRemoteIpFilter extends
}
}
- private HttpServletRequest testRemoteIpFilter(FilterDef filterDef, Request
request) throws LifecycleException, IOException,
- ServletException {
+ private MockFilterChain testRemoteIpFilter(FilterDef filterDef, Request
request)
+ throws LifecycleException, IOException, ServletException {
Tomcat tomcat = getTomcatInstance();
Context root = tomcat.addContext("", TEMP_DIR);
@@ -504,8 +524,10 @@ public class TestRemoteIpFilter extends
MockFilterChain filterChain = new MockFilterChain();
// TEST
- remoteIpFilter.doFilter(request, new Response(), filterChain);
- return filterChain.getRequest();
+ TesterResponse response = new TesterResponse();
+ response.setRequest(request);
+ remoteIpFilter.doFilter(request, response, filterChain);
+ return filterChain;
}
@Test
@@ -522,8 +544,7 @@ public class TestRemoteIpFilter extends
request.setHeader("x-forwarded-proto", "http");
// TEST
- HttpServletRequest actualRequest =
- testRemoteIpFilter(filterDef, request);
+ HttpServletRequest actualRequest = testRemoteIpFilter(filterDef,
request).getRequest();
// VERIFY
Assert.assertEquals("org.apache.catalina.AccessLog.ServerPort",
@@ -581,7 +602,6 @@ public class TestRemoteIpFilter extends
httpURLConnection.addRequestProperty("x-forwarded-proto", "https");
// VALIDATE
-
Assert.assertEquals(HttpURLConnection.HTTP_OK,
httpURLConnection.getResponseCode());
HttpServletRequest request = mockServlet.getRequest();
Assert.assertNotNull(request);
@@ -594,6 +614,98 @@ public class TestRemoteIpFilter extends
Assert.assertTrue(request.isSecure());
Assert.assertEquals("https", request.getScheme());
Assert.assertEquals(443, request.getServerPort());
+ }
+
+
+ @Test
+ public void testSendRedirectWithXForwardedProto01() throws Exception {
+ doTestSendRedirectWithXForwardedProtol(false, false, 8080, 8080);
+ }
+
+
+ @Test
+ public void testSendRedirectWithXForwardedProto02() throws Exception {
+ doTestSendRedirectWithXForwardedProtol(false, true, 8080, 8080);
+ }
+
+
+ @Test
+ public void testSendRedirectWithXForwardedProto03() throws Exception {
+ doTestSendRedirectWithXForwardedProtol(true, false, 8080, 8080);
+ }
+
+
+ @Test
+ public void testSendRedirectWithXForwardedProto04() throws Exception {
+ doTestSendRedirectWithXForwardedProtol(true, true, 8080, 8080);
+ }
+
+
+ @Test
+ public void testSendRedirectWithXForwardedProto05() throws Exception {
+ doTestSendRedirectWithXForwardedProtol(false, false, 8080, 80);
+ }
+
+ @Test
+ public void testSendRedirectWithXForwardedProto06() throws Exception {
+ doTestSendRedirectWithXForwardedProtol(false, false, 80, 8080);
+ }
+
+
+ @Test
+ public void testSendRedirectWithXForwardedProto07() throws Exception {
+ doTestSendRedirectWithXForwardedProtol(false, true, 8443, 443);
+ }
+
+
+ @Test
+ public void testSendRedirectWithXForwardedProto08() throws Exception {
+ doTestSendRedirectWithXForwardedProtol(false, true, 443, 8443);
+ }
+
+
+ private void doTestSendRedirectWithXForwardedProtol(boolean
incomingIsHttps,
+ boolean headerIsHttps, int incomingPort, int headerPort) throws
Exception {
+
+ // PREPARE
+ FilterDef filterDef = new FilterDef();
+ filterDef.addInitParameter("protocolHeader", "x-forwarded-proto");
+ filterDef.addInitParameter("remoteIpHeader", "x-my-forwarded-for");
+ if (headerIsHttps) {
+ filterDef.addInitParameter("httpsServerPort",
Integer.toString(headerPort));
+ } else {
+ filterDef.addInitParameter("httpServerPort",
Integer.toString(headerPort));
+ }
+
+ MockHttpServletRequest request = new MockHttpServletRequest();
+ request.setRemoteAddr("192.168.0.10");
+ request.setServerPort(incomingPort);
+ request.setSecure(true);
+ if (incomingIsHttps) {
+ request.setScheme("https");
+ } else {
+ request.setScheme("http");
+ }
+ request.setHeader("x-my-forwarded-for", "140.211.11.130");
+ if (headerIsHttps) {
+ request.setHeader("x-forwarded-proto", "https");
+ } else {
+ request.setHeader("x-forwarded-proto", "http");
+ }
+
+ // TEST
+ HttpServletResponse actualResponse = testRemoteIpFilter(filterDef,
request).getResponse();
+ actualResponse.sendRedirect("/");
+ String location = actualResponse.getHeader("Location");
+
+ // VERIFY
+ Assert.assertEquals(location,
+ Boolean.valueOf(location.startsWith("https")),
Boolean.valueOf(headerIsHttps));
+ if (headerPort == 80 && !headerIsHttps || headerPort == 443 &&
headerIsHttps) {
+ Assert.assertTrue(location, location.contains("://localhost/"));
+ } else {
+ Assert.assertTrue(location, location.contains("://localhost:" +
headerPort + "/"));
+ }
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
