From 364bb696e0d45644919f418a9d7bad1522147820 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Tue, 21 Aug 2012 15:27:21 -0400 Subject: [PATCH] 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 --- .../springframework/web/servlet/FlashMap.java | 34 ++++++++-------- .../support/AbstractFlashMapManager.java | 39 ++++++++++++++----- .../support/AbstractFlashMapManagerTests.java | 15 +++++++ 3 files changed, 61 insertions(+), 27 deletions(-) diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/FlashMap.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/FlashMap.java index 08b7f736a4..983930714c 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/FlashMap.java +++ b/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. - * - *

A FlashMap can be set up with a request path and request parameters to + * + *

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. - * + * *

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 implements Comparable { private static final long serialVersionUID = 1L; - + private String targetRequestPath; - + private final MultiValueMap targetRequestParams = new LinkedMultiValueMap(); - + private long expirationStartTime; - + private int timeToLive; /** @@ -100,12 +100,12 @@ public final class FlashMap extends HashMap implements Comparabl } return this; } - + /** * Return the parameters identifying the target request, or an empty map. */ public MultiValueMap getTargetRequestParams() { - return targetRequestParams; + return this.targetRequestParams; } /** @@ -131,7 +131,7 @@ public final class FlashMap extends HashMap 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. */ 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 e745cfb47b..9fe88e8ef7 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 @@ -83,28 +83,33 @@ public abstract class AbstractFlashMapManager implements FlashMapManager { } public final FlashMap retrieveAndUpdate(HttpServletRequest request, HttpServletResponse response) { - List allMaps = retrieveFlashMaps(request); - if (CollectionUtils.isEmpty(allMaps)) { + + List 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 mapsToRemove = getExpiredFlashMaps(allMaps); - FlashMap match = getMatchingFlashMap(allMaps, request); + + List 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 allMaps = retrieveFlashMaps(request); allMaps = (allMaps == null) ? new CopyOnWriteArrayList() : allMaps; @@ -203,6 +214,14 @@ 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))) { + 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 diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/support/AbstractFlashMapManagerTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/support/AbstractFlashMapManagerTests.java index f4b80f6b1a..c5a259fb22 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/support/AbstractFlashMapManagerTests.java +++ b/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 {