From 7f482eda7d2907abd0bd71ae3b5ca85aa223001f Mon Sep 17 00:00:00 2001 From: Eleftheria Stein Date: Thu, 26 Nov 2020 18:16:42 +0100 Subject: [PATCH] Fix CookieRequestCache for URL encoded query parameters Avoid populating the saved request parameters with encoded values. Since the query strings of the request and saved URL are compared and must be equal, we can just use the parameters from the incoming request. Closes gh-9203 --- .../web/savedrequest/CookieRequestCache.java | 6 ------ .../savedrequest/CookieRequestCacheTests.java | 19 +++++++++++++++++++ 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/web/src/main/java/org/springframework/security/web/savedrequest/CookieRequestCache.java b/web/src/main/java/org/springframework/security/web/savedrequest/CookieRequestCache.java index 1afefeba74..c24000f943 100644 --- a/web/src/main/java/org/springframework/security/web/savedrequest/CookieRequestCache.java +++ b/web/src/main/java/org/springframework/security/web/savedrequest/CookieRequestCache.java @@ -17,7 +17,6 @@ package org.springframework.security.web.savedrequest; import java.util.Base64; -import java.util.HashMap; import javax.servlet.http.Cookie; import javax.servlet.http.HttpServletRequest; @@ -79,11 +78,6 @@ public class CookieRequestCache implements RequestCache { DefaultSavedRequest.Builder builder = new DefaultSavedRequest.Builder(); int port = getPort(uriComponents); MultiValueMap queryParams = uriComponents.getQueryParams(); - if (!queryParams.isEmpty()) { - HashMap parameters = new HashMap<>(queryParams.size()); - queryParams.forEach((key, value) -> parameters.put(key, value.toArray(new String[] {}))); - builder.setParameters(parameters); - } return builder.setScheme(uriComponents.getScheme()).setServerName(uriComponents.getHost()) .setRequestURI(uriComponents.getPath()).setQueryString(uriComponents.getQuery()).setServerPort(port) .setMethod(request.getMethod()).build(); diff --git a/web/src/test/java/org/springframework/security/web/savedrequest/CookieRequestCacheTests.java b/web/src/test/java/org/springframework/security/web/savedrequest/CookieRequestCacheTests.java index 9572f9e012..c3bb91500c 100644 --- a/web/src/test/java/org/springframework/security/web/savedrequest/CookieRequestCacheTests.java +++ b/web/src/test/java/org/springframework/security/web/savedrequest/CookieRequestCacheTests.java @@ -153,6 +153,25 @@ public class CookieRequestCacheTests { assertThat(expiredCookie).isNull(); } + @Test + public void matchingRequestWhenUrlEncodedQueryParametersThenDoesNotDuplicate() { + CookieRequestCache cookieRequestCache = new CookieRequestCache(); + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setServerPort(443); + request.setSecure(true); + request.setScheme("https"); + request.setServerName("abc.com"); + request.setRequestURI("/destination"); + request.setQueryString("goto=https%3A%2F%2Fstart.spring.io"); + request.setParameter("goto", "https://start.spring.io"); + String redirectUrl = "https://abc.com/destination?goto=https%3A%2F%2Fstart.spring.io"; + request.setCookies(new Cookie(DEFAULT_COOKIE_NAME, encodeCookie(redirectUrl))); + MockHttpServletResponse response = new MockHttpServletResponse(); + final HttpServletRequest matchingRequest = cookieRequestCache.getMatchingRequest(request, response); + assertThat(matchingRequest).isNotNull(); + assertThat(matchingRequest.getParameterValues("goto")).containsExactly("https://start.spring.io"); + } + @Test public void removeRequestWhenInvokedThenSetsAnExpiredCookieOnResponse() { CookieRequestCache cookieRequestCache = new CookieRequestCache();