From 57307a0b2e5bce8f70d5deddf8df11d034dc8c5a Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Thu, 17 May 2012 15:54:16 -0400 Subject: [PATCH] Decode path vars when url decoding is turned off When URL decoding is turned off in AbstractHandlerMapping, the extracted path variables are also not encoded. Turning off URL decoding may be necessary for request mapping to work correctly when the path may contain the (encoded) special character '/'. At the same time there is no good reason not to leave path variables encoded. This change ensures path variables are encoded when URL decoding is turned off. Issue: SPR-9098 --- .../web/util/UrlPathHelper.java | 57 ++++++++++++++----- .../RequestMappingInfoHandlerMapping.java | 7 ++- ...RequestMappingInfoHandlerMappingTests.java | 56 ++++++++++++------ src/dist/changelog.txt | 1 + 4 files changed, 88 insertions(+), 33 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/util/UrlPathHelper.java b/spring-web/src/main/java/org/springframework/web/util/UrlPathHelper.java index 65a167bbe59..8b134869611 100644 --- a/spring-web/src/main/java/org/springframework/web/util/UrlPathHelper.java +++ b/spring-web/src/main/java/org/springframework/web/util/UrlPathHelper.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2010 the original author or authors. + * Copyright 2002-2012 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. @@ -18,12 +18,15 @@ package org.springframework.web.util; import java.io.UnsupportedEncodingException; import java.net.URLDecoder; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.Map.Entry; import java.util.Properties; + import javax.servlet.http.HttpServletRequest; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; - import org.springframework.util.StringUtils; /** @@ -37,6 +40,7 @@ import org.springframework.util.StringUtils; * * @author Juergen Hoeller * @author Rob Harrop + * @author Rossen Stoyanchev * @since 14.01.2004 */ public class UrlPathHelper { @@ -301,7 +305,7 @@ public class UrlPathHelper { * @return the query string */ public String getOriginatingQueryString(HttpServletRequest request) { - if ((request.getAttribute(WebUtils.FORWARD_REQUEST_URI_ATTRIBUTE) != null) || + if ((request.getAttribute(WebUtils.FORWARD_REQUEST_URI_ATTRIBUTE) != null) || (request.getAttribute(WebUtils.ERROR_REQUEST_URI_ATTRIBUTE) != null)) { return (String) request.getAttribute(WebUtils.FORWARD_QUERY_STRING_ATTRIBUTE); } @@ -333,21 +337,25 @@ public class UrlPathHelper { */ public String decodeRequestString(HttpServletRequest request, String source) { if (this.urlDecode) { - String enc = determineEncoding(request); - try { - return UriUtils.decode(source, enc); - } - catch (UnsupportedEncodingException ex) { - if (logger.isWarnEnabled()) { - logger.warn("Could not decode request string [" + source + "] with encoding '" + enc + - "': falling back to platform default encoding; exception message: " + ex.getMessage()); - } - return URLDecoder.decode(source); - } + return decodeInternal(request, source); } return source; } + private String decodeInternal(HttpServletRequest request, String source) { + String enc = determineEncoding(request); + try { + return UriUtils.decode(source, enc); + } + catch (UnsupportedEncodingException ex) { + if (logger.isWarnEnabled()) { + logger.warn("Could not decode request string [" + source + "] with encoding '" + enc + + "': falling back to platform default encoding; exception message: " + ex.getMessage()); + } + return URLDecoder.decode(source); + } + } + /** * Determine the encoding for the given request. * Can be overridden in subclasses. @@ -366,6 +374,27 @@ public class UrlPathHelper { return enc; } + /** + * Decode the given URI path variables via {@link #decodeRequestString(HttpServletRequest, String)} + * unless {@link #setUrlDecode(boolean)} is set to {@code true} in which case + * it is assumed the URL path from which the variables were extracted is + * already decoded through a call to {@link #getLookupPathForRequest(HttpServletRequest)}. + * @param request current HTTP request + * @param vars URI variables extracted from the URL path + * @return the same Map or a new Map instance + */ + public Map decodePathVariables(HttpServletRequest request, Map vars) { + if (this.urlDecode) { + return vars; + } + else { + Map decodedVars = new LinkedHashMap(vars.size()); + for (Entry entry : vars.entrySet()) { + decodedVars.put(entry.getKey(), decodeInternal(request, entry.getValue())); + } + return decodedVars; + } + } private boolean shouldRemoveTrailingServletPathSlash(HttpServletRequest request) { if (request.getAttribute(WEBSPHERE_URI_ATTRIBUTE) == null) { diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMapping.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMapping.java index b206b10c0fe..b27f49c4372 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMapping.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMapping.java @@ -88,9 +88,10 @@ public abstract class RequestMappingInfoHandlerMapping extends AbstractHandlerMe Set patterns = info.getPatternsCondition().getPatterns(); String bestPattern = patterns.isEmpty() ? lookupPath : patterns.iterator().next(); request.setAttribute(BEST_MATCHING_PATTERN_ATTRIBUTE, bestPattern); - - Map uriTemplateVariables = getPathMatcher().extractUriTemplateVariables(bestPattern, lookupPath); - request.setAttribute(HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE, uriTemplateVariables); + + Map vars = getPathMatcher().extractUriTemplateVariables(bestPattern, lookupPath); + Map decodedVars = getUrlPathHelper().decodePathVariables(request, vars); + request.setAttribute(HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE, decodedVars); if (!info.getProducesCondition().getProducibleMediaTypes().isEmpty()) { Set mediaTypes = info.getProducesCondition().getProducibleMediaTypes(); diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMappingTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMappingTests.java index d6562766e76..36a3e3c778f 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMappingTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMappingTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2011 the original author or authors. + * Copyright 2002-2012 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. @@ -131,7 +131,7 @@ public class RequestMappingInfoHandlerMappingTests { HandlerMethod hm = (HandlerMethod) this.mapping.getHandler(request).getHandler(); assertEquals(this.fooParamMethod.getMethod(), hm.getMethod()); } - + @Test public void requestMethodNotAllowed() throws Exception { try { @@ -143,7 +143,7 @@ public class RequestMappingInfoHandlerMappingTests { assertArrayEquals("Invalid supported methods", new String[]{"GET", "HEAD"}, ex.getSupportedMethods()); } } - + @Test public void mediaTypeNotSupported() throws Exception { testMediaTypeNotSupported("/person/1"); @@ -159,11 +159,11 @@ public class RequestMappingInfoHandlerMappingTests { fail("HttpMediaTypeNotSupportedException expected"); } catch (HttpMediaTypeNotSupportedException ex) { - assertEquals("Invalid supported consumable media types", + assertEquals("Invalid supported consumable media types", Arrays.asList(new MediaType("application", "xml")), ex.getSupportedMediaTypes()); } } - + @Test public void mediaTypeNotAccepted() throws Exception { testMediaTypeNotAccepted("/persons"); @@ -179,7 +179,7 @@ public class RequestMappingInfoHandlerMappingTests { fail("HttpMediaTypeNotAcceptableException expected"); } catch (HttpMediaTypeNotAcceptableException ex) { - assertEquals("Invalid supported producible media types", + assertEquals("Invalid supported producible media types", Arrays.asList(new MediaType("application", "xml")), ex.getSupportedMediaTypes()); } } @@ -191,17 +191,41 @@ public class RequestMappingInfoHandlerMappingTests { MockHttpServletRequest request = new MockHttpServletRequest("GET", "/1/2"); String lookupPath = new UrlPathHelper().getLookupPathForRequest(request); this.mapping.handleMatch(key, lookupPath, request); - + @SuppressWarnings("unchecked") - Map uriVariables = + Map uriVariables = (Map) request.getAttribute( HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE); - + assertNotNull(uriVariables); assertEquals("1", uriVariables.get("path1")); assertEquals("2", uriVariables.get("path2")); } + // SPR-9098 + + @Test + public void uriTemplateVariablesDecode() { + PatternsRequestCondition patterns = new PatternsRequestCondition("/{group}/{identifier}"); + RequestMappingInfo key = new RequestMappingInfo(patterns, null, null, null, null, null, null); + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/group/a%2Fb"); + + UrlPathHelper pathHelper = new UrlPathHelper(); + pathHelper.setUrlDecode(false); + String lookupPath = pathHelper.getLookupPathForRequest(request); + + this.mapping.setUrlPathHelper(pathHelper); + this.mapping.handleMatch(key, lookupPath, request); + + @SuppressWarnings("unchecked") + Map uriVariables = + (Map) request.getAttribute(HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE); + + assertNotNull(uriVariables); + assertEquals("group", uriVariables.get("group")); + assertEquals("a/b", uriVariables.get("identifier")); + } + @Test public void bestMatchingPatternAttribute() { PatternsRequestCondition patterns = new PatternsRequestCondition("/{path1}/2", "/**"); @@ -209,7 +233,7 @@ public class RequestMappingInfoHandlerMappingTests { MockHttpServletRequest request = new MockHttpServletRequest("GET", "/1/2"); this.mapping.handleMatch(key, "/1/2", request); - + assertEquals("/{path1}/2", request.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE)); } @@ -220,7 +244,7 @@ public class RequestMappingInfoHandlerMappingTests { MockHttpServletRequest request = new MockHttpServletRequest("GET", "/1/2"); this.mapping.handleMatch(key, "/1/2", request); - + assertEquals("/1/2", request.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE)); } @@ -237,10 +261,10 @@ public class RequestMappingInfoHandlerMappingTests { request.addHeader("Accept", "application/json"); this.mapping.getHandler(request); - assertNull("Negated expression should not be listed as a producible type", + assertNull("Negated expression should not be listed as a producible type", request.getAttribute(HandlerMapping.PRODUCIBLE_MEDIA_TYPES_ATTRIBUTE)); } - + @Test public void mappedInterceptors() throws Exception { String path = "/foo"; @@ -268,7 +292,7 @@ public class RequestMappingInfoHandlerMappingTests { @RequestMapping(value = "/foo", method = RequestMethod.GET) public void foo() { } - + @RequestMapping(value = "/foo", method = RequestMethod.GET, params="p") public void fooParam() { } @@ -306,7 +330,7 @@ public class RequestMappingInfoHandlerMappingTests { public void registerHandler(Object handler) { super.detectHandlerMethods(handler); } - + @Override protected boolean isHandler(Class beanType) { return AnnotationUtils.findAnnotation(beanType, RequestMapping.class) != null; @@ -329,5 +353,5 @@ public class RequestMappingInfoHandlerMappingTests { } } } - + } \ No newline at end of file diff --git a/src/dist/changelog.txt b/src/dist/changelog.txt index 1fe61e300ca..5d271edffe3 100644 --- a/src/dist/changelog.txt +++ b/src/dist/changelog.txt @@ -27,6 +27,7 @@ Changes in version 3.2 M1 * add CompositeRequestCondition for use with multiple custom request mapping conditions * add ability to configure custom MessageCodesResolver through the MVC Java config * add option in MappingJacksonJsonView for setting the Content-Length header +* decode path variables when url decoding is turned off in AbstractHandlerMapping Changes in version 3.1.1 (2012-02-16) -------------------------------------