Author: markt
Date: Mon Nov 30 16:25:19 2015
New Revision: 1717290
URL: http://svn.apache.org/viewvc?rev=1717290&view=rev
Log:
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=58655
Fix an IllegalStateException when calling HttpServletResponse.sendRedirect()
with the RemoteIpFilter. This was caused by trying to correctly generate the
absolute URI for the redirect. With the fix for 56917, redirects may now be
relative making the sendRedirect() implementation for the RemoteIpFilter much
simpler. This also addresses issues where the redirect may not have behaved as
expected when redirecting from http to https to from https to http.
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/filters/TestRemoteIpFilter.java
tomcat/trunk/webapps/docs/changelog.xml
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=1717290&r1=1717289&r2=1717290&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/filters/LocalStrings.properties
(original)
+++ tomcat/trunk/java/org/apache/catalina/filters/LocalStrings.properties Mon
Nov 30 16:25:19 2015
@@ -47,5 +47,4 @@ expiresFilter.invalidDurationUnit=Invali
httpHeaderSecurityFilter.committed=Unable to add HTTP headers since response
is already committed on entry to the HTTP header security Filter
httpHeaderSecurityFilter.clickjack.invalid=An invalid value [{0}] was
specified for the anti click-jacking header
-remoteIpFilter.invalidLocation=Failed to modify the rewrite location [{0}] to
use scheme [{1}] and port [{2}]
restCsrfPreventionFilter.invalidNonce=CSRF nonce validation failed
\ 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=1717290&r1=1717289&r2=1717290&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/filters/RemoteIpFilter.java (original)
+++ tomcat/trunk/java/org/apache/catalina/filters/RemoteIpFilter.java Mon Nov
30 16:25:19 2015
@@ -17,8 +17,6 @@
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;
@@ -41,7 +39,6 @@ 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 javax.servlet.http.PushBuilder;
import org.apache.catalina.AccessLog;
@@ -49,7 +46,6 @@ import org.apache.catalina.Globals;
import org.apache.catalina.core.ApplicationPushBuilder;
import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
-import org.apache.tomcat.util.res.StringManager;
/**
* <p>
@@ -651,48 +647,6 @@ public class RemoteIpFilter extends Gene
}
}
- 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
@@ -709,7 +663,6 @@ public class RemoteIpFilter extends Gene
* 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";
@@ -883,15 +836,6 @@ public class RemoteIpFilter extends Gene
}
}
- HttpServletResponse xResponse;
- if (xRequest.getScheme() != null &&
- (!xRequest.getScheme().equals(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()
@@ -914,7 +858,7 @@ public class RemoteIpFilter extends Gene
request.setAttribute(AccessLog.SERVER_PORT_ATTRIBUTE,
Integer.valueOf(xRequest.getServerPort()));
}
- chain.doFilter(xRequest, xResponse);
+ chain.doFilter(xRequest, response);
} else {
if (log.isDebugEnabled()) {
log.debug("Skip RemoteIpFilter for request " +
request.getRequestURI() + " with originalRemoteAddr '"
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=1717290&r1=1717289&r2=1717290&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/filters/TestRemoteIpFilter.java
(original)
+++ tomcat/trunk/test/org/apache/catalina/filters/TestRemoteIpFilter.java Mon
Nov 30 16:25:19 2015
@@ -615,97 +615,4 @@ public class TestRemoteIpFilter extends
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 + "/"));
- }
- }
}
Modified: tomcat/trunk/webapps/docs/changelog.xml
URL:
http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1717290&r1=1717289&r2=1717290&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Mon Nov 30 16:25:19 2015
@@ -98,6 +98,17 @@
Fixed potential NPE in <code>HostConfig</code> while deploying an
application. Issue reported by coverity scan. (violetagg)
</fix>
+ <fix>
+ <bug>58655</bug>: Fix an <code> IllegalStateException</code> when
+ calling <code>HttpServletResponse.sendRedirect()</code> with the
+ <code>RemoteIpFilter</code>. This was caused by trying to correctly
+ generate the absolute URI for the redirect. With the fix for
+ <bug>56917</bug>, redirects may now be relative making the
+ <code>sendRedirect()</code> implementation for the
+ <code>RemoteIpFilter</code> much simpler. This also addresses issues
+ where the redirect may not have behaved as expected when redirecting
+ from http to https to from https to http. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
