SPR-6164 Add option to disable '.*' pattern matching in RequestMappingHandlerMapping and PatternsRequestCondition

git-svn-id: https://src.springframework.org/svn/spring-framework/trunk@4599 50f2f4bb-b051-0410-bef5-90022cba6387

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/condition/ConsumesRequestCondition.java

@@ -44,7 +44,7 @@ import org.springframework.web.servlet.mvc.condition.HeadersRequestCondition.Hea
  * @author Rossen Stoyanchev
  * @since 3.1
  */
-public class ConsumesRequestCondition extends AbstractRequestCondition<ConsumesRequestCondition> {
+public final class ConsumesRequestCondition extends AbstractRequestCondition<ConsumesRequestCondition> {
 
 	private final List<ConsumeMediaTypeExpression> expressions;
 

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/condition/HeadersRequestCondition.java

@@ -39,7 +39,7 @@ import org.springframework.web.bind.annotation.RequestMapping;
  * @author Rossen Stoyanchev
  * @since 3.1
  */
-public class HeadersRequestCondition extends AbstractRequestCondition<HeadersRequestCondition> {
+public final class HeadersRequestCondition extends AbstractRequestCondition<HeadersRequestCondition> {
 
 	private final Set<HeaderExpression> expressions;
 

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/condition/ParamsRequestCondition.java

@@ -36,7 +36,7 @@ import org.springframework.web.util.WebUtils;
  * @author Rossen Stoyanchev
  * @since 3.1
  */
-public class ParamsRequestCondition extends AbstractRequestCondition<ParamsRequestCondition> {
+public final class ParamsRequestCondition extends AbstractRequestCondition<ParamsRequestCondition> {
 
 	private final Set<ParamExpression> expressions;
 	

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/condition/PatternsRequestCondition.java

@@ -41,7 +41,7 @@ import org.springframework.web.util.UrlPathHelper;
  * @author Rossen Stoyanchev
  * @since 3.1
  */
-public class PatternsRequestCondition extends AbstractRequestCondition<PatternsRequestCondition> {
+public final class PatternsRequestCondition extends AbstractRequestCondition<PatternsRequestCondition> {
 	
 	private final Set<String> patterns; 
 
@@ -49,14 +49,7 @@ public class PatternsRequestCondition extends AbstractRequestCondition<PatternsR
 	
 	private final PathMatcher pathMatcher;
 
-	/**
-	 * Creates a new {@link PatternsRequestCondition} with the given URL patterns.
-	 * Each pattern that is not empty and does not start with "/" is prepended with "/".
-	 * @param patterns 0 or more URL patterns; if 0 the condition will match to every request. 
-	 */
-	public PatternsRequestCondition(String... patterns) {
-		this(patterns, new UrlPathHelper(), new AntPathMatcher());
-	}
+	private final boolean useSuffixPatternMatch;
 
 	/**
 	 * Creates a new {@link PatternsRequestCondition} with the given URL patterns. 
@@ -65,9 +58,22 @@ public class PatternsRequestCondition extends AbstractRequestCondition<PatternsR
 	 * @param patterns the URL patterns to use; if 0, the condition will match to every request. 
 	 * @param urlPathHelper a {@link UrlPathHelper} for determining the lookup path for a request
 	 * @param pathMatcher a {@link PathMatcher} for pattern path matching
+	 * @param useSuffixPatternMatch whether to enable matching by suffix (".*")
+	 */
+	public PatternsRequestCondition(String[] patterns, 
+									UrlPathHelper urlPathHelper, 
+									PathMatcher pathMatcher, 
+									boolean useSuffixPatternMatch) {
+		this(asList(patterns), urlPathHelper, pathMatcher, useSuffixPatternMatch);
+	}
+
+	/**
+	 * Creates a new {@link PatternsRequestCondition} with the given URL patterns.
+	 * Each pattern that is not empty and does not start with "/" is prepended with "/".
+	 * @param patterns 0 or more URL patterns; if 0 the condition will match to every request. 
 	 */
-	public PatternsRequestCondition(String[] patterns, UrlPathHelper urlPathHelper, PathMatcher pathMatcher) {
-		this(asList(patterns), urlPathHelper, pathMatcher);
+	public PatternsRequestCondition(String... patterns) {
+		this(patterns, null, null, true);
 	}
 
 	private static List<String> asList(String... patterns) {
@@ -77,10 +83,14 @@ public class PatternsRequestCondition extends AbstractRequestCondition<PatternsR
 	/**
 	 * Private constructor.
 	 */
-	private PatternsRequestCondition(Collection<String> patterns, UrlPathHelper urlPathHelper, PathMatcher pathMatcher) {
+	private PatternsRequestCondition(Collection<String> patterns, 
+									 UrlPathHelper urlPathHelper, 
+									 PathMatcher pathMatcher, 
+									 boolean useSuffixPatternMatch) {
 		this.patterns = Collections.unmodifiableSet(prependLeadingSlash(patterns));
-		this.urlPathHelper = urlPathHelper;
-		this.pathMatcher = pathMatcher;
+		this.urlPathHelper = urlPathHelper != null ? urlPathHelper : new UrlPathHelper();
+		this.pathMatcher = pathMatcher != null ? pathMatcher : new AntPathMatcher();
+		this.useSuffixPatternMatch = useSuffixPatternMatch;
 	}
 
 	private static Set<String> prependLeadingSlash(Collection<String> patterns) {
@@ -139,7 +149,7 @@ public class PatternsRequestCondition extends AbstractRequestCondition<PatternsR
 		else {
 			result.add("");
 		}
-		return new PatternsRequestCondition(result, urlPathHelper, pathMatcher);
+		return new PatternsRequestCondition(result, urlPathHelper, pathMatcher, useSuffixPatternMatch);
 	}
 
 	/**
@@ -172,16 +182,19 @@ public class PatternsRequestCondition extends AbstractRequestCondition<PatternsR
 			}
 		}
 		Collections.sort(matches, pathMatcher.getPatternComparator(lookupPath));
-		return matches.isEmpty() ? null : new PatternsRequestCondition(matches, urlPathHelper, pathMatcher);
+		return matches.isEmpty() ? null : 
+			new PatternsRequestCondition(matches, urlPathHelper, pathMatcher, useSuffixPatternMatch);
 	}
 
 	private String getMatchingPattern(String pattern, String lookupPath) {
 		if (pattern.equals(lookupPath)) {
 			return pattern;
 		}
-		boolean hasSuffix = pattern.indexOf('.') != -1;
-		if (!hasSuffix && pathMatcher.match(pattern + ".*", lookupPath)) {
-			return pattern + ".*";
+		if (useSuffixPatternMatch) {
+			boolean hasSuffix = pattern.indexOf('.') != -1;
+			if (!hasSuffix && pathMatcher.match(pattern + ".*", lookupPath)) {
+				return pattern + ".*";
+			}
 		}
 		if (pathMatcher.match(pattern, lookupPath)) {
 			return pattern;

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/condition/ProducesRequestCondition.java

@@ -44,7 +44,7 @@ import org.springframework.web.servlet.mvc.condition.HeadersRequestCondition.Hea
  * @author Rossen Stoyanchev
  * @since 3.1
  */
-public class ProducesRequestCondition extends AbstractRequestCondition<ProducesRequestCondition> {
+public final class ProducesRequestCondition extends AbstractRequestCondition<ProducesRequestCondition> {
 
 	private final List<ProduceMediaTypeExpression> expressions;
 

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/condition/RequestMethodsRequestCondition.java

@@ -36,7 +36,7 @@ import org.springframework.web.bind.annotation.RequestMethod;
  * @author Rossen Stoyanchev
  * @since 3.1
  */
-public class RequestMethodsRequestCondition extends AbstractRequestCondition<RequestMethodsRequestCondition> {
+public final class RequestMethodsRequestCondition extends AbstractRequestCondition<RequestMethodsRequestCondition> {
 
 	private final Set<RequestMethod> methods;
 

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfo.java

@@ -37,7 +37,7 @@ import org.springframework.web.servlet.mvc.condition.RequestMethodsRequestCondit
  * 	<li>{@link ProducesRequestCondition}</li>
  * </ul>
  * 
- * Optionally a custom request condition may also be provided. 
+ * Optionally a custom request condition may be provided. 
  * 
  * @author Arjen Poutsma
  * @author Rossen Stoyanchev
@@ -80,6 +80,14 @@ public final class RequestMappingInfo {
 		this.customCondition = custom != null ? new CustomRequestCondition(custom) : new CustomRequestCondition();
 	}
 
+	/**
+	 * Re-create a {@link RequestMappingInfo} with the given custom {@link RequestCondition}.
+	 */
+	public RequestMappingInfo(RequestMappingInfo info, RequestCondition<?> customRequestCondition) {
+		this(info.patternsCondition, info.methodsCondition, info.paramsCondition, info.headersCondition,
+				info.consumesCondition, info.producesCondition, customRequestCondition);
+	}
+
 	/**
 	 * Returns the URL patterns of this {@link RequestMappingInfo}; 
 	 * or instance with 0 patterns, never {@code null}

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMapping.java

@@ -20,13 +20,13 @@ import java.lang.reflect.Method;
 
 import org.springframework.core.annotation.AnnotationUtils;
 import org.springframework.stereotype.Controller;
-import org.springframework.util.PathMatcher;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.servlet.mvc.condition.ConsumesRequestCondition;
 import org.springframework.web.servlet.mvc.condition.HeadersRequestCondition;
 import org.springframework.web.servlet.mvc.condition.ParamsRequestCondition;
 import org.springframework.web.servlet.mvc.condition.PatternsRequestCondition;
 import org.springframework.web.servlet.mvc.condition.ProducesRequestCondition;
+import org.springframework.web.servlet.mvc.condition.RequestCondition;
 import org.springframework.web.servlet.mvc.condition.RequestMethodsRequestCondition;
 import org.springframework.web.servlet.mvc.method.RequestMappingInfo;
 import org.springframework.web.servlet.mvc.method.RequestMappingInfoHandlerMapping;
@@ -41,9 +41,28 @@ import org.springframework.web.servlet.mvc.method.RequestMappingInfoHandlerMappi
  */
 public class RequestMappingHandlerMapping extends RequestMappingInfoHandlerMapping {
 
+	private boolean useSuffixPatternMatch = true;
+
+	/**
+	 * Set whether to use a suffix pattern match (".*") when matching patterns to URLs.
+	 * If enabled a method mapped to "/users" will also match to "/users.*".
+	 * <p>Default is "true". Turn this convention off if you intend to interpret path mappings strictly. 
+	 */
+	public void setUseSuffixPatternMatch(boolean useSuffixPatternMatch) {
+		this.useSuffixPatternMatch = useSuffixPatternMatch;
+	}
+
+	/**
+	 * Returns the value of the useSuffixPatternMatch flag, see {@link #setUseSuffixPatternMatch(boolean)}.
+	 */
+	public boolean isUseSuffixPatternMatch() {
+		return useSuffixPatternMatch;
+	}
+
 	/**
-	 * {@inheritDoc} The handler determination in this method is made based on the presence of a type-level {@link
-	 * Controller} annotation.
+	 * {@inheritDoc} 
+	 * The default implementation checks for the presence of a type-level {@link Controller} 
+	 * annotation via {@link AnnotationUtils#findAnnotation(Class, Class)}.
 	 */
 	@Override
 	protected boolean isHandler(Class<?> beanType) {
@@ -51,40 +70,54 @@ public class RequestMappingHandlerMapping extends RequestMappingInfoHandlerMappi
 	}
 
 	/**
-	 * Provides a {@link RequestMappingInfo} for the given method. <p>Only {@link RequestMapping @RequestMapping}-annotated
-	 * methods are considered. Type-level {@link RequestMapping @RequestMapping} annotations are also detected and their
-	 * attributes combined with method-level {@link RequestMapping @RequestMapping} attributes.
+	 * Determines if the given method is a handler method and creates a {@link RequestMappingInfo} for it. 
+	 * 
+	 * <p>The default implementation expects the presence of a method-level @{@link RequestMapping} 
+	 * annotation via {@link AnnotationUtils#findAnnotation(Class, Class)}. The presence of 
+	 * type-level annotations is also checked and if present a RequestMappingInfo is created for each type- 
+	 * and method-level annotations and combined via {@link RequestMappingInfo#combine(RequestMappingInfo)}.
 	 *
-	 * @param method the method to create a mapping for
+	 * @param method the method to create a RequestMappingInfo for
 	 * @param handlerType the actual handler type, possibly a sub-type of {@code method.getDeclaringClass()}
-	 * @return the mapping, or {@code null}
-	 * @see RequestMappingInfo#combine(RequestMappingInfo, PathMatcher)
+	 * @return the info, or {@code null}
 	 */
 	@Override
 	protected RequestMappingInfo getMappingForMethod(Method method, Class<?> handlerType) {
-		RequestMapping methodAnnotation = AnnotationUtils.findAnnotation(method, RequestMapping.class);
-		if (methodAnnotation == null) {
-			return null;
-		}
-		RequestMappingInfo methodInfo = createFromRequestMapping(methodAnnotation);
-		RequestMapping typeAnnotation = AnnotationUtils.findAnnotation(handlerType, RequestMapping.class);
-		if (typeAnnotation != null) {
-			RequestMappingInfo typeInfo = createFromRequestMapping(typeAnnotation);
-			return typeInfo.combine(methodInfo);
-		}
-		else {
-			return methodInfo;
+		RequestMapping methodAnnot = AnnotationUtils.findAnnotation(method, RequestMapping.class);
+		if (methodAnnot != null) {
+			RequestMapping typeAnnot = AnnotationUtils.findAnnotation(handlerType, RequestMapping.class);
+			RequestMappingInfo methodInfo = createRequestMappingInfo(methodAnnot, handlerType, method);
+			if (typeAnnot != null) {
+				RequestMappingInfo typeInfo = createRequestMappingInfo(typeAnnot, handlerType, method);
+				return typeInfo.combine(methodInfo);
+			}
+			else {
+				return methodInfo;
+			}
 		}
+		return null;
 	}
 
-	private RequestMappingInfo createFromRequestMapping(RequestMapping annotation) {
+	/**
+	 * Override this method to create a {@link RequestMappingInfo} from a @{@link RequestMapping} annotation. The main 
+	 * reason for doing so is to provide a custom {@link RequestCondition} to the RequestMappingInfo constructor. 
+	 * 
+	 * <p>This method is invoked both for type- and method-level @{@link RequestMapping} annotations. The resulting 
+	 * {@link RequestMappingInfo}s are combined via {@link RequestMappingInfo#combine(RequestMappingInfo)}.
+	 * 
+	 * @param annot a type- or a method-level {@link RequestMapping} annotation
+	 * @param handlerType the handler type
+	 * @param method the method with which the created RequestMappingInfo will be combined
+	 * @return a {@link RequestMappingInfo} instance; never {@code null}
+	 */
+	protected RequestMappingInfo createRequestMappingInfo(RequestMapping annot, Class<?> handlerType, Method method) {
 		return new RequestMappingInfo(
-				new PatternsRequestCondition(annotation.value(), getUrlPathHelper(), getPathMatcher()),
-				new RequestMethodsRequestCondition(annotation.method()),
-				new ParamsRequestCondition(annotation.params()),
-				new HeadersRequestCondition(annotation.headers()),
-				new ConsumesRequestCondition(annotation.consumes(), annotation.headers()),
-				new ProducesRequestCondition(annotation.produces(), annotation.headers()), null);
+				new PatternsRequestCondition(annot.value(), getUrlPathHelper(), getPathMatcher(), useSuffixPatternMatch),
+				new RequestMethodsRequestCondition(annot.method()),
+				new ParamsRequestCondition(annot.params()),
+				new HeadersRequestCondition(annot.headers()),
+				new ConsumesRequestCondition(annot.consumes(), annot.headers()),
+				new ProducesRequestCondition(annot.produces(), annot.headers()), null);
 	}
-	
+
 }

org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/condition/PatternsRequestConditionTests.java

@@ -97,21 +97,39 @@ public class PatternsRequestConditionTests {
 	}
 
 	@Test
-	public void matchImplicitByExtension() { 
-		PatternsRequestCondition condition = new PatternsRequestCondition("/foo");
-		PatternsRequestCondition match = condition.getMatchingCondition(new MockHttpServletRequest("GET", "/foo.html"));
+	public void matchSuffixPattern() { 
+		MockHttpServletRequest request = new MockHttpServletRequest("GET", "/foo.html");
+		
+		PatternsRequestCondition condition = new PatternsRequestCondition("/{foo}");
+		PatternsRequestCondition match = condition.getMatchingCondition(request);
+
+		assertNotNull(match);
+		assertEquals("/{foo}.*", match.getPatterns().iterator().next());
+
+		condition = new PatternsRequestCondition(new String[] {"/{foo}"}, null, null, false);
+		match = condition.getMatchingCondition(request);
 
 		assertNotNull(match);
-		assertEquals("/foo.*", match.getPatterns().iterator().next());
+		assertEquals("/{foo}", match.getPatterns().iterator().next());
 	}
 
 	@Test
-	public void matchImplicitTrailingSlash() {
+	public void matchTrailingSlash() {
+		MockHttpServletRequest request = new MockHttpServletRequest("GET", "/foo/");
+		
 		PatternsRequestCondition condition = new PatternsRequestCondition("/foo");
-		PatternsRequestCondition match = condition.getMatchingCondition(new MockHttpServletRequest("GET", "/foo/"));
+		PatternsRequestCondition match = condition.getMatchingCondition(request);
 
 		assertNotNull(match);
 		assertEquals("/foo/", match.getPatterns().iterator().next());
+		
+		boolean useSuffixPatternMatch = false;
+		condition = new PatternsRequestCondition(new String[] {"/foo"}, null, null, useSuffixPatternMatch);
+		match = condition.getMatchingCondition(request);
+
+		assertNotNull(match);
+		assertEquals("Trailing slash should be insensitive to useSuffixPatternMatch settings (SPR-6164, SPR-5636)", 
+				"/foo/", match.getPatterns().iterator().next());
 	}
 
 	@Test

org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMappingTests.java

@@ -268,7 +268,7 @@ public class RequestMappingInfoHandlerMappingTests {
 		protected RequestMappingInfo getMappingForMethod(Method method, Class<?> handlerType) {
 			RequestMapping annotation = AnnotationUtils.findAnnotation(method, RequestMapping.class);
 			return new RequestMappingInfo(
-					new PatternsRequestCondition(annotation.value(), getUrlPathHelper(), getPathMatcher()),
+					new PatternsRequestCondition(annotation.value(), getUrlPathHelper(), getPathMatcher(), true),
 					new RequestMethodsRequestCondition(annotation.method()),
 					new ParamsRequestCondition(annotation.params()),
 					new HeadersRequestCondition(annotation.headers()),