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 503662876f6..c859a2301a8 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,11 +16,11 @@ package org.springframework.web.servlet.support; -import java.util.ArrayList; import java.util.Collections; import java.util.LinkedList; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; + import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -30,12 +30,13 @@ import org.apache.commons.logging.LogFactory; import org.springframework.util.Assert; import org.springframework.util.CollectionUtils; import org.springframework.util.MultiValueMap; -import org.springframework.util.ObjectUtils; 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; + /** * A base class for {@link FlashMapManager} implementations. * @@ -172,10 +173,16 @@ public abstract class AbstractFlashMapManager implements FlashMapManager { return false; } } - MultiValueMap targetParams = flashMap.getTargetRequestParams(); - for (String expectedName : targetParams.keySet()) { - for (String expectedValue : targetParams.get(expectedName)) { - if (!ObjectUtils.containsElement(request.getParameterValues(expectedName), expectedValue)) { + UriComponents uriComponents = ServletUriComponentsBuilder.fromRequest(request).build(); + MultiValueMap actualParams = uriComponents.getQueryParams(); + MultiValueMap expectedParams = flashMap.getTargetRequestParams(); + for (String expectedName : expectedParams.keySet()) { + List actualValues = actualParams.get(expectedName); + if (actualValues == null) { + return false; + } + for (String expectedValue : expectedParams.get(expectedName)) { + if (!actualValues.contains(expectedValue)) { return false; } } @@ -191,7 +198,6 @@ public abstract class AbstractFlashMapManager implements FlashMapManager { String path = decodeAndNormalizePath(flashMap.getTargetRequestPath(), request); flashMap.setTargetRequestPath(path); - decodeParameters(flashMap.getTargetRequestParams(), request); if (logger.isDebugEnabled()) { logger.debug("Saving FlashMap=" + flashMap); @@ -227,17 +233,6 @@ public abstract class AbstractFlashMapManager implements FlashMapManager { return path; } - private void decodeParameters(MultiValueMap params, HttpServletRequest request) { - for (String name : new ArrayList(params.keySet())) { - for (String value : new ArrayList(params.remove(name))) { - name = getUrlPathHelper().decodeRequestString(request, name); - value = getUrlPathHelper().decodeRequestString(request, value); - params.add(name, value); - } - } - } - - /** * Retrieve saved FlashMap instances from the underlying storage. * @param request the current request diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java index 737408a083e..e7d7e1abc08 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,6 +16,8 @@ package org.springframework.web.servlet.mvc.method.annotation; +import static org.junit.Assert.*; + import java.beans.PropertyEditorSupport; import java.io.IOException; import java.io.Serializable; @@ -42,6 +44,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Set; + import javax.servlet.ServletConfig; import javax.servlet.ServletContext; import javax.servlet.ServletException; @@ -137,8 +140,6 @@ import org.springframework.web.servlet.mvc.support.RedirectAttributes; import org.springframework.web.servlet.support.RequestContextUtils; import org.springframework.web.servlet.view.InternalResourceViewResolver; -import static org.junit.Assert.*; - /** * The origin of this test class is {@link ServletAnnotationControllerHandlerMethodTests}. * @@ -1532,7 +1533,7 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl // GET after POST request = new MockHttpServletRequest("GET", "/messages/1"); - request.addParameter("name", "value"); + request.setQueryString("name=value"); request.setSession(session); response = new MockHttpServletResponse(); getServlet().service(request, response); 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 6b5fd2b3917..9a5e3cb7731 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 @@ -18,6 +18,7 @@ package org.springframework.web.servlet.support; import static org.junit.Assert.*; +import java.net.URLEncoder; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -31,7 +32,6 @@ import org.junit.Test; import org.springframework.mock.web.test.MockHttpServletRequest; import org.springframework.mock.web.test.MockHttpServletResponse; -import org.springframework.util.MultiValueMap; import org.springframework.web.servlet.FlashMap; import org.springframework.web.util.WebUtils; @@ -113,19 +113,19 @@ public class FlashMapManagerTests { this.flashMapManager.setFlashMaps(Arrays.asList(flashMap)); - this.request.setParameter("number", (String) null); + this.request.setQueryString("number="); FlashMap inputFlashMap = this.flashMapManager.retrieveAndUpdate(this.request, this.response); assertNull(inputFlashMap); assertEquals("FlashMap should not have been removed", 1, this.flashMapManager.getFlashMaps().size()); - this.request.setParameter("number", "two"); + this.request.setQueryString("number=two"); inputFlashMap = this.flashMapManager.retrieveAndUpdate(this.request, this.response); assertNull(inputFlashMap); assertEquals("FlashMap should not have been removed", 1, this.flashMapManager.getFlashMaps().size()); - this.request.setParameter("number", "one"); + this.request.setQueryString("number=one"); inputFlashMap = this.flashMapManager.retrieveAndUpdate(this.request, this.response); assertEquals(flashMap, inputFlashMap); @@ -143,13 +143,13 @@ public class FlashMapManagerTests { this.flashMapManager.setFlashMaps(Arrays.asList(flashMap)); - this.request.setParameter("id", "1"); + this.request.setQueryString("id=1"); FlashMap inputFlashMap = this.flashMapManager.retrieveAndUpdate(this.request, this.response); assertNull(inputFlashMap); assertEquals("FlashMap should not have been removed", 1, this.flashMapManager.getFlashMaps().size()); - this.request.addParameter("id", "2"); + this.request.setQueryString("id=1&id=2"); inputFlashMap = this.flashMapManager.retrieveAndUpdate(this.request, this.response); assertEquals(flashMap, inputFlashMap); @@ -268,20 +268,54 @@ public class FlashMapManagerTests { @Test public void saveOutputFlashMapDecodeParameters() throws Exception { - this.request.setCharacterEncoding("UTF-8"); FlashMap flashMap = new FlashMap(); - flashMap.put("anyKey", "anyValue"); - - flashMap.addTargetRequestParam("key", "%D0%90%D0%90"); - flashMap.addTargetRequestParam("key", "%D0%91%D0%91"); - flashMap.addTargetRequestParam("key", "%D0%92%D0%92"); + flashMap.put("key", "value"); + flashMap.setTargetRequestPath("/path"); + flashMap.addTargetRequestParam("param", "%D0%90%D0%90"); + flashMap.addTargetRequestParam("param", "%D0%91%D0%91"); + flashMap.addTargetRequestParam("param", "%D0%92%D0%92"); flashMap.addTargetRequestParam("%3A%2F%3F%23%5B%5D%40", "value"); + + this.request.setCharacterEncoding("UTF-8"); this.flashMapManager.saveOutputFlashMap(flashMap, this.request, this.response); - MultiValueMap targetRequestParams = flashMap.getTargetRequestParams(); - assertEquals(Arrays.asList("\u0410\u0410", "\u0411\u0411", "\u0412\u0412"), targetRequestParams.get("key")); - assertEquals(Arrays.asList("value"), targetRequestParams.get(":/?#[]@")); + MockHttpServletRequest requestAfterRedirect = new MockHttpServletRequest("GET", "/path"); + requestAfterRedirect.setQueryString("param=%D0%90%D0%90¶m=%D0%91%D0%91¶m=%D0%92%D0%92&%3A%2F%3F%23%5B%5D%40=value"); + requestAfterRedirect.addParameter("param", "\u0410\u0410"); + requestAfterRedirect.addParameter("param", "\u0411\u0411"); + requestAfterRedirect.addParameter("param", "\u0412\u0412"); + requestAfterRedirect.addParameter(":/?#[]@", "value"); + + flashMap = this.flashMapManager.retrieveAndUpdate(requestAfterRedirect, new MockHttpServletResponse()); + assertNotNull(flashMap); + assertEquals(1, flashMap.size()); + assertEquals("value", flashMap.get("key")); + } + + // SPR-12569 + + @Test + public void flashAttributesWithQueryParamsWithSpace() throws Exception { + + String encodedValue = URLEncoder.encode("1 2", "UTF-8"); + + FlashMap flashMap = new FlashMap(); + flashMap.put("key", "value"); + flashMap.setTargetRequestPath("/path"); + flashMap.addTargetRequestParam("param", encodedValue); + + this.request.setCharacterEncoding("UTF-8"); + this.flashMapManager.saveOutputFlashMap(flashMap, this.request, this.response); + + MockHttpServletRequest requestAfterRedirect = new MockHttpServletRequest("GET", "/path"); + requestAfterRedirect.setQueryString("param=" + encodedValue); + requestAfterRedirect.addParameter("param", "1 2"); + + flashMap = this.flashMapManager.retrieveAndUpdate(requestAfterRedirect, new MockHttpServletResponse()); + assertNotNull(flashMap); + assertEquals(1, flashMap.size()); + assertEquals("value", flashMap.get("key")); }