From 2421268baa1573700500aa37fc95080c1ed6e453 Mon Sep 17 00:00:00 2001 From: Ben Alex Date: Thu, 29 Apr 2004 02:14:20 +0000 Subject: [PATCH] Improve IE 6 bug detection logic. --- .../acegisecurity/util/PortResolverImpl.java | 71 ++++++------ .../util/PortResolverImplTests.java | 104 ++++-------------- 2 files changed, 55 insertions(+), 120 deletions(-) diff --git a/core/src/main/java/org/acegisecurity/util/PortResolverImpl.java b/core/src/main/java/org/acegisecurity/util/PortResolverImpl.java index c886963e03..ce0e6f0bd7 100644 --- a/core/src/main/java/org/acegisecurity/util/PortResolverImpl.java +++ b/core/src/main/java/org/acegisecurity/util/PortResolverImpl.java @@ -25,14 +25,14 @@ import javax.servlet.ServletRequest; * ServletRequest.getServerPort(). * *

- * If either the alwaysHttpPort or alwaysHttpsPort - * properties are set, these ports will be used instead of those - * obtained from the ServletRequest.getServerPort() method. - * Setting these properties will cause the - * ServletRequest.getScheme() method to be used to determine - * whether a request was HTTP or HTTPS, and then return the port defined by - * the always[Scheme]Port property. You can configure zero, one - * or both of these properties. + * This class is capable of handling the IE bug which results in an incorrect + * URL being presented in the header subsequent to a redirect to a different + * scheme and port where the port is not a well-known number (ie 80 or 443). + * Handling involves detecting an incorrect response from + * ServletRequest.getServerPort() for the scheme (eg a HTTP + * request on 8443) and then determining the real server port (eg HTTP request + * is really on 8080). The map of valid ports is obtained from the configured + * {@link PortMapper}. *

* * @author Ben Alex @@ -41,52 +41,45 @@ import javax.servlet.ServletRequest; public class PortResolverImpl implements InitializingBean, PortResolver { //~ Instance fields ======================================================== - private int alwaysHttpPort = 0; - private int alwaysHttpsPort = 0; + private PortMapper portMapper = new PortMapperImpl(); //~ Methods ================================================================ - public void setAlwaysHttpPort(int alwaysHttpPort) { - this.alwaysHttpPort = alwaysHttpPort; + public void setPortMapper(PortMapper portMapper) { + this.portMapper = portMapper; } - public int getAlwaysHttpPort() { - return alwaysHttpPort; - } - - public void setAlwaysHttpsPort(int alwaysHttpsPort) { - this.alwaysHttpsPort = alwaysHttpsPort; - } - - public int getAlwaysHttpsPort() { - return alwaysHttpsPort; + public PortMapper getPortMapper() { + return portMapper; } public int getServerPort(ServletRequest request) { - if ("http".equals(request.getScheme().toLowerCase()) - && (alwaysHttpPort != 0)) { - return alwaysHttpPort; + int result = request.getServerPort(); + + if ("http".equals(request.getScheme().toLowerCase())) { + Integer http = portMapper.lookupHttpPort(new Integer(result)); + + if (http != null) { + // IE 6 bug + result = http.intValue(); + } } - if ("https".equals(request.getScheme().toLowerCase()) - && (alwaysHttpsPort != 0)) { - return alwaysHttpsPort; + if ("https".equals(request.getScheme().toLowerCase())) { + Integer https = portMapper.lookupHttpsPort(new Integer(result)); + + if (https != null) { + // IE 6 bug + result = https.intValue(); + } } - return request.getServerPort(); + return result; } public void afterPropertiesSet() throws Exception { - if ((alwaysHttpPort != 0) - && ((alwaysHttpPort > 65535) || (alwaysHttpPort < 0))) { - throw new IllegalArgumentException( - "alwaysHttpPort must be between 1 and 65535"); - } - - if ((alwaysHttpsPort != 0) - && ((alwaysHttpsPort > 65535) || (alwaysHttpsPort < 0))) { - throw new IllegalArgumentException( - "alwaysHttpsPort must be between 1 and 65535"); + if (portMapper == null) { + throw new IllegalArgumentException("portMapper required"); } } } diff --git a/core/src/test/java/org/acegisecurity/util/PortResolverImplTests.java b/core/src/test/java/org/acegisecurity/util/PortResolverImplTests.java index b789b09429..920ab1ef8f 100644 --- a/core/src/test/java/org/acegisecurity/util/PortResolverImplTests.java +++ b/core/src/test/java/org/acegisecurity/util/PortResolverImplTests.java @@ -47,15 +47,31 @@ public class PortResolverImplTests extends TestCase { junit.textui.TestRunner.run(PortResolverImplTests.class); } + public void testDetectsBuggyIeHttpRequest() throws Exception { + PortResolverImpl pr = new PortResolverImpl(); + pr.afterPropertiesSet(); + + MockHttpServletRequest request = new MockHttpServletRequest("X"); + request.setServerPort(8443); + request.setScheme("HTtP"); // proves case insensitive handling + assertEquals(8080, pr.getServerPort(request)); + } + + public void testDetectsBuggyIeHttpsRequest() throws Exception { + PortResolverImpl pr = new PortResolverImpl(); + pr.afterPropertiesSet(); + + MockHttpServletRequest request = new MockHttpServletRequest("X"); + request.setServerPort(8080); + request.setScheme("HTtPs"); // proves case insensitive handling + assertEquals(8443, pr.getServerPort(request)); + } + public void testGettersSetters() throws Exception { PortResolverImpl pr = new PortResolverImpl(); - assertEquals(0, pr.getAlwaysHttpPort()); - assertEquals(0, pr.getAlwaysHttpsPort()); - - pr.setAlwaysHttpPort(80); - pr.setAlwaysHttpsPort(443); - assertEquals(80, pr.getAlwaysHttpPort()); - assertEquals(443, pr.getAlwaysHttpsPort()); + assertTrue(pr.getPortMapper() != null); + pr.setPortMapper(new PortMapperImpl()); + assertTrue(pr.getPortMapper() != null); } public void testNormalOperation() throws Exception { @@ -67,78 +83,4 @@ public class PortResolverImplTests extends TestCase { request.setServerPort(1021); assertEquals(1021, pr.getServerPort(request)); } - - public void testOverridesHttp() throws Exception { - PortResolverImpl pr = new PortResolverImpl(); - pr.setAlwaysHttpPort(495); - pr.afterPropertiesSet(); - - MockHttpServletRequest request = new MockHttpServletRequest("X"); - request.setServerPort(7676); - request.setScheme("HTtP"); // proves case insensitive handling - assertEquals(495, pr.getServerPort(request)); - - request.setScheme("https"); - assertEquals(7676, pr.getServerPort(request)); - } - - public void testOverridesHttps() throws Exception { - PortResolverImpl pr = new PortResolverImpl(); - pr.setAlwaysHttpsPort(987); - pr.afterPropertiesSet(); - - MockHttpServletRequest request = new MockHttpServletRequest("X"); - request.setServerPort(6949); - request.setScheme("HTtPs"); // proves case insensitive handling - assertEquals(987, pr.getServerPort(request)); - - request.setScheme("http"); - assertEquals(6949, pr.getServerPort(request)); - } - - public void testRejectsOutOfRangeHttp() throws Exception { - PortResolverImpl pr = new PortResolverImpl(); - pr.setAlwaysHttpPort(9999999); - - try { - pr.afterPropertiesSet(); - fail("Should have thrown IllegalArgumentException"); - } catch (IllegalArgumentException expected) { - assertEquals("alwaysHttpPort must be between 1 and 65535", - expected.getMessage()); - } - - pr.setAlwaysHttpPort(-49); - - try { - pr.afterPropertiesSet(); - fail("Should have thrown IllegalArgumentException"); - } catch (IllegalArgumentException expected) { - assertEquals("alwaysHttpPort must be between 1 and 65535", - expected.getMessage()); - } - } - - public void testRejectsOutOfRangeHttps() throws Exception { - PortResolverImpl pr = new PortResolverImpl(); - pr.setAlwaysHttpsPort(9999999); - - try { - pr.afterPropertiesSet(); - fail("Should have thrown IllegalArgumentException"); - } catch (IllegalArgumentException expected) { - assertEquals("alwaysHttpsPort must be between 1 and 65535", - expected.getMessage()); - } - - pr.setAlwaysHttpsPort(-49); - - try { - pr.afterPropertiesSet(); - fail("Should have thrown IllegalArgumentException"); - } catch (IllegalArgumentException expected) { - assertEquals("alwaysHttpsPort must be between 1 and 65535", - expected.getMessage()); - } - } }