Decode target parameters prior to saving a FlashMap

The target parameters for a FlashMap must be decoded to be able to
match them to the parameters of incoming requests given that the
HttpServletRequest returns decoded request parameters.

SPR-9657

spring-webmvc/src/main/java/org/springframework/web/servlet/FlashMap.java

@@ -23,38 +23,38 @@ import org.springframework.util.MultiValueMap;
 import org.springframework.util.StringUtils;
 
 /**
- * A FlashMap provides a way for one request to store attributes intended for 
+ * A FlashMap provides a way for one request to store attributes intended for
  * use in another. This is most commonly needed when redirecting from one URL
- * to another -- e.g. the Post/Redirect/Get pattern. A FlashMap is saved before 
- * the redirect (typically in the session) and is made available after the 
+ * to another -- e.g. the Post/Redirect/Get pattern. A FlashMap is saved before
+ * the redirect (typically in the session) and is made available after the
  * redirect and removed immediately.
- * 
- * <p>A FlashMap can be set up with a request path and request parameters to 
+ *
+ * <p>A FlashMap can be set up with a request path and request parameters to
  * help identify the target request. Without this information, a FlashMap is
- * made available to the next request, which may or may not be the intended 
+ * made available to the next request, which may or may not be the intended
  * recipient. On a redirect, the target URL is known and a FlashMap can be
  * updated with that information. This is done automatically when the
  * {@code org.springframework.web.servlet.view.RedirectView} is used.
- * 
+ *
  * <p>Note: annotated controllers will usually not use FlashMap directly.
  * See {@code org.springframework.web.servlet.mvc.support.RedirectAttributes}
- * for an overview of using flash attributes in annotated controllers. 
- * 
+ * for an overview of using flash attributes in annotated controllers.
+ *
  * @author Rossen Stoyanchev
  * @since 3.1
- * 
+ *
  * @see FlashMapManager
  */
 public final class FlashMap extends HashMap<String, Object> implements Comparable<FlashMap> {
 
 	private static final long serialVersionUID = 1L;
-	
+
 	private String targetRequestPath;
-	
+
 	private final MultiValueMap<String, String> targetRequestParams = new LinkedMultiValueMap<String, String>();
-	
+
 	private long expirationStartTime;
-	
+
 	private int timeToLive;
 
 	/**
@@ -100,12 +100,12 @@ public final class FlashMap extends HashMap<String, Object> implements Comparabl
 		}
 		return this;
 	}
-	
+
 	/**
 	 * Return the parameters identifying the target request, or an empty map.
 	 */
 	public MultiValueMap<String, String> getTargetRequestParams() {
-		return targetRequestParams;
+		return this.targetRequestParams;
 	}
 
 	/**
@@ -131,7 +131,7 @@ public final class FlashMap extends HashMap<String, Object> implements Comparabl
 	}
 
 	/**
-	 * Compare two FlashMaps and prefer the one that specifies a target URL 
+	 * Compare two FlashMaps and prefer the one that specifies a target URL
 	 * path or has more target URL parameters. Before comparing FlashMap
 	 * instances ensure that they match a given request.
 	 */

spring-webmvc/src/main/java/org/springframework/web/servlet/support/AbstractFlashMapManager.java

@@ -83,28 +83,33 @@ public abstract class AbstractFlashMapManager implements FlashMapManager {
 	}
 
 	public final FlashMap retrieveAndUpdate(HttpServletRequest request, HttpServletResponse response) {
-		List<FlashMap> allMaps = retrieveFlashMaps(request);
-		if (CollectionUtils.isEmpty(allMaps)) {
+
+		List<FlashMap> maps = retrieveFlashMaps(request);
+		if (CollectionUtils.isEmpty(maps)) {
 			return null;
 		}
 		if (logger.isDebugEnabled()) {
-			logger.debug("Retrieved FlashMap(s): " + allMaps);
+			logger.debug("Retrieved FlashMap(s): " + maps);
 		}
-		List<FlashMap> mapsToRemove = getExpiredFlashMaps(allMaps);
-		FlashMap match = getMatchingFlashMap(allMaps, request);
+
+		List<FlashMap> mapsToRemove = getExpiredFlashMaps(maps);
+
+		FlashMap match = getMatchingFlashMap(maps, request);
 		if (match != null) {
 			mapsToRemove.add(match);
 		}
+
 		if (!mapsToRemove.isEmpty()) {
 			if (logger.isDebugEnabled()) {
-				logger.debug("Removing FlashMap(s): " + allMaps);
+				logger.debug("Removing FlashMap(s): " + mapsToRemove);
 			}
 			synchronized (writeLock) {
-				allMaps = retrieveFlashMaps(request);
-				allMaps.removeAll(mapsToRemove);
-				updateFlashMaps(allMaps, request, response);
+				maps = retrieveFlashMaps(request);
+				maps.removeAll(mapsToRemove);
+				updateFlashMaps(maps, request, response);
 			}
 		}
+
 		return match;
 	}
 
@@ -177,12 +182,18 @@ public abstract class AbstractFlashMapManager implements FlashMapManager {
 		if (CollectionUtils.isEmpty(flashMap)) {
 			return;
 		}
+
 		String path = decodeAndNormalizePath(flashMap.getTargetRequestPath(), request);
 		flashMap.setTargetRequestPath(path);
-		flashMap.startExpirationPeriod(this.flashMapTimeout);
+
+		decodeParameters(flashMap.getTargetRequestParams(), request);
+
 		if (logger.isDebugEnabled()) {
 			logger.debug("Saving FlashMap=" + flashMap);
 		}
+
+		flashMap.startExpirationPeriod(this.flashMapTimeout);
+
 		synchronized (writeLock) {
 			List<FlashMap> allMaps = retrieveFlashMaps(request);
 			allMaps = (allMaps == null) ? new CopyOnWriteArrayList<FlashMap>() : allMaps;
@@ -203,6 +214,14 @@ public abstract class AbstractFlashMapManager implements FlashMapManager {
 		return path;
 	}
 
+	private void decodeParameters(MultiValueMap<String, String> params, HttpServletRequest request) {
+		for (String name : new ArrayList<String>(params.keySet())) {
+			for (String value : new ArrayList<String>(params.remove(name))) {
+				params.add(name, this.urlPathHelper.decodeRequestString(request, value));
+			}
+		}
+	}
+
 	/**
 	 * Update the FlashMap instances in some underlying storage.
 	 * @param flashMaps a non-empty list of FlashMap instances to save

spring-webmvc/src/test/java/org/springframework/web/servlet/support/AbstractFlashMapManagerTests.java

@@ -264,6 +264,21 @@ public class AbstractFlashMapManagerTests {
 		assertEquals("/once/only", flashMap.getTargetRequestPath());
 	}
 
+	@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");
+		this.flashMapManager.saveOutputFlashMap(flashMap, this.request, this.response);
+
+		assertEquals(Arrays.asList("АА", "ББ", "ВВ"), flashMap.getTargetRequestParams().get("key"));
+	}
+
 
 	private static class TestFlashMapManager extends AbstractFlashMapManager {