diff --git a/config/src/test/java/org/springframework/security/config/http/HttpSecurityBeanDefinitionParserTests.java b/config/src/test/java/org/springframework/security/config/http/HttpSecurityBeanDefinitionParserTests.java
index 1f9f49f6fa..2c48ce587e 100644
--- a/config/src/test/java/org/springframework/security/config/http/HttpSecurityBeanDefinitionParserTests.java
+++ b/config/src/test/java/org/springframework/security/config/http/HttpSecurityBeanDefinitionParserTests.java
@@ -1056,14 +1056,5 @@ public class HttpSecurityBeanDefinitionParserTests {
return ((RememberMeProcessingFilter)getFilter(RememberMeProcessingFilter.class)).getRememberMeServices();
}
-// @SuppressWarnings("unchecked")
-// private ConcurrentSessionController getConcurrentSessionController() {
-// Map beans = appContext.getBeansOfType(ConcurrentSessionController.class);
-//
-// if (beans.size() == 0) {
-// return null;
-// }
-// return (ConcurrentSessionController) new ArrayList(beans.values()).get(0);
-// }
}
diff --git a/itest/web/src/main/webapp/WEB-INF/custom-filters.xml b/itest/web/src/main/webapp/WEB-INF/custom-filters.xml
deleted file mode 100644
index bc55fa037c..0000000000
--- a/itest/web/src/main/webapp/WEB-INF/custom-filters.xml
+++ /dev/null
@@ -1,19 +0,0 @@
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
diff --git a/itest/web/src/main/webapp/secure/file?with?special?chars.html b/itest/web/src/main/webapp/secure/file?with?special?chars.html
new file mode 100644
index 0000000000..68ac0a0302
--- /dev/null
+++ b/itest/web/src/main/webapp/secure/file?with?special?chars.html
@@ -0,0 +1,12 @@
+
+
+
+
+ Special Chars File
+
+
+ I'm file?with?special?chars.html
+
+
diff --git a/itest/web/src/test/java/org/springframework/security/integration/InMemoryProviderWebAppTests.java b/itest/web/src/test/java/org/springframework/security/integration/InMemoryProviderWebAppTests.java
index 7bc9b7ce8c..d208805940 100644
--- a/itest/web/src/test/java/org/springframework/security/integration/InMemoryProviderWebAppTests.java
+++ b/itest/web/src/test/java/org/springframework/security/integration/InMemoryProviderWebAppTests.java
@@ -1,6 +1,6 @@
package org.springframework.security.integration;
-import org.testng.annotations.*;
+import org.testng.annotations.Test;
/**
* @author Luke Taylor
@@ -39,4 +39,12 @@ public class InMemoryProviderWebAppTests extends AbstractWebServerIntegrationTes
assertTextPresent("xcount=2");
}
+ // SEC-1255
+ @Test
+ public void redirectToUrlWithSpecialCharsInFilenameWorksOk() throws Exception {
+ beginAt("secure/file%3Fwith%3Fspecial%3Fchars.html?someArg=1");
+ login("jimi", "jimispassword");
+ assertTextPresent("I'm file?with?special?chars.html");
+ }
+
}
diff --git a/web/src/main/java/org/springframework/security/web/savedrequest/DefaultSavedRequest.java b/web/src/main/java/org/springframework/security/web/savedrequest/DefaultSavedRequest.java
index 36b4af4e09..e7c0b58054 100644
--- a/web/src/main/java/org/springframework/security/web/savedrequest/DefaultSavedRequest.java
+++ b/web/src/main/java/org/springframework/security/web/savedrequest/DefaultSavedRequest.java
@@ -234,8 +234,7 @@ public class DefaultSavedRequest implements SavedRequest {
* @return the full URL of this request
*/
public String getRedirectUrl() {
- return UrlUtils.buildFullRequestUrl(scheme, serverName, serverPort, contextPath, servletPath, requestURI,
- pathInfo, queryString);
+ return UrlUtils.buildFullRequestUrl(scheme, serverName, serverPort, requestURI, queryString);
}
public Collection getHeaderNames() {
diff --git a/web/src/main/java/org/springframework/security/web/util/UrlUtils.java b/web/src/main/java/org/springframework/security/web/util/UrlUtils.java
index a4ae7427a9..e2c48ef851 100644
--- a/web/src/main/java/org/springframework/security/web/util/UrlUtils.java
+++ b/web/src/main/java/org/springframework/security/web/util/UrlUtils.java
@@ -29,8 +29,8 @@ public final class UrlUtils {
//~ Methods ========================================================================================================
public static String buildFullRequestUrl(HttpServletRequest r) {
- return buildFullRequestUrl(r.getScheme(), r.getServerName(), r.getServerPort(), r.getContextPath(),
- r.getServletPath(), r.getRequestURI(), r.getPathInfo(), r.getQueryString());
+ return buildFullRequestUrl(r.getScheme(), r.getServerName(), r.getServerPort(), r.getRequestURI(),
+ r.getQueryString());
}
/**
@@ -39,29 +39,53 @@ public final class UrlUtils {
* Note that the server port will not be shown if it is the default server port for HTTP or HTTPS
* (80 and 443 respectively).
*
- * @return the full URL
+ * @return the full URL, suitable for redirects (not decoded).
*/
- public static String buildFullRequestUrl(String scheme, String serverName, int serverPort, String contextPath,
- String servletPath, String requestURI, String pathInfo, String queryString) {
+ public static String buildFullRequestUrl(String scheme, String serverName, int serverPort, String requestURI,
+ String queryString) {
- boolean includePort = true;
+ scheme = scheme.toLowerCase();
- if ("http".equals(scheme.toLowerCase()) && (serverPort == 80)) {
- includePort = false;
+ StringBuilder url = new StringBuilder();
+ url.append(scheme).append("://").append(serverName);
+
+ // Only add port if not default
+ if ("http".equals(scheme)) {
+ if (serverPort != 80) {
+ url.append(":").append(serverPort);
+ }
+ } else if ("https".equals(scheme)) {
+ if (serverPort != 443) {
+ url.append(":").append(serverPort);
+ }
}
- if ("https".equals(scheme.toLowerCase()) && (serverPort == 443)) {
- includePort = false;
+ // Use the requestURI as it is encoded (RFC 3986) and hence suitable for redirects.
+ url.append(requestURI);
+
+ if (queryString != null) {
+ url.append("?").append(queryString);
}
- return scheme + "://" + serverName + ((includePort) ? (":" + serverPort) : "") + contextPath
- + buildRequestUrl(servletPath, requestURI, contextPath, pathInfo, queryString);
+ return url.toString();
}
/**
* Obtains the web application-specific fragment of the request URL.
+ *
+ * Under normal spec conditions,
+ *
+ * requestURI = contextPath + servletPath + pathInfo
+ *
+ *
+ * But the requestURI is not decoded, whereas the servletPath and pathInfo are (SEC-1255).
+ * This method is typically used to return a URL for matching against secured paths, hence the decoded form is
+ * used in preference to the requestURI for building the returned value. But this method may also be called using
+ * dummy request objects which just have the requestURI and contextPatth set, for example, so it will fall back to
+ * using those.
+ *
+ * @return the decoded URL, excluding any server name, context path or servlet path
*
- * @return the URL, excluding any server name, context path or servlet path
*/
public static String buildRequestUrl(HttpServletRequest r) {
return buildRequestUrl(r.getServletPath(), r.getRequestURI(), r.getContextPath(), r.getPathInfo(),
@@ -70,18 +94,9 @@ public final class UrlUtils {
/**
* Obtains the web application-specific fragment of the URL.
- *
- * Under normal spec conditions,
- *
- * requestURI = contextPath + servletPath + pathInfo
- *
- *
- * But this method may also be called using dummy request objects which just have the requestURI and contextPath
- * set, for example.
- *
- * @return the URL, excluding any server name, context path or servlet path
+
*/
- public static String buildRequestUrl(String servletPath, String requestURI, String contextPath, String pathInfo,
+ private static String buildRequestUrl(String servletPath, String requestURI, String contextPath, String pathInfo,
String queryString) {
StringBuilder url = new StringBuilder();