SEC-1500: Convert AbstractRetryEntryPoint to use requestURI to correctly encode URLs.

web/src/main/java/org/springframework/security/web/access/channel/AbstractRetryEntryPoint.java

@@ -40,13 +40,8 @@ public abstract class AbstractRetryEntryPoint implements ChannelEntryPoint {
     //~ Methods ========================================================================================================
 
     public void commence(HttpServletRequest request, HttpServletResponse res) throws IOException, ServletException {
-        String pathInfo = request.getPathInfo();
         String queryString = request.getQueryString();
-        String contextPath = request.getContextPath();
-        String destination = request.getServletPath() + ((pathInfo == null) ? "" : pathInfo)
-            + ((queryString == null) ? "" : ("?" + queryString));
-
-        String redirectUrl = contextPath;
+        String redirectUrl = request.getRequestURI() + ((queryString == null) ? "" : ("?" + queryString));
 
         Integer currentPort = new Integer(portResolver.getServerPort(request));
         Integer redirectPort = getMappedPort(currentPort);
@@ -54,8 +49,7 @@ public abstract class AbstractRetryEntryPoint implements ChannelEntryPoint {
         if (redirectPort != null) {
             boolean includePort = redirectPort.intValue() != standardPort;
 
-            redirectUrl = scheme + request.getServerName() + ((includePort) ? (":" + redirectPort) : "") + contextPath
-                + destination;
+            redirectUrl = scheme + request.getServerName() + ((includePort) ? (":" + redirectPort) : "") + redirectUrl;
         }
 
         if (logger.isDebugEnabled()) {
@@ -67,11 +61,11 @@ public abstract class AbstractRetryEntryPoint implements ChannelEntryPoint {
 
     protected abstract Integer getMappedPort(Integer mapFromPort);
 
-    protected PortMapper getPortMapper() {
+    protected final PortMapper getPortMapper() {
         return portMapper;
     }
 
-    protected PortResolver getPortResolver() {
+    protected final PortResolver getPortResolver() {
         return portResolver;
     }
 

web/src/main/java/org/springframework/security/web/util/UrlUtils.java

@@ -19,8 +19,9 @@ import javax.servlet.http.HttpServletRequest;
 
 
 /**
- * Provides static methods for composing URLs.<p>Placed into a separate class for visibility, so that changes to
- * URL formatting conventions will affect all users.</p>
+ * Provides static methods for composing URLs.
+ * <p>
+ * Placed into a separate class for visibility, so that changes to URL formatting conventions will affect all users.
  *
  * @author Ben Alex
  */

web/src/test/java/org/springframework/security/web/access/channel/RetryWithHttpEntryPointTests.java

@@ -66,13 +66,10 @@ public class RetryWithHttpEntryPointTests extends TestCase {
     }
 
     public void testNormalOperation() throws Exception {
-        MockHttpServletRequest request = new MockHttpServletRequest();
+        MockHttpServletRequest request = new MockHttpServletRequest("GET", "/bigWebApp/hello/pathInfo.html");
         request.setQueryString("open=true");
         request.setScheme("https");
         request.setServerName("www.example.com");
-        request.setContextPath("/bigWebApp");
-        request.setServletPath("/hello");
-        request.setPathInfo("/pathInfo.html");
         request.setServerPort(443);
 
         MockHttpServletResponse response = new MockHttpServletResponse();
@@ -85,14 +82,10 @@ public class RetryWithHttpEntryPointTests extends TestCase {
         assertEquals("http://www.example.com/bigWebApp/hello/pathInfo.html?open=true", response.getRedirectedUrl());
     }
 
-    public void testNormalOperationWithNullPathInfoAndNullQueryString()
-        throws Exception {
-        MockHttpServletRequest request = new MockHttpServletRequest();
+    public void testNormalOperationWithNullQueryString() throws Exception {
+        MockHttpServletRequest request = new MockHttpServletRequest("GET", "/bigWebApp/hello");
         request.setScheme("https");
         request.setServerName("www.example.com");
-        request.setContextPath("/bigWebApp");
-        request.setServletPath("/hello");
-        request.setPathInfo(null);
         request.setServerPort(443);
 
         MockHttpServletResponse response = new MockHttpServletResponse();
@@ -105,15 +98,11 @@ public class RetryWithHttpEntryPointTests extends TestCase {
         assertEquals("http://www.example.com/bigWebApp/hello", response.getRedirectedUrl());
     }
 
-    public void testOperationWhenTargetPortIsUnknown()
-        throws Exception {
-        MockHttpServletRequest request = new MockHttpServletRequest();
+    public void testOperationWhenTargetPortIsUnknown() throws Exception {
+        MockHttpServletRequest request = new MockHttpServletRequest("GET", "/bigWebApp");
         request.setQueryString("open=true");
         request.setScheme("https");
         request.setServerName("www.example.com");
-        request.setContextPath("/bigWebApp");
-        request.setServletPath("/hello");
-        request.setPathInfo("/pathInfo.html");
         request.setServerPort(8768);
 
         MockHttpServletResponse response = new MockHttpServletResponse();
@@ -123,17 +112,14 @@ public class RetryWithHttpEntryPointTests extends TestCase {
         ep.setPortResolver(new MockPortResolver(8768, 1234));
 
         ep.commence(request, response);
-        assertEquals("/bigWebApp", response.getRedirectedUrl());
+        assertEquals("/bigWebApp?open=true", response.getRedirectedUrl());
     }
 
     public void testOperationWithNonStandardPort() throws Exception {
-        MockHttpServletRequest request = new MockHttpServletRequest();
+        MockHttpServletRequest request = new MockHttpServletRequest("GET", "/bigWebApp/hello/pathInfo.html");
         request.setQueryString("open=true");
         request.setScheme("https");
         request.setServerName("www.example.com");
-        request.setContextPath("/bigWebApp");
-        request.setServletPath("/hello");
-        request.setPathInfo("/pathInfo.html");
         request.setServerPort(9999);
 
         MockHttpServletResponse response = new MockHttpServletResponse();

web/src/test/java/org/springframework/security/web/access/channel/RetryWithHttpsEntryPointTests.java

@@ -37,10 +37,6 @@ import java.util.Map;
 public class RetryWithHttpsEntryPointTests extends TestCase {
     //~ Methods ========================================================================================================
 
-    public static void main(String[] args) {
-        junit.textui.TestRunner.run(RetryWithHttpsEntryPointTests.class);
-    }
-
     public final void setUp() throws Exception {
         super.setUp();
     }
@@ -74,13 +70,10 @@ public class RetryWithHttpsEntryPointTests extends TestCase {
     }
 
     public void testNormalOperation() throws Exception {
-        MockHttpServletRequest request = new MockHttpServletRequest();
+        MockHttpServletRequest request = new MockHttpServletRequest("GET", "/bigWebApp/hello/pathInfo.html");
         request.setQueryString("open=true");
         request.setScheme("http");
         request.setServerName("www.example.com");
-        request.setContextPath("/bigWebApp");
-        request.setServletPath("/hello");
-        request.setPathInfo("/pathInfo.html");
         request.setServerPort(80);
 
         MockHttpServletResponse response = new MockHttpServletResponse();
@@ -93,14 +86,10 @@ public class RetryWithHttpsEntryPointTests extends TestCase {
         assertEquals("https://www.example.com/bigWebApp/hello/pathInfo.html?open=true", response.getRedirectedUrl());
     }
 
-    public void testNormalOperationWithNullPathInfoAndNullQueryString()
-        throws Exception {
-        MockHttpServletRequest request = new MockHttpServletRequest();
+    public void testNormalOperationWithNullQueryString() throws Exception {
+        MockHttpServletRequest request = new MockHttpServletRequest("GET", "/bigWebApp/hello");
         request.setScheme("http");
         request.setServerName("www.example.com");
-        request.setContextPath("/bigWebApp");
-        request.setServletPath("/hello");
-        request.setPathInfo(null);
         request.setServerPort(80);
 
         MockHttpServletResponse response = new MockHttpServletResponse();
@@ -114,13 +103,10 @@ public class RetryWithHttpsEntryPointTests extends TestCase {
     }
 
     public void testOperationWhenTargetPortIsUnknown() throws Exception {
-        MockHttpServletRequest request = new MockHttpServletRequest();
+        MockHttpServletRequest request = new MockHttpServletRequest("GET", "/bigWebApp");
         request.setQueryString("open=true");
         request.setScheme("http");
         request.setServerName("www.example.com");
-        request.setContextPath("/bigWebApp");
-        request.setServletPath("/hello");
-        request.setPathInfo("/pathInfo.html");
         request.setServerPort(8768);
 
         MockHttpServletResponse response = new MockHttpServletResponse();
@@ -130,17 +116,14 @@ public class RetryWithHttpsEntryPointTests extends TestCase {
         ep.setPortResolver(new MockPortResolver(8768, 1234));
 
         ep.commence(request, response);
-        assertEquals("/bigWebApp", response.getRedirectedUrl());
+        assertEquals("/bigWebApp?open=true", response.getRedirectedUrl());
     }
 
     public void testOperationWithNonStandardPort() throws Exception {
-        MockHttpServletRequest request = new MockHttpServletRequest();
+        MockHttpServletRequest request = new MockHttpServletRequest("GET", "/bigWebApp/hello/pathInfo.html");
         request.setQueryString("open=true");
         request.setScheme("http");
         request.setServerName("www.example.com");
-        request.setContextPath("/bigWebApp");
-        request.setServletPath("/hello");
-        request.setPathInfo("/pathInfo.html");
         request.setServerPort(8888);
 
         MockHttpServletResponse response = new MockHttpServletResponse();