Improve IE 6 bug detection logic.

core/src/main/java/org/acegisecurity/util/PortResolverImpl.java

@@ -25,14 +25,14 @@ import javax.servlet.ServletRequest;
  * <code>ServletRequest.getServerPort()</code>.
  * 
  * <P>
- * If either the <code>alwaysHttpPort</code> or <code>alwaysHttpsPort</code>
- * properties are set, these ports will be used <B>instead of</B> those
- * obtained from the <code>ServletRequest.getServerPort()</code> method.
- * Setting these properties will cause the
- * <code>ServletRequest.getScheme()</code> method to be used to determine
- * whether a request was HTTP or HTTPS, and then return the port defined by
- * the <code>always[Scheme]Port</code> 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
+ * <code>ServletRequest.getServerPort()</code> 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}.
  * </p>
  *
  * @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 PortMapper getPortMapper() {
+        return portMapper;
     }
 
-    public void setAlwaysHttpsPort(int alwaysHttpsPort) {
-        this.alwaysHttpsPort = alwaysHttpsPort;
-    }
+    public int getServerPort(ServletRequest request) {
+        int result = request.getServerPort();
 
-    public int getAlwaysHttpsPort() {
-        return alwaysHttpsPort;
-    }
+        if ("http".equals(request.getScheme().toLowerCase())) {
+            Integer http = portMapper.lookupHttpPort(new Integer(result));
 
-    public int getServerPort(ServletRequest request) {
-        if ("http".equals(request.getScheme().toLowerCase())
-            && (alwaysHttpPort != 0)) {
-            return alwaysHttpPort;
+            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");
         }
     }
 }

core/src/test/java/org/acegisecurity/util/PortResolverImplTests.java

@@ -47,98 +47,40 @@ public class PortResolverImplTests extends TestCase {
         junit.textui.TestRunner.run(PortResolverImplTests.class);
     }
 
-    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());
-    }
-
-    public void testNormalOperation() throws Exception {
-        PortResolverImpl pr = new PortResolverImpl();
-        pr.afterPropertiesSet();
-
-        MockHttpServletRequest request = new MockHttpServletRequest("X");
-        request.setScheme("http");
-        request.setServerPort(1021);
-        assertEquals(1021, pr.getServerPort(request));
-    }
-
-    public void testOverridesHttp() throws Exception {
+    public void testDetectsBuggyIeHttpRequest() throws Exception {
         PortResolverImpl pr = new PortResolverImpl();
-        pr.setAlwaysHttpPort(495);
         pr.afterPropertiesSet();
 
         MockHttpServletRequest request = new MockHttpServletRequest("X");
-        request.setServerPort(7676);
+        request.setServerPort(8443);
         request.setScheme("HTtP"); // proves case insensitive handling
-        assertEquals(495, pr.getServerPort(request));
-
-        request.setScheme("https");
-        assertEquals(7676, pr.getServerPort(request));
+        assertEquals(8080, pr.getServerPort(request));
     }
 
-    public void testOverridesHttps() throws Exception {
+    public void testDetectsBuggyIeHttpsRequest() throws Exception {
         PortResolverImpl pr = new PortResolverImpl();
-        pr.setAlwaysHttpsPort(987);
         pr.afterPropertiesSet();
 
         MockHttpServletRequest request = new MockHttpServletRequest("X");
-        request.setServerPort(6949);
+        request.setServerPort(8080);
         request.setScheme("HTtPs"); // proves case insensitive handling
-        assertEquals(987, pr.getServerPort(request));
-
-        request.setScheme("http");
-        assertEquals(6949, pr.getServerPort(request));
+        assertEquals(8443, pr.getServerPort(request));
     }
 
-    public void testRejectsOutOfRangeHttp() throws Exception {
+    public void testGettersSetters() 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());
-        }
+        assertTrue(pr.getPortMapper() != null);
+        pr.setPortMapper(new PortMapperImpl());
+        assertTrue(pr.getPortMapper() != null);
     }
 
-    public void testRejectsOutOfRangeHttps() throws Exception {
+    public void testNormalOperation() 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());
-        }
+        pr.afterPropertiesSet();
+
+        MockHttpServletRequest request = new MockHttpServletRequest("X");
+        request.setScheme("http");
+        request.setServerPort(1021);
+        assertEquals(1021, pr.getServerPort(request));
     }
 }