From bf83e4e8612512ef51272d8a7dd8533686e8a4e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20=C5=A0vorc?= Date: Sun, 7 May 2017 20:59:09 +0200 Subject: [PATCH 1/2] Use original query string of forwarded request Prior to this commit, the AbstractFlashMapManager has used the originating URI but the query string of the forwarded request. That resulted to FlashMap not being matched even when both originating URI and query string matched the FlashMap attributes. The originating query string is now used to match the forwarded request. Issue: SPR-15505 --- .../support/AbstractFlashMapManager.java | 5 +++-- .../servlet/support/FlashMapManagerTests.java | 20 +++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/support/AbstractFlashMapManager.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/support/AbstractFlashMapManager.java index 61f3ef9216..06a04df339 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/support/AbstractFlashMapManager.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/support/AbstractFlashMapManager.java @@ -167,13 +167,14 @@ public abstract class AbstractFlashMapManager implements FlashMapManager { */ protected boolean isFlashMapForRequest(FlashMap flashMap, HttpServletRequest request) { String expectedPath = flashMap.getTargetRequestPath(); + String requestUri = getUrlPathHelper().getOriginatingRequestUri(request); if (expectedPath != null) { - String requestUri = getUrlPathHelper().getOriginatingRequestUri(request); if (!requestUri.equals(expectedPath) && !requestUri.equals(expectedPath + "/")) { return false; } } - UriComponents uriComponents = ServletUriComponentsBuilder.fromRequest(request).build(); + String queryString = getUrlPathHelper().getOriginatingQueryString(request); + UriComponents uriComponents = ServletUriComponentsBuilder.fromUriString(requestUri).query(queryString).build(); MultiValueMap actualParams = uriComponents.getQueryParams(); MultiValueMap expectedParams = flashMap.getTargetRequestParams(); for (String expectedName : expectedParams.keySet()) { diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/support/FlashMapManagerTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/support/FlashMapManagerTests.java index 8fa215672e..829975b0c3 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/support/FlashMapManagerTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/support/FlashMapManagerTests.java @@ -318,6 +318,26 @@ public class FlashMapManagerTests { assertEquals("value", flashMap.get("key")); } + // SPR-15505 + + @Test + public void retrieveAndUpdateMatchByOriginatingPathAndQueryString() { + FlashMap flashMap = new FlashMap(); + flashMap.put("key", "value"); + flashMap.setTargetRequestPath("/accounts"); + flashMap.addTargetRequestParam("a", "b"); + + this.flashMapManager.setFlashMaps(Arrays.asList(flashMap)); + + this.request.setAttribute(WebUtils.FORWARD_REQUEST_URI_ATTRIBUTE, "/accounts"); + this.request.setAttribute(WebUtils.FORWARD_QUERY_STRING_ATTRIBUTE, "a=b"); + this.request.setRequestURI("/mvc/accounts"); + this.request.setQueryString("x=y"); + FlashMap inputFlashMap = this.flashMapManager.retrieveAndUpdate(this.request, this.response); + + assertEquals(flashMap, inputFlashMap); + assertEquals("Input FlashMap should have been removed", 0, this.flashMapManager.getFlashMaps().size()); + } private static class TestFlashMapManager extends AbstractFlashMapManager { From 48a5938cd4dcbfcf8198b08cf078a774f8ba6b1f Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Fri, 19 May 2017 16:59:36 -0400 Subject: [PATCH 2/2] Polish --- .../web/servlet/support/AbstractFlashMapManager.java | 12 +++++++----- .../web/servlet/support/FlashMapManagerTests.java | 8 ++++---- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/support/AbstractFlashMapManager.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/support/AbstractFlashMapManager.java index 06a04df339..0cc4d99ef8 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/support/AbstractFlashMapManager.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/support/AbstractFlashMapManager.java @@ -33,7 +33,6 @@ import org.springframework.util.MultiValueMap; import org.springframework.util.StringUtils; import org.springframework.web.servlet.FlashMap; import org.springframework.web.servlet.FlashMapManager; -import org.springframework.web.util.UriComponents; import org.springframework.web.util.UrlPathHelper; @@ -167,15 +166,13 @@ public abstract class AbstractFlashMapManager implements FlashMapManager { */ protected boolean isFlashMapForRequest(FlashMap flashMap, HttpServletRequest request) { String expectedPath = flashMap.getTargetRequestPath(); - String requestUri = getUrlPathHelper().getOriginatingRequestUri(request); if (expectedPath != null) { + String requestUri = getUrlPathHelper().getOriginatingRequestUri(request); if (!requestUri.equals(expectedPath) && !requestUri.equals(expectedPath + "/")) { return false; } } - String queryString = getUrlPathHelper().getOriginatingQueryString(request); - UriComponents uriComponents = ServletUriComponentsBuilder.fromUriString(requestUri).query(queryString).build(); - MultiValueMap actualParams = uriComponents.getQueryParams(); + MultiValueMap actualParams = getOriginatingRequestParams(request); MultiValueMap expectedParams = flashMap.getTargetRequestParams(); for (String expectedName : expectedParams.keySet()) { List actualValues = actualParams.get(expectedName); @@ -191,6 +188,11 @@ public abstract class AbstractFlashMapManager implements FlashMapManager { return true; } + private MultiValueMap getOriginatingRequestParams(HttpServletRequest request) { + String query = getUrlPathHelper().getOriginatingQueryString(request); + return ServletUriComponentsBuilder.fromPath("/").query(query).build().getQueryParams(); + } + @Override public final void saveOutputFlashMap(FlashMap flashMap, HttpServletRequest request, HttpServletResponse response) { if (CollectionUtils.isEmpty(flashMap)) { diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/support/FlashMapManagerTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/support/FlashMapManagerTests.java index 829975b0c3..07906ab4f5 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/support/FlashMapManagerTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/support/FlashMapManagerTests.java @@ -21,6 +21,7 @@ import static org.junit.Assert.*; import java.net.URLEncoder; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; @@ -318,16 +319,14 @@ public class FlashMapManagerTests { assertEquals("value", flashMap.get("key")); } - // SPR-15505 - - @Test + @Test // SPR-15505 public void retrieveAndUpdateMatchByOriginatingPathAndQueryString() { FlashMap flashMap = new FlashMap(); flashMap.put("key", "value"); flashMap.setTargetRequestPath("/accounts"); flashMap.addTargetRequestParam("a", "b"); - this.flashMapManager.setFlashMaps(Arrays.asList(flashMap)); + this.flashMapManager.setFlashMaps(Collections.singletonList(flashMap)); this.request.setAttribute(WebUtils.FORWARD_REQUEST_URI_ATTRIBUTE, "/accounts"); this.request.setAttribute(WebUtils.FORWARD_QUERY_STRING_ATTRIBUTE, "a=b"); @@ -339,6 +338,7 @@ public class FlashMapManagerTests { assertEquals("Input FlashMap should have been removed", 0, this.flashMapManager.getFlashMaps().size()); } + private static class TestFlashMapManager extends AbstractFlashMapManager { private List flashMaps;