Merge pull request #795 from poutsma/SPR-12975

* SPR-12975:
  Remove duplicate separators when combining paths

Author: sbrannen

spring-core/src/main/java/org/springframework/util/AntPathMatcher.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -26,26 +26,38 @@
 import java.util.regex.Pattern;
 
 /**
- * PathMatcher implementation for Ant-style path patterns. Examples are provided below.
+ * {@link PathMatcher} implementation for Ant-style path patterns.
  *
  * <p>Part of this mapping code has been kindly borrowed from <a href="http://ant.apache.org">Apache Ant</a>.
  *
- * <p>The mapping matches URLs using the following rules:<br> <ul> <li>? matches one character</li> <li>* matches zero
- * or more characters</li> <li>** matches zero or more 'directories' in a path</li> </ul>
+ * <p>The mapping matches URLs using the following rules:<br>
+ * <ul>
+ * <li>{@code ?} matches one character</li>
+ * <li>{@code *} matches zero or more characters</li>
+ * <li>{@code **} matches zero or more <em>directories</em> in a path</li>
+ * </ul>
  *
- * <p>Some examples:<br> <ul> <li>{@code com/t?st.jsp} - matches {@code com/test.jsp} but also
- * {@code com/tast.jsp} or {@code com/txst.jsp}</li> <li>{@code com/*.jsp} - matches all
- * {@code .jsp} files in the {@code com} directory</li> <li>{@code com/&#42;&#42;/test.jsp} - matches all
- * {@code test.jsp} files underneath the {@code com} path</li> <li>{@code org/springframework/&#42;&#42;/*.jsp}
- * - matches all {@code .jsp} files underneath the {@code org/springframework} path</li>
- * <li>{@code org/&#42;&#42;/servlet/bla.jsp} - matches {@code org/springframework/servlet/bla.jsp} but also
- * {@code org/springframework/testing/servlet/bla.jsp} and {@code org/servlet/bla.jsp}</li> </ul>
+ * <h3>Examples</h3>
+ * <ul>
+ * <li>{@code com/t?st.jsp} &mdash; matches {@code com/test.jsp} but also
+ * {@code com/tast.jsp} or {@code com/txst.jsp}</li>
+ * <li>{@code com/*.jsp} &mdash; matches all {@code .jsp} files in the
+ * {@code com} directory</li>
+ * <li><code>com/&#42;&#42;/test.jsp</code> &mdash; matches all {@code test.jsp}
+ * files underneath the {@code com} path</li>
+ * <li><code>org/springframework/&#42;&#42;/*.jsp</code> &mdash; matches all
+ * {@code .jsp} files underneath the {@code org/springframework} path</li>
+ * <li><code>org/&#42;&#42;/servlet/bla.jsp</code> &mdash; matches
+ * {@code org/springframework/servlet/bla.jsp} but also
+ * {@code org/springframework/testing/servlet/bla.jsp} and {@code org/servlet/bla.jsp}</li>
+ * </ul>
  *
  * @author Alef Arendsen
  * @author Juergen Hoeller
  * @author Rob Harrop
  * @author Arjen Poutsma
  * @author Rossen Stoyanchev
+ * @author Sam Brannen
  * @since 16.07.2003
  */
 public class AntPathMatcher implements PathMatcher {
@@ -80,7 +92,7 @@ public AntPathMatcher() {
 	}
 
 	/**
-	 * A convenience alternative constructor to use with a custom path separator.
+	 * A convenient, alternative constructor to use with a custom path separator.
 	 * @param pathSeparator the path separator to use, must not be {@code null}.
 	 * @since 4.1
 	 */
@@ -93,7 +105,7 @@ public AntPathMatcher(String pathSeparator) {
 
 	/**
 	 * Set the path separator to use for pattern parsing.
-	 * Default is "/", as in Ant.
+	 * <p>Default is "/", as in Ant.
 	 */
 	public void setPathSeparator(String pathSeparator) {
 		this.pathSeparator = (pathSeparator != null ? pathSeparator : DEFAULT_PATH_SEPARATOR);
@@ -102,7 +114,7 @@ public void setPathSeparator(String pathSeparator) {
 
 	/**
 	 * Specify whether to trim tokenized paths and patterns.
-	 * Default is {@code true}.
+	 * <p>Default is {@code true}.
 	 */
 	public void setTrimTokens(boolean trimTokens) {
 		this.trimTokens = trimTokens;
@@ -116,7 +128,7 @@ public void setTrimTokens(boolean trimTokens) {
 	 * <p>Default is for the cache to be on, but with the variant to automatically
 	 * turn it off when encountering too many patterns to cache at runtime
 	 * (the threshold is 65536), assuming that arbitrary permutations of patterns
-	 * are coming in, with little chance for encountering a reoccurring pattern.
+	 * are coming in, with little chance for encountering a recurring pattern.
 	 * @see #getStringMatcher(String)
 	 */
 	public void setCachePatterns(boolean cachePatterns) {
@@ -317,7 +329,7 @@ protected String[] tokenizePath(String path) {
 	}
 
 	/**
-	 * Tests whether or not a string matches against a pattern.
+	 * Test whether or not a string matches against a pattern.
 	 * @param pattern the pattern to match against (never {@code null})
 	 * @param str the String which must be matched against the pattern (never {@code null})
 	 * @return {@code true} if the string matches against the pattern, or {@code false} otherwise
@@ -331,10 +343,10 @@ private boolean matchStrings(String pattern, String str, Map<String, String> uri
 	 * <p>The default implementation checks this AntPathMatcher's internal cache
 	 * (see {@link #setCachePatterns}), creating a new AntPathStringMatcher instance
 	 * if no cached copy is found.
-	 * When encountering too many patterns to cache at runtime (the threshold is 65536),
+	 * <p>When encountering too many patterns to cache at runtime (the threshold is 65536),
 	 * it turns the default cache off, assuming that arbitrary permutations of patterns
-	 * are coming in, with little chance for encountering a reoccurring pattern.
-	 * <p>This method may get overridden to implement a custom cache strategy.
+	 * are coming in, with little chance for encountering a recurring pattern.
+	 * <p>This method may be overridden to implement a custom cache strategy.
 	 * @param pattern the pattern to match against (never {@code null})
 	 * @return a corresponding AntPathStringMatcher (never {@code null})
 	 * @see #setCachePatterns
@@ -406,23 +418,35 @@ public Map<String, String> extractUriTemplateVariables(String pattern, String pa
 	}
 
 	/**
-	 * Combines two patterns into a new pattern that is returned.
-	 * <p>This implementation simply concatenates the two patterns, unless the first pattern
-	 * contains a file extension match (such as {@code *.html}. In that case, the second pattern
-	 * should be included in the first, or an {@code IllegalArgumentException} is thrown.
-	 * <p>For example: <table>
-	 * <tr><th>Pattern 1</th><th>Pattern 2</th><th>Result</th></tr> <tr><td>/hotels</td><td>{@code
-	 * null}</td><td>/hotels</td></tr> <tr><td>{@code null}</td><td>/hotels</td><td>/hotels</td></tr>
-	 * <tr><td>/hotels</td><td>/bookings</td><td>/hotels/bookings</td></tr> <tr><td>/hotels</td><td>bookings</td><td>/hotels/bookings</td></tr>
-	 * <tr><td>/hotels/*</td><td>/bookings</td><td>/hotels/bookings</td></tr> <tr><td>/hotels/&#42;&#42;</td><td>/bookings</td><td>/hotels/&#42;&#42;/bookings</td></tr>
-	 * <tr><td>/hotels</td><td>{hotel}</td><td>/hotels/{hotel}</td></tr> <tr><td>/hotels/*</td><td>{hotel}</td><td>/hotels/{hotel}</td></tr>
+	 * Combine two patterns into a new pattern.
+	 *
+	 * <p>This implementation simply concatenates the two patterns, unless
+	 * the first pattern contains a file extension match (e.g., {@code *.html}).
+	 * In that case, the second pattern will be merged into the first. Otherwise,
+	 * an {@code IllegalArgumentException} will be thrown.
+	 *
+	 * <h3>Examples</h3>
+	 * <table border="1">
+	 * <tr><th>Pattern 1</th><th>Pattern 2</th><th>Result</th></tr>
+	 * <tr><td>{@code null}</td><td>{@code null}</td><td>&nbsp;</td></tr>
+	 * <tr><td>/hotels</td><td>{@code null}</td><td>/hotels</td></tr>
+	 * <tr><td>{@code null}</td><td>/hotels</td><td>/hotels</td></tr>
+	 * <tr><td>/hotels</td><td>/bookings</td><td>/hotels/bookings</td></tr>
+	 * <tr><td>/hotels</td><td>bookings</td><td>/hotels/bookings</td></tr>
+	 * <tr><td>/hotels/*</td><td>/bookings</td><td>/hotels/bookings</td></tr>
+	 * <tr><td>/hotels/&#42;&#42;</td><td>/bookings</td><td>/hotels/&#42;&#42;/bookings</td></tr>
+	 * <tr><td>/hotels</td><td>{hotel}</td><td>/hotels/{hotel}</td></tr>
+	 * <tr><td>/hotels/*</td><td>{hotel}</td><td>/hotels/{hotel}</td></tr>
 	 * <tr><td>/hotels/&#42;&#42;</td><td>{hotel}</td><td>/hotels/&#42;&#42;/{hotel}</td></tr>
-	 * <tr><td>/*.html</td><td>/hotels.html</td><td>/hotels.html</td></tr> <tr><td>/*.html</td><td>/hotels</td><td>/hotels.html</td></tr>
-	 * <tr><td>/*.html</td><td>/*.txt</td><td>IllegalArgumentException</td></tr> </table>
+	 * <tr><td>/*.html</td><td>/hotels.html</td><td>/hotels.html</td></tr>
+	 * <tr><td>/*.html</td><td>/hotels</td><td>/hotels.html</td></tr>
+	 * <tr><td>/*.html</td><td>/*.txt</td><td>{@code IllegalArgumentException}</td></tr>
+	 * </table>
+	 *
 	 * @param pattern1 the first pattern
 	 * @param pattern2 the second pattern
 	 * @return the combination of the two patterns
-	 * @throws IllegalArgumentException when the two patterns cannot be combined
+	 * @throws IllegalArgumentException if the two patterns cannot be combined
 	 */
 	@Override
 	public String combine(String pattern1, String pattern2) {
@@ -469,10 +493,18 @@ public String combine(String pattern1, String pattern2) {
 	}
 
 	private String concat(String path1, String path2) {
-		if (path1.endsWith(this.pathSeparator) || path2.startsWith(this.pathSeparator)) {
+		boolean path1EndsWithSeparator = path1.endsWith(this.pathSeparator);
+		boolean path2StartsWithSeparator = path2.startsWith(this.pathSeparator);
+
+		if (path1EndsWithSeparator && path2StartsWithSeparator) {
+			return path1 + path2.substring(1);
+		}
+		else if (path1EndsWithSeparator || path2StartsWithSeparator) {
 			return path1 + path2;
 		}
-		return path1 + this.pathSeparator + path2;
+		else {
+			return path1 + this.pathSeparator + path2;
+		}
 	}
 
 	/**

spring-core/src/test/java/org/springframework/util/AntPathMatcherTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -63,7 +63,7 @@ public void match() {
 		assertFalse(pathMatcher.match("tes?", "testt"));
 		assertFalse(pathMatcher.match("tes?", "tsst"));
 
-		// test matchin with *'s
+		// test matching with *'s
 		assertTrue(pathMatcher.match("*", "test"));
 		assertTrue(pathMatcher.match("test*", "test"));
 		assertTrue(pathMatcher.match("test*", "testTest"));
@@ -148,7 +148,7 @@ public void withMatchStart() {
 		assertFalse(pathMatcher.matchStart("tes?", "testt"));
 		assertFalse(pathMatcher.matchStart("tes?", "tsst"));
 
-		// test matchin with *'s
+		// test matching with *'s
 		assertTrue(pathMatcher.matchStart("*", "test"));
 		assertTrue(pathMatcher.matchStart("test*", "test"));
 		assertTrue(pathMatcher.matchStart("test*", "testTest"));
@@ -237,7 +237,7 @@ public void uniqueDeliminator() {
 		assertFalse(pathMatcher.match("tes?", "testt"));
 		assertFalse(pathMatcher.match("tes?", "tsst"));
 
-		// test matchin with *'s
+		// test matching with *'s
 		assertTrue(pathMatcher.match("*", "test"));
 		assertTrue(pathMatcher.match("test*", "test"));
 		assertTrue(pathMatcher.match("test*", "testTest"));
@@ -354,8 +354,9 @@ public void extractUriTemplateVariablesRegex() {
 		assertEquals("1.0.0", result.get("version"));
 	}
 
-	// SPR-7787
-
+	/**
+	 * SPR-7787
+	 */
 	@Test
 	public void extractUriTemplateVarsRegexQualifiers() {
 		Map<String, String> result = pathMatcher.extractUriTemplateVariables(
@@ -380,8 +381,9 @@ public void extractUriTemplateVarsRegexQualifiers() {
 		assertEquals("1.0.0.{12}", result.get("version"));
 	}
 
-	// SPR-8455
-
+	/**
+	 * SPR-8455
+	 */
 	@Test
 	public void extractUriTemplateVarsRegexCapturingGroups() {
 		try {
@@ -421,6 +423,8 @@ public void combine() {
 		assertEquals("/user/user", pathMatcher.combine("/user", "/user"));	// SPR-7970
 		assertEquals("/{foo:.*[^0-9].*}/edit/", pathMatcher.combine("/{foo:.*[^0-9].*}", "/edit/")); // SPR-10062
 		assertEquals("/1.0/foo/test", pathMatcher.combine("/1.0", "/foo/test")); // SPR-10554
+		assertEquals("/hotel", pathMatcher.combine("/", "/hotel")); // SPR-12975
+		assertEquals("/hotel/booking", pathMatcher.combine("/hotel/", "/booking")); // SPR-12975
 	}
 
 	@Test
@@ -568,8 +572,9 @@ public void patternComparatorSort() {
 		paths.clear();
 	}
 
-	// SPR-8687
-
+	/**
+	 * SPR-8687
+	 */
 	@Test
 	public void trimTokensOff() {
 		pathMatcher.setTrimTokens(false);
@@ -579,7 +584,7 @@ public void trimTokensOff() {
 	}
 
 	@Test
-	public void testDefaultCacheSetting() {
+	public void defaultCacheSetting() {
 		match();
 		assertTrue(pathMatcher.stringMatcherCache.size() > 20);
 
@@ -591,7 +596,7 @@ public void testDefaultCacheSetting() {
 	}
 
 	@Test
-	public void testCacheSetToTrue() {
+	public void cachePatternsSetToTrue() {
 		pathMatcher.setCachePatterns(true);
 		match();
 		assertTrue(pathMatcher.stringMatcherCache.size() > 20);
@@ -604,14 +609,14 @@ public void testCacheSetToTrue() {
 	}
 
 	@Test
-	public void testCacheSetToFalse() {
+	public void cachePatternsSetToFalse() {
 		pathMatcher.setCachePatterns(false);
 		match();
 		assertTrue(pathMatcher.stringMatcherCache.isEmpty());
 	}
 
 	@Test
-	public void testExtensionMappingWithDotPathSeparator() {
+	public void extensionMappingWithDotPathSeparator() {
 		pathMatcher.setPathSeparator(".");
 		assertEquals("Extension mapping should be disabled with \".\" as path separator",
 				"/*.html.hotel.*", pathMatcher.combine("/*.html", "hotel.*"));

spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/AbstractRequestCondition.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -59,17 +59,27 @@ public String toString() {
 		return builder.toString();
 	}
 
+	/**
+	 * Indicates whether this condition is empty, i.e. whether or not it
+	 * contains any discrete items.
+	 * @return {@code true} if empty; {@code false} otherwise
+	 */
+	public boolean isEmpty() {
+		return getContent().isEmpty();
+	}
+
 
 	/**
 	 * Return the discrete items a request condition is composed of.
-	 * For example URL patterns, HTTP request methods, param expressions, etc.
+	 * <p>For example URL patterns, HTTP request methods, param expressions, etc.
 	 * @return a collection of objects, never {@code null}
 	 */
 	protected abstract Collection<?> getContent();
 
 	/**
 	 * The notation to use when printing discrete items of content.
-	 * For example " || " for URL patterns or " && " for param expressions.
+	 * <p>For example {@code " || "} for URL patterns or {@code " && "}
+	 * for param expressions.
 	 */
 	protected abstract String getToStringInfix();
 

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

@@ -326,12 +326,24 @@ public int hashCode() {
 	public String toString() {
 		StringBuilder builder = new StringBuilder("{");
 		builder.append(this.patternsCondition);
-		builder.append(",methods=").append(this.methodsCondition);
-		builder.append(",params=").append(this.paramsCondition);
-		builder.append(",headers=").append(this.headersCondition);
-		builder.append(",consumes=").append(this.consumesCondition);
-		builder.append(",produces=").append(this.producesCondition);
-		builder.append(",custom=").append(this.customConditionHolder);
+		if (!this.methodsCondition.isEmpty()) {
+			builder.append(",methods=").append(this.methodsCondition);
+		}
+		if (!this.paramsCondition.isEmpty()) {
+			builder.append(",params=").append(this.paramsCondition);
+		}
+		if (!this.headersCondition.isEmpty()) {
+			builder.append(",headers=").append(this.headersCondition);
+		}
+		if (!this.consumesCondition.isEmpty()) {
+			builder.append(",consumes=").append(this.consumesCondition);
+		}
+		if (!this.producesCondition.isEmpty()) {
+			builder.append(",produces=").append(this.producesCondition);
+		}
+		if (!this.customConditionHolder.isEmpty()) {
+			builder.append(",custom=").append(this.customConditionHolder);
+		}
 		builder.append('}');
 		return builder.toString();
 	}