diff --git a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/DispatcherServlet.java b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/DispatcherServlet.java index afada19528..1f7ebf7972 100644 --- a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/DispatcherServlet.java +++ b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/DispatcherServlet.java @@ -53,7 +53,6 @@ import org.springframework.web.context.request.ServletWebRequest; import org.springframework.web.multipart.MultipartException; import org.springframework.web.multipart.MultipartHttpServletRequest; import org.springframework.web.multipart.MultipartResolver; -import org.springframework.web.servlet.support.RequestContextUtils; import org.springframework.web.util.NestedServletException; import org.springframework.web.util.UrlPathHelper; import org.springframework.web.util.WebUtils; @@ -815,7 +814,7 @@ public class DispatcherServlet extends FrameworkServlet { } } - boolean flashInitialized = this.flashMapManager.requestStarted(request); + this.flashMapManager.requestStarted(request); // Make framework objects available to handlers and view objects. request.setAttribute(WEB_APPLICATION_CONTEXT_ATTRIBUTE, getWebApplicationContext()); @@ -827,9 +826,8 @@ public class DispatcherServlet extends FrameworkServlet { doDispatch(request, response); } finally { - if (flashInitialized) { - this.flashMapManager.requestCompleted(request); - } + this.flashMapManager.requestCompleted(request); + // Restore the original attribute snapshot, in case of an include. if (attributesSnapshot != null) { restoreAttributesAfterInclude(request, attributesSnapshot); diff --git a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/FlashMap.java b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/FlashMap.java index 4e0dc66ed8..5e80f0f363 100644 --- a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/FlashMap.java +++ b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/FlashMap.java @@ -20,98 +20,90 @@ import java.util.HashMap; import java.util.LinkedHashMap; import java.util.Map; -import javax.servlet.http.HttpServletRequest; - import org.springframework.beans.BeanUtils; import org.springframework.util.Assert; -import org.springframework.util.StringUtils; -import org.springframework.web.util.UrlPathHelper; /** - * Stores attributes that need to be made available in the next request. + * 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 + * redirect and removed immediately. + * + *

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 + * result. Before a redirect, the target URL is known and when using the + * {@code org.springframework.web.servlet.view.RedirectView}, FlashMap + * instances are automatically updated with redirect URL information. + * + *

Annotated controllers will usually not access a FlashMap directly.. TODO * * @author Rossen Stoyanchev * @since 3.1 + * + * @see FlashMapManager */ public class FlashMap extends HashMap implements Comparable { private static final long serialVersionUID = 1L; - private String expectedRequestUri; + private String targetRequestPath; - private final Map expectedRequestParameters = new LinkedHashMap(); + private final Map targetRequestParams = new LinkedHashMap(); private long expirationStartTime; private int timeToLive; - private final UrlPathHelper urlPathHelper = new UrlPathHelper(); - + private final int createdBy; + /** - * Provide a URL to identify the target request for this FlashMap. - * Only the path of the provided URL will be used for matching purposes. - * If the URL is absolute or has a query string, the URL path is - * extracted. Or if the URL is relative, it is appended to the current - * request URI and normalized. - * - * @param request the current request, used to normalize relative URLs - * @param url an absolute URL, a URL path, or a relative URL, never {@code null} + * Create a new instance. */ - public FlashMap setExpectedRequestUri(HttpServletRequest request, String url) { - Assert.notNull(url, "Expected URL must not be null"); - String path = extractRequestUri(url); - this.expectedRequestUri = path.startsWith("/") ? path : normalizeRelativePath(request, path); - return this; - } - - private String extractRequestUri(String url) { - int index = url.indexOf("?"); - if (index != -1) { - url = url.substring(0, index); - } - index = url.indexOf("://"); - if (index != -1) { - int pathBegin = url.indexOf("/", index + 3); - url = (pathBegin != -1 ) ? url.substring(pathBegin) : ""; - } - return url; - } - - private String normalizeRelativePath(HttpServletRequest request, String relativeUrl) { - String requestUri = this.urlPathHelper.getRequestUri(request); - relativeUrl = requestUri.substring(0, requestUri.lastIndexOf('/') + 1) + relativeUrl; - return StringUtils.cleanPath(relativeUrl); + public FlashMap() { + this.createdBy = 0; } /** - * Add a request parameter pair to help identify the request this FlashMap - * should be made available to. If expected parameters are not set, the - * FlashMap instance will match to requests with any parameters. - * - * @param name the name of the expected parameter (never {@code null}) - * @param value the value for the expected parameter (never {@code null}) + * Create a new instance with an id uniquely identifying the creator of + * this FlashMap. */ - public FlashMap setExpectedRequestParam(String name, String value) { - this.expectedRequestParameters.put(name, value.toString()); - return this; + public FlashMap(int createdBy) { + this.createdBy = createdBy; } /** - * Provide request parameter pairs to help identify the request this FlashMap - * should be made available to. If expected parameters are not set, the - * FlashMap instance will match to requests with any parameters. - * - *

Although the provided map contain any Object values, only non-"simple" - * value types as defined in {@link BeanUtils#isSimpleValueType} are used. - * + * Provide a URL path to help identify the target request for this FlashMap. + * The path may be absolute (e.g. /application/resource) or relative to the + * current request (e.g. ../resource). + * @param path the URI path, never {@code null} + */ + public void setTargetRequestPath(String path) { + Assert.notNull(path, "Expected path must not be null"); + this.targetRequestPath = path; + } + + /** + * Return the URL path of the target request, or {@code null} if none. + */ + public String getTargetRequestPath() { + return targetRequestPath; + } + + /** + * Provide request parameter pairs to identify the request for this FlashMap. + * If not set, the FlashMap will match to requests with any parameters. + * Only simple value types, as defined in {@link BeanUtils#isSimpleValueType}, + * are used. * @param params a Map with the names and values of expected parameters. */ - public FlashMap setExpectedRequestParams(Map params) { + public FlashMap addTargetRequestParams(Map params) { if (params != null) { for (String name : params.keySet()) { Object value = params.get(name); if ((value != null) && BeanUtils.isSimpleValueType(value.getClass())) { - this.expectedRequestParameters.put(name, value.toString()); + this.targetRequestParams.put(name, value.toString()); } } } @@ -119,37 +111,24 @@ public class FlashMap extends HashMap implements Comparable entry : this.expectedRequestParameters.entrySet()) { - if (!entry.getValue().equals(request.getParameter(entry.getKey()))) { - return false; - } - } - } - return true; + public FlashMap addTargetRequestParam(String name, String value) { + this.targetRequestParams.put(name, value.toString()); + return this; } - private boolean matchPathsIgnoreTrailingSlash(String path1, String path2) { - path1 = path1.endsWith("/") ? path1.substring(0, path1.length() - 1) : path1; - path2 = path2.endsWith("/") ? path2.substring(0, path2.length() - 1) : path2; - return path1.equals(path2); + /** + * Return the parameters identifying the target request, or an empty Map. + */ + public Map getTargetRequestParams() { + return targetRequestParams; } - + /** * Start the expiration period for this instance. After the given number of * seconds calls to {@link #isExpired()} will return "true". @@ -174,21 +153,24 @@ public class FlashMap extends HashMap implements ComparableIt is expected that both instances have been matched against the - * current request via {@link FlashMap#matches}. + * Whether the given id matches the id of the creator of this FlashMap. + */ + public boolean isCreatedBy(int createdBy) { + return this.createdBy == createdBy; + } + + /** + * Compare two FlashMaps and select the one that has a target URL path or + * has more target request parameters. */ public int compareTo(FlashMap other) { - int thisUrlPath = (this.expectedRequestUri != null) ? 1 : 0; - int otherUrlPath = (other.expectedRequestUri != null) ? 1 : 0; + int thisUrlPath = (this.targetRequestPath != null) ? 1 : 0; + int otherUrlPath = (other.targetRequestPath != null) ? 1 : 0; if (thisUrlPath != otherUrlPath) { return otherUrlPath - thisUrlPath; } else { - return other.expectedRequestParameters.size() - this.expectedRequestParameters.size(); + return other.targetRequestParams.size() - this.targetRequestParams.size(); } } @@ -196,8 +178,8 @@ public class FlashMap extends HashMap implements ComparableThe most common use case for using flash storage is a redirect. - * For example creating a resource in a POST request and then redirecting - * to the page that shows the resource. Flash storage may be used to - * pass along a success message. + *

A FlashMapManager is invoked at the beginning and at the end of a request. + * For each request, it exposes an "input" FlashMap with attributes passed from + * a previous request (if any) and an "output" FlashMap with attributes to pass + * to a subsequent request. Both FlashMap instances are exposed via request + * attributes and can be accessed through methods in + * {@code org.springframework.web.servlet.support.RequestContextUtils}. * * @author Rossen Stoyanchev * @since 3.1 @@ -37,41 +39,42 @@ import org.springframework.web.servlet.support.RequestContextUtils; public interface FlashMapManager { /** - * Request attribute holding the read-only Map with flash attributes saved - * during the previous request. - * @see RequestContextUtils#getInputFlashMap(HttpServletRequest) + * Name of request attribute that holds a read-only {@link Map} with + * "input" flash attributes from a previous request (if any). + * @see org.springframework.web.servlet.support.RequestContextUtils#getInputFlashMap(HttpServletRequest) */ public static final String INPUT_FLASH_MAP_ATTRIBUTE = FlashMapManager.class.getName() + ".INPUT_FLASH_MAP"; /** - * Request attribute holding the {@link FlashMap} to add attributes to during - * the current request. - * @see RequestContextUtils#getOutputFlashMap(HttpServletRequest) + * Name of request attribute that holds the "output" {@link FlashMap} with + * attributes to pass to a subsequent request. + * @see org.springframework.web.servlet.support.RequestContextUtils#getOutputFlashMap(HttpServletRequest) */ public static final String OUTPUT_FLASH_MAP_ATTRIBUTE = FlashMapManager.class.getName() + ".OUTPUT_FLASH_MAP"; /** - * Perform flash storage tasks at the start of a new request: - *

- *

If the {@link #OUTPUT_FLASH_MAP_ATTRIBUTE} request attribute exists - * return "false" immediately. + * * * @param request the current request - * @return "true" if flash storage tasks were performed; "false" otherwise. */ - boolean requestStarted(HttpServletRequest request); + void requestStarted(HttpServletRequest request); /** - * Access the FlashMap with attributes added during the current request and - * if it is not empty, save it in the underlying storage. - *

If the call to {@link #requestStarted} returned "false", this - * method is not invoked. + * Save the "output" FlashMap in underlying storage, start its expiration + * period, and decode/normalize its target request path. + * + *

The "output" FlashMap is not saved if it is empty or if it was not + * created by this FlashMapManager. */ void requestCompleted(HttpServletRequest request); diff --git a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/condition/ParamsRequestCondition.java b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/condition/ParamsRequestCondition.java index 661f6ffe46..253333667c 100644 --- a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/condition/ParamsRequestCondition.java +++ b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/condition/ParamsRequestCondition.java @@ -54,8 +54,8 @@ public final class ParamsRequestCondition extends AbstractRequestCondition parseExpressions(String... params) { Set expressions = new LinkedHashSet(); if (params != null) { - for (String header : params) { - expressions.add(new ParamExpression(header)); + for (String param : params) { + expressions.add(new ParamExpression(param)); } } return expressions; diff --git a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/support/DefaultFlashMapManager.java b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/support/DefaultFlashMapManager.java index e521b2fd75..f5dbdb2e4a 100644 --- a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/support/DefaultFlashMapManager.java +++ b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/support/DefaultFlashMapManager.java @@ -28,12 +28,13 @@ import javax.servlet.http.HttpSession; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.util.CollectionUtils; +import org.springframework.util.StringUtils; import org.springframework.web.servlet.FlashMap; import org.springframework.web.servlet.FlashMapManager; +import org.springframework.web.util.UrlPathHelper; /** - * A {@link FlashMapManager} that saves and retrieves FlashMap instances in the - * HTTP session. + * A {@link FlashMapManager} that stores FlashMap instances in the HTTP session. * * @author Rossen Stoyanchev * @since 3.1 @@ -46,10 +47,11 @@ public class DefaultFlashMapManager implements FlashMapManager { private int flashTimeout = 180; + private final UrlPathHelper urlPathHelper = new UrlPathHelper(); + /** - * The amount of time in seconds after a request has completed processing - * and before a FlashMap is considered expired. - * The default value is 180. + * The amount of time in seconds after a FlashMap is saved (after request + * completion) before it is considered expired. The default value is 180. */ public void setFlashMapTimeout(int flashTimeout) { this.flashTimeout = flashTimeout; @@ -57,126 +59,153 @@ public class DefaultFlashMapManager implements FlashMapManager { /** * {@inheritDoc} - * - *

This method never creates an HTTP session. The new FlashMap created - * for the current request is exposed as a request attribute only and is - * not saved in the session until {@link #requestCompleted} is called. + *

This method never causes the HTTP session to be created. */ - public boolean requestStarted(HttpServletRequest request) { + public void requestStarted(HttpServletRequest request) { if (request.getAttribute(OUTPUT_FLASH_MAP_ATTRIBUTE) != null) { - return false; + return; } - - FlashMap outputFlashMap = new FlashMap(); - request.setAttribute(OUTPUT_FLASH_MAP_ATTRIBUTE, outputFlashMap); - Map inputFlashMap = getFlashMap(request); + Map inputFlashMap = lookupFlashMap(request); if (inputFlashMap != null) { request.setAttribute(INPUT_FLASH_MAP_ATTRIBUTE, inputFlashMap); } - + + FlashMap outputFlashMap = new FlashMap(this.hashCode()); + request.setAttribute(OUTPUT_FLASH_MAP_ATTRIBUTE, outputFlashMap); + removeExpiredFlashMaps(request); - - return true; } /** - * Return the flash attributes saved during the previous request if any. - * - * @return a read-only Map; or {@code null} if not found. + * Look up the "input" FlashMap by matching the target request path and + * the target request parameters configured in each available FlashMap + * to the current request. */ - private Map getFlashMap(HttpServletRequest request) { - List allMaps = retrieveFlashMaps(request, false); - if (CollectionUtils.isEmpty(allMaps)) { + private Map lookupFlashMap(HttpServletRequest request) { + List allFlashMaps = retrieveFlashMaps(request, false); + if (CollectionUtils.isEmpty(allFlashMaps)) { return null; } - if (logger.isDebugEnabled()) { - logger.debug("Looking up previous FlashMap among available FlashMaps: " + allMaps); + logger.debug("Retrieved flash maps: " + allFlashMaps); } - - List matches = new ArrayList(); - for (FlashMap flashMap : allMaps) { - if (flashMap.matches(request)) { - if (logger.isDebugEnabled()) { - logger.debug("Matched " + flashMap); - } - matches.add(flashMap); + List result = new ArrayList(); + for (FlashMap flashMap : allFlashMaps) { + if (isFlashMapForRequest(flashMap, request)) { + result.add(flashMap); } } - - if (!matches.isEmpty()) { - Collections.sort(matches); - FlashMap match = matches.remove(0); - allMaps.remove(match); + if (!result.isEmpty()) { + Collections.sort(result); + if (logger.isDebugEnabled()) { + logger.debug("Matching flash maps: " + result); + } + FlashMap match = result.remove(0); + allFlashMaps.remove(match); return Collections.unmodifiableMap(match); } - return null; } /** - * Retrieve the list of all FlashMap instances from the HTTP session. + * Compares the target request path and the target request parameters in the + * given FlashMap and returns "true" if they match. If the FlashMap does not + * have target request information, it matches any request. + */ + protected boolean isFlashMapForRequest(FlashMap flashMap, HttpServletRequest request) { + if (flashMap.getTargetRequestPath() != null) { + String requestUri = this.urlPathHelper.getRequestUri(request); + if (!requestUri.equals(flashMap.getTargetRequestPath()) + && !requestUri.equals(flashMap.getTargetRequestPath() + "/")) { + return false; + } + } + if (flashMap.getTargetRequestParams() != null) { + for (Map.Entry entry : flashMap.getTargetRequestParams().entrySet()) { + if (!entry.getValue().equals(request.getParameter(entry.getKey()))) { + return false; + } + } + } + return true; + } + + /** + * Retrieve all available FlashMap instances from the HTTP session. * @param request the current request - * @param allowCreate whether to create and the FlashMap container if not found - * @return a Map with all stored FlashMap instances; or {@code null} + * @param allowCreate whether to create and save the FlashMap in the session + * @return a Map with all FlashMap instances; or {@code null} */ @SuppressWarnings("unchecked") - private List retrieveFlashMaps(HttpServletRequest request, boolean allowCreate) { + protected List retrieveFlashMaps(HttpServletRequest request, boolean allowCreate) { HttpSession session = request.getSession(allowCreate); if (session == null) { return null; - } - - List allMaps = (List) session.getAttribute(FLASH_MAPS_SESSION_ATTRIBUTE); - if (allMaps == null && allowCreate) { - synchronized (DefaultFlashMapManager.class) { - allMaps = (List) session.getAttribute(FLASH_MAPS_SESSION_ATTRIBUTE); - if (allMaps == null) { - allMaps = new CopyOnWriteArrayList(); - session.setAttribute(FLASH_MAPS_SESSION_ATTRIBUTE, allMaps); + } + List allFlashMaps = (List) session.getAttribute(FLASH_MAPS_SESSION_ATTRIBUTE); + if (allFlashMaps == null && allowCreate) { + synchronized (this) { + allFlashMaps = (List) session.getAttribute(FLASH_MAPS_SESSION_ATTRIBUTE); + if (allFlashMaps == null) { + allFlashMaps = new CopyOnWriteArrayList(); + session.setAttribute(FLASH_MAPS_SESSION_ATTRIBUTE, allFlashMaps); } } } - - return allMaps; - } - - private void removeExpiredFlashMaps(HttpServletRequest request) { - List allMaps = retrieveFlashMaps(request, false); - if (allMaps != null && !allMaps.isEmpty()) { - List expiredMaps = new ArrayList(); - for (FlashMap flashMap : allMaps) { - if (flashMap.isExpired()) { - if (logger.isDebugEnabled()) { - logger.debug("Removing expired FlashMap: " + flashMap); - } - expiredMaps.add(flashMap); - } - } - allMaps.removeAll(expiredMaps); - } + return allFlashMaps; } /** - * {@inheritDoc} - * - *

The HTTP session is not created if the current FlashMap instance is empty. + * Iterate available FlashMap instances and remove the ones that have expired. */ + private void removeExpiredFlashMaps(HttpServletRequest request) { + List allMaps = retrieveFlashMaps(request, false); + if (CollectionUtils.isEmpty(allMaps)) { + return; + } + List expiredMaps = new ArrayList(); + for (FlashMap flashMap : allMaps) { + if (flashMap.isExpired()) { + if (logger.isDebugEnabled()) { + logger.debug("Removing expired FlashMap: " + flashMap); + } + expiredMaps.add(flashMap); + } + } + allMaps.removeAll(expiredMaps); + } + public void requestCompleted(HttpServletRequest request) { FlashMap flashMap = (FlashMap) request.getAttribute(OUTPUT_FLASH_MAP_ATTRIBUTE); if (flashMap == null) { throw new IllegalStateException( "Did not find a FlashMap exposed as the request attribute " + OUTPUT_FLASH_MAP_ATTRIBUTE); } - - if (!flashMap.isEmpty()) { + if (!flashMap.isEmpty() && flashMap.isCreatedBy(this.hashCode())) { if (logger.isDebugEnabled()) { logger.debug("Saving FlashMap=" + flashMap); } - List allFlashMaps = retrieveFlashMaps(request, true); + decodeAndNormalizeTargetPath(flashMap, request); flashMap.startExpirationPeriod(this.flashTimeout); - allFlashMaps.add(flashMap); + retrieveFlashMaps(request, true).add(flashMap); + } + } + + /** + * Ensure the target request path in the given FlashMap is decoded and also + * normalized (if it is relative) against the current request URL. + */ + private void decodeAndNormalizeTargetPath(FlashMap flashMap, HttpServletRequest request) { + String path = flashMap.getTargetRequestPath(); + if (path != null) { + path = urlPathHelper.decodeRequestString(request, path); + if (path.charAt(0) != '/') { + String requestUri = this.urlPathHelper.getRequestUri(request); + path = requestUri.substring(0, requestUri.lastIndexOf('/') + 1) + path; + path = StringUtils.cleanPath(path); + } + flashMap.setTargetRequestPath(path); } } diff --git a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/view/RedirectView.java b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/view/RedirectView.java index 2948f42ae3..247135c551 100644 --- a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/view/RedirectView.java +++ b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/view/RedirectView.java @@ -38,6 +38,7 @@ import org.springframework.beans.BeanUtils; import org.springframework.http.HttpStatus; import org.springframework.util.CollectionUtils; import org.springframework.util.ObjectUtils; +import org.springframework.util.StringUtils; import org.springframework.web.servlet.FlashMap; import org.springframework.web.servlet.HandlerMapping; import org.springframework.web.servlet.SmartView; @@ -261,24 +262,27 @@ public class RedirectView extends AbstractUrlBasedView implements SmartView { enc = WebUtils.DEFAULT_CHARACTER_ENCODING; } - UriTemplate uriTemplate = createUriTemplate(targetUrl, enc); - if (uriTemplate.getVariableNames().size() > 0) { - Map vars = new HashMap(); - vars.putAll(getCurrentUriVars(request)); - vars.putAll(model); - targetUrl = new StringBuilder(uriTemplate.expand(vars).toString()); - model = removeKeys(model, uriTemplate.getVariableNames()); + if (StringUtils.hasText(targetUrl)) { + UriTemplate uriTemplate = createUriTemplate(targetUrl, enc); + if (uriTemplate.getVariableNames().size() > 0) { + Map vars = new HashMap(); + vars.putAll(getCurrentUriVars(request)); + vars.putAll(model); + targetUrl = new StringBuilder(uriTemplate.expand(vars).toString()); + model = removeKeys(model, uriTemplate.getVariableNames()); + } } - + FlashMap flashMap = RequestContextUtils.getOutputFlashMap(request); if (!CollectionUtils.isEmpty(flashMap)) { - flashMap.setExpectedRequestUri(request, targetUrl.toString()); + String targetPath = WebUtils.extractUrlPath(targetUrl.toString()); + flashMap.setTargetRequestPath(targetPath); } if (this.exposeModelAttributes) { appendQueryProperties(targetUrl, model, enc); if (!CollectionUtils.isEmpty(flashMap)) { - flashMap.setExpectedRequestParams(model); + flashMap.addTargetRequestParams(model); } } diff --git a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/FlashMapTests.java b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/FlashMapTests.java index bdc0f7682e..01d0a97a0d 100644 --- a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/FlashMapTests.java +++ b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/FlashMapTests.java @@ -21,7 +21,6 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import org.junit.Test; -import org.springframework.mock.web.MockHttpServletRequest; /** * Test fixture for {@link FlashMap} tests. @@ -30,101 +29,6 @@ import org.springframework.mock.web.MockHttpServletRequest; */ public class FlashMapTests { - @Test - public void matchAnyUrlPath() { - FlashMap flashMap = new FlashMap(); - - assertTrue(flashMap.matches(new MockHttpServletRequest("GET", ""))); - assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/"))); - assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/yes"))); - } - - @Test - public void matchUrlPath() { - FlashMap flashMap = new FlashMap(); - flashMap.setExpectedRequestUri(null, "/yes"); - - assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/yes"))); - assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/yes/"))); - assertFalse(flashMap.matches(new MockHttpServletRequest("GET", "/yes/but"))); - assertFalse(flashMap.matches(new MockHttpServletRequest("GET", "/no"))); - - flashMap.setExpectedRequestUri(null, "/thats it?"); - - assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/thats it"))); - assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/thats%20it"))); - } - - @Test - public void matchRelativeUrlPath() { - FlashMap flashMap = new FlashMap(); - - flashMap.setExpectedRequestUri(new MockHttpServletRequest("GET", "/oh/no"), "yes"); - assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/oh/yes"))); - - flashMap.setExpectedRequestUri(new MockHttpServletRequest("GET", "/oh/not/again"), "../ok"); - assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/oh/ok"))); - - flashMap.setExpectedRequestUri(new MockHttpServletRequest("GET", "/yes/it/is"), ".."); - assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/yes"))); - - flashMap.setExpectedRequestUri(new MockHttpServletRequest("GET", "/yes/it/is"), "../"); - assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/yes"))); - - flashMap.setExpectedRequestUri(new MockHttpServletRequest("GET", "/thats it/really"), "./"); - assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/thats%20it"))); - - flashMap.setExpectedRequestUri(new MockHttpServletRequest("GET", "/yes/it/is"), "..?url=http://example.com"); - assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/yes"))); - } - - @Test - public void matchAbsoluteUrlPath() { - FlashMap flashMap = new FlashMap(); - flashMap.setExpectedRequestUri(new MockHttpServletRequest(), "http://example.com"); - - assertTrue(flashMap.matches(new MockHttpServletRequest("GET", ""))); - assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/"))); - assertFalse(flashMap.matches(new MockHttpServletRequest("GET", "/no"))); - - flashMap.setExpectedRequestUri(null, "http://example.com/"); - - assertTrue(flashMap.matches(new MockHttpServletRequest("GET", ""))); - assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/"))); - assertFalse(flashMap.matches(new MockHttpServletRequest("GET", "/no"))); - - flashMap.setExpectedRequestUri(null, "http://example.com/yes"); - - assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/yes"))); - assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/yes/"))); - assertFalse(flashMap.matches(new MockHttpServletRequest("GET", "/no"))); - assertFalse(flashMap.matches(new MockHttpServletRequest("GET", "/yes/no"))); - - flashMap.setExpectedRequestUri(null, "http://example.com/yes?a=1"); - - assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/yes"))); - } - - @Test - public void matchExpectedRequestParameter() { - String parameterName = "numero"; - - FlashMap flashMap = new FlashMap(); - flashMap.setExpectedRequestParam(parameterName, "uno"); - - MockHttpServletRequest request = new MockHttpServletRequest(); - assertFalse(flashMap.matches(request)); - - request.setParameter(parameterName, "uno"); - assertTrue(flashMap.matches(request)); - - request.setParameter(parameterName, "dos"); - assertFalse(flashMap.matches(request)); - - request.setParameter(parameterName, (String) null); - assertFalse(flashMap.matches(request)); - } - @Test public void isExpired() throws InterruptedException { FlashMap flashMap = new FlashMap(); @@ -152,18 +56,18 @@ public class FlashMapTests { FlashMap flashMap2 = new FlashMap(); assertEquals(0, flashMap1.compareTo(flashMap2)); - flashMap1.setExpectedRequestUri(null, "/path1"); + flashMap1.setTargetRequestPath("/path1"); assertEquals(-1, flashMap1.compareTo(flashMap2)); assertEquals(1, flashMap2.compareTo(flashMap1)); - flashMap2.setExpectedRequestUri(null, "/path2"); + flashMap2.setTargetRequestPath("/path2"); assertEquals(0, flashMap1.compareTo(flashMap2)); - flashMap1.setExpectedRequestParam("id", "1"); + flashMap1.addTargetRequestParam("id", "1"); assertEquals(-1, flashMap1.compareTo(flashMap2)); assertEquals(1, flashMap2.compareTo(flashMap1)); - flashMap2.setExpectedRequestParam("id", "2"); + flashMap2.addTargetRequestParam("id", "2"); assertEquals(0, flashMap1.compareTo(flashMap2)); } diff --git a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/support/DefaultFlashMapManagerTests.java b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/support/DefaultFlashMapManagerTests.java index 258c5b2ec5..e8cfc5046e 100644 --- a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/support/DefaultFlashMapManagerTests.java +++ b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/support/DefaultFlashMapManagerTests.java @@ -16,12 +16,14 @@ package org.springframework.web.servlet.support; + import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; +import static org.springframework.web.servlet.FlashMapManager.INPUT_FLASH_MAP_ATTRIBUTE; +import static org.springframework.web.servlet.FlashMapManager.OUTPUT_FLASH_MAP_ATTRIBUTE; import java.util.Collections; import java.util.List; @@ -51,55 +53,111 @@ public class DefaultFlashMapManagerTests { @Test public void requestStarted() { - boolean initialized = this.flashMapManager.requestStarted(this.request); - - assertTrue("Current FlashMap not initialized on first call", initialized); - assertNotNull("Current FlashMap not found", RequestContextUtils.getOutputFlashMap(request)); + this.flashMapManager.requestStarted(this.request); + FlashMap flashMap = RequestContextUtils.getOutputFlashMap(request); - initialized = this.flashMapManager.requestStarted(this.request); - - assertFalse("Current FlashMap initialized twice", initialized); + assertNotNull("Current FlashMap not found", flashMap); } @Test - public void lookupInputFlashMap() { + public void requestStartedAlready() { FlashMap flashMap = new FlashMap(); - flashMap.put("key", "value"); - - List allMaps = createFlashMapsSessionAttribute(); - allMaps.add(flashMap); - + this.request.setAttribute(OUTPUT_FLASH_MAP_ATTRIBUTE, flashMap); this.flashMapManager.requestStarted(this.request); - assertEquals(flashMap, request.getAttribute(DefaultFlashMapManager.INPUT_FLASH_MAP_ATTRIBUTE)); - assertEquals("Input FlashMap should have been removed", 0, allMaps.size()); + assertSame(flashMap, RequestContextUtils.getOutputFlashMap(request)); } @Test - public void lookupInputFlashMapExpectedUrlPath() { - FlashMap emptyFlashMap = new FlashMap(); - - FlashMap oneFlashMap = new FlashMap(); - oneFlashMap.setExpectedRequestUri(null, "/one"); - - FlashMap secondFlashMap = new FlashMap(); - secondFlashMap.setExpectedRequestUri(null, "/one/two"); + public void lookupFlashMapByPath() { + FlashMap flashMap = new FlashMap(); + flashMap.put("key", "value"); + flashMap.setTargetRequestPath("/path"); - List allMaps = createFlashMapsSessionAttribute(); - allMaps.add(emptyFlashMap); - allMaps.add(oneFlashMap); - allMaps.add(secondFlashMap); - Collections.shuffle(allMaps); - - this.request.setRequestURI("/one"); + List allMaps = createFlashMaps(); + allMaps.add(flashMap); + + this.request.setRequestURI("/path"); this.flashMapManager.requestStarted(this.request); - assertEquals(oneFlashMap, request.getAttribute(DefaultFlashMapManager.INPUT_FLASH_MAP_ATTRIBUTE)); + assertEquals(flashMap, RequestContextUtils.getInputFlashMap(this.request)); + assertEquals("Input FlashMap should have been removed", 0, getFlashMaps().size()); + } + + @Test + public void lookupFlashMapByPathWithTrailingSlash() { + FlashMap flashMap = new FlashMap(); + flashMap.put("key", "value"); + flashMap.setTargetRequestPath("/path"); + + List allMaps = createFlashMaps(); + allMaps.add(flashMap); + + this.request.setRequestURI("/path/"); + this.flashMapManager.requestStarted(this.request); + + assertEquals(flashMap, RequestContextUtils.getInputFlashMap(this.request)); + assertEquals("Input FlashMap should have been removed", 0, getFlashMaps().size()); + } + + @Test + public void lookupFlashMapWithParams() { + FlashMap flashMap = new FlashMap(); + flashMap.put("key", "value"); + flashMap.addTargetRequestParam("number", "one"); + + List allMaps = createFlashMaps(); + allMaps.add(flashMap); + + this.request.setParameter("number", (String) null); + this.flashMapManager.requestStarted(this.request); + + assertNull(RequestContextUtils.getInputFlashMap(this.request)); + assertEquals("FlashMap should have been removed", 1, getFlashMaps().size()); + + clearRequestAttributes(); + this.request.setParameter("number", "two"); + this.flashMapManager.requestStarted(this.request); + + assertNull(RequestContextUtils.getInputFlashMap(this.request)); + assertEquals("FlashMap should have been removed", 1, getFlashMaps().size()); + + clearRequestAttributes(); + this.request.setParameter("number", "one"); + this.flashMapManager.requestStarted(this.request); + + assertEquals(flashMap, RequestContextUtils.getInputFlashMap(this.request)); + assertEquals("Input FlashMap should have been removed", 0, getFlashMaps().size()); + } + + @Test + public void lookupFlashMapSortOrder() { + FlashMap emptyFlashMap = new FlashMap(); + + FlashMap flashMapOne = new FlashMap(); + flashMapOne.put("key1", "value1"); + flashMapOne.setTargetRequestPath("/one"); + + FlashMap flashMapTwo = new FlashMap(); + flashMapTwo.put("key1", "value1"); + flashMapTwo.put("key2", "value2"); + flashMapTwo.setTargetRequestPath("/one/two"); + + List allMaps = createFlashMaps(); + allMaps.add(emptyFlashMap); + allMaps.add(flashMapOne); + allMaps.add(flashMapTwo); + Collections.shuffle(allMaps); + + this.request.setRequestURI("/one/two"); + this.flashMapManager.requestStarted(this.request); + + assertEquals(flashMapTwo, request.getAttribute(INPUT_FLASH_MAP_ATTRIBUTE)); } @Test public void removeExpiredFlashMaps() throws InterruptedException { - List allMaps = createFlashMapsSessionAttribute(); + List allMaps = createFlashMaps(); for (int i=0; i < 5; i++) { FlashMap flashMap = new FlashMap(); allMaps.add(flashMap); @@ -114,17 +172,34 @@ public class DefaultFlashMapManagerTests { } @Test - public void saveFlashMap() throws InterruptedException { + public void saveFlashMapWithoutAttributes() throws InterruptedException { + this.flashMapManager.requestStarted(this.request); + this.flashMapManager.requestCompleted(this.request); + + assertNull(getFlashMaps()); + } + + @Test + public void saveFlashMapNotCreatedByThisManager() throws InterruptedException { FlashMap flashMap = new FlashMap(); + this.request.setAttribute(OUTPUT_FLASH_MAP_ATTRIBUTE, flashMap); + this.flashMapManager.requestCompleted(this.request); + + assertNull(getFlashMaps()); + } + + @Test + public void saveFlashMapWithAttributes() throws InterruptedException { + this.flashMapManager.requestStarted(this.request); + FlashMap flashMap = RequestContextUtils.getOutputFlashMap(this.request); flashMap.put("name", "value"); - request.setAttribute(DefaultFlashMapManager.OUTPUT_FLASH_MAP_ATTRIBUTE, flashMap); this.flashMapManager.setFlashMapTimeout(0); this.flashMapManager.requestCompleted(this.request); Thread.sleep(1); - List allMaps = getFlashMapsSessionAttribute(); + List allMaps = getFlashMaps(); assertNotNull(allMaps); assertSame(flashMap, allMaps.get(0)); @@ -132,22 +207,68 @@ public class DefaultFlashMapManagerTests { } @Test - public void saveFlashMapIsEmpty() throws InterruptedException { - request.setAttribute(DefaultFlashMapManager.OUTPUT_FLASH_MAP_ATTRIBUTE, new FlashMap()); + public void decodeTargetPath() throws InterruptedException { + this.flashMapManager.requestStarted(this.request); + FlashMap flashMap = RequestContextUtils.getOutputFlashMap(this.request); + flashMap.put("key", "value"); + + flashMap.setTargetRequestPath("/once%20upon%20a%20time"); + this.flashMapManager.requestCompleted(this.request); + + assertEquals("/once upon a time", flashMap.getTargetRequestPath()); + } + + @Test + public void normalizeTargetPath() throws InterruptedException { + this.flashMapManager.requestStarted(this.request); + FlashMap flashMap = RequestContextUtils.getOutputFlashMap(this.request); + flashMap.put("key", "value"); + + flashMap.setTargetRequestPath("."); + this.request.setRequestURI("/once/upon/a/time"); this.flashMapManager.requestCompleted(this.request); - assertNull(getFlashMapsSessionAttribute()); + assertEquals("/once/upon/a", flashMap.getTargetRequestPath()); + + flashMap.setTargetRequestPath("./"); + this.request.setRequestURI("/once/upon/a/time"); + this.flashMapManager.requestCompleted(this.request); + + assertEquals("/once/upon/a/", flashMap.getTargetRequestPath()); + + flashMap.setTargetRequestPath(".."); + this.request.setRequestURI("/once/upon/a/time"); + this.flashMapManager.requestCompleted(this.request); + + assertEquals("/once/upon", flashMap.getTargetRequestPath()); + + flashMap.setTargetRequestPath("../"); + this.request.setRequestURI("/once/upon/a/time"); + this.flashMapManager.requestCompleted(this.request); + + assertEquals("/once/upon/", flashMap.getTargetRequestPath()); + + flashMap.setTargetRequestPath("../../only"); + this.request.setRequestURI("/once/upon/a/time"); + this.flashMapManager.requestCompleted(this.request); + + assertEquals("/once/only", flashMap.getTargetRequestPath()); } @SuppressWarnings("unchecked") - private List getFlashMapsSessionAttribute() { + private List getFlashMaps() { return (List) this.request.getSession().getAttribute(DefaultFlashMapManager.class + ".FLASH_MAPS"); } - private List createFlashMapsSessionAttribute() { + private List createFlashMaps() { List allMaps = new CopyOnWriteArrayList(); this.request.getSession().setAttribute(DefaultFlashMapManager.class + ".FLASH_MAPS", allMaps); return allMaps; } + + private void clearRequestAttributes() { + request.removeAttribute(INPUT_FLASH_MAP_ATTRIBUTE); + request.removeAttribute(OUTPUT_FLASH_MAP_ATTRIBUTE); + } } diff --git a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/view/RedirectViewTests.java b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/view/RedirectViewTests.java index 30081ae85a..3cb570ceb3 100644 --- a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/view/RedirectViewTests.java +++ b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/view/RedirectViewTests.java @@ -16,18 +16,25 @@ package org.springframework.web.servlet.view; +import static org.easymock.EasyMock.createMock; +import static org.easymock.EasyMock.createNiceMock; +import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.replay; +import static org.easymock.EasyMock.verify; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; + import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import junit.framework.AssertionFailedError; -import static org.easymock.EasyMock.*; -import static org.junit.Assert.*; -import org.junit.Test; +import org.junit.Test; import org.springframework.beans.TestBean; import org.springframework.http.HttpStatus; import org.springframework.mock.web.MockHttpServletRequest; @@ -116,13 +123,13 @@ public class RedirectViewTests { FlashMap flashMap = new FlashMap(); flashMap.put("successMessage", "yay!"); request.setAttribute(FlashMapManager.OUTPUT_FLASH_MAP_ATTRIBUTE, flashMap); - rv.render(new ModelMap("id", "1"), request, response); + ModelMap model = new ModelMap("id", "1"); + rv.render(model, request, response); assertEquals(303, response.getStatus()); assertEquals("http://url.somewhere.com/path?id=1", response.getHeader("Location")); - MockHttpServletRequest nextRequest = new MockHttpServletRequest("GET", "/path"); - nextRequest.addParameter("id", "1"); - assertTrue(flashMap.matches(nextRequest)); + assertEquals("/path", flashMap.getTargetRequestPath()); + assertEquals(model, flashMap.getTargetRequestParams()); } @Test diff --git a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/view/RedirectViewUriTemplateTests.java b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/view/RedirectViewUriTemplateTests.java index c573a50e99..bd8b8d4a1b 100644 --- a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/view/RedirectViewUriTemplateTests.java +++ b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/view/RedirectViewUriTemplateTests.java @@ -94,4 +94,14 @@ public class RedirectViewUriTemplateTests { assertEquals(url + "/value1/v1/value2?key3=value3", response.getRedirectedUrl()); } + @Test + public void emptyRedirectString() throws Exception { + Map model = new HashMap(); + + RedirectView redirectView = new RedirectView(""); + redirectView.renderMergedOutputModel(model, request, response); + + assertEquals("", response.getRedirectedUrl()); + } + } diff --git a/org.springframework.web/src/main/java/org/springframework/web/util/WebUtils.java b/org.springframework.web/src/main/java/org/springframework/web/util/WebUtils.java index 8e19d70926..b2e2d22258 100644 --- a/org.springframework.web/src/main/java/org/springframework/web/util/WebUtils.java +++ b/org.springframework.web/src/main/java/org/springframework/web/util/WebUtils.java @@ -21,6 +21,7 @@ import java.io.FileNotFoundException; import java.util.Enumeration; import java.util.Map; import java.util.TreeMap; + import javax.servlet.ServletContext; import javax.servlet.ServletRequest; import javax.servlet.ServletRequestWrapper; @@ -723,4 +724,29 @@ public abstract class WebUtils { return urlPath.substring(begin, end); } + /** + * Extracts the path from the given URL by removing the query at the end + * and the scheme and authority in the front, if present. + * @param url a URL, never {@code null} + * @return the extracted URL path + */ + public static String extractUrlPath(String url) { + // Remove query/fragment + int end = url.indexOf('?'); + if (end == -1) { + end = url.indexOf('#'); + if (end == -1) { + end = url.length(); + } + } + url = url.substring(0, end); + // Remove scheme + authority + int start = url.indexOf("://"); + if (start != -1) { + start = url.indexOf('/', start + 3); + url = (start != -1 ) ? url.substring(start) : ""; + } + return url; + } + } diff --git a/org.springframework.web/src/test/java/org/springframework/web/util/WebUtilsTests.java b/org.springframework.web/src/test/java/org/springframework/web/util/WebUtilsTests.java index 616dc66686..cf33f29b64 100644 --- a/org.springframework.web/src/test/java/org/springframework/web/util/WebUtilsTests.java +++ b/org.springframework.web/src/test/java/org/springframework/web/util/WebUtilsTests.java @@ -64,4 +64,15 @@ public class WebUtilsTests { assertEquals("view.html", WebUtils.extractFullFilenameFromUrlPath("/products/view.html?param=/path/a.do")); } + @Test + public void extractUriPath() { + assertEquals("", WebUtils.extractUrlPath("http://example.com")); + assertEquals("/", WebUtils.extractUrlPath("http://example.com/")); + assertEquals("/rfc/rfc3986.txt", WebUtils.extractUrlPath("http://www.ietf.org/rfc/rfc3986.txt")); + assertEquals("/over/there", WebUtils.extractUrlPath("http://example.com/over/there?name=ferret#nose")); + assertEquals("/over/there", WebUtils.extractUrlPath("http://example.com/over/there#nose")); + assertEquals("/over/there", WebUtils.extractUrlPath("/over/there?name=ferret#nose")); + assertEquals("/over/there", WebUtils.extractUrlPath("/over/there?url=http://example.com")); + } + }