From 76beb36e4bd1162bed6ff91f4ba5df6f9e47b528 Mon Sep 17 00:00:00 2001 From: Arjen Poutsma Date: Thu, 7 May 2015 16:17:36 +0200 Subject: [PATCH] Remove duplicate separators when combining paths Prior to this commit, AntPathMatcher would not correctly combine a path that ends with a separator with a path that starts with a separator. For example, `/foo/` + `/bar` combined into `/foo//bar`. Specifically, this commit: - Removes the duplicated separator in combined paths - Improves RequestMappingInfo's toString() representation - Fixes Javadoc formatting in AntPathMatcher - Polishes AntPathMatcherTests - Polishes Javadoc in AbstractRequestCondition Issue: SPR-12975 --- .../springframework/util/AntPathMatcher.java | 100 ++++++++++++------ .../util/AntPathMatcherTests.java | 33 +++--- .../condition/AbstractRequestCondition.java | 16 ++- .../mvc/method/RequestMappingInfo.java | 24 +++-- 4 files changed, 116 insertions(+), 57 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/util/AntPathMatcher.java b/spring-core/src/main/java/org/springframework/util/AntPathMatcher.java index ff2056ab62..89fe9fbd3b 100644 --- a/spring-core/src/main/java/org/springframework/util/AntPathMatcher.java +++ b/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.Matcher; import java.util.regex.Pattern; /** - * PathMatcher implementation for Ant-style path patterns. Examples are provided below. + * {@link PathMatcher} implementation for Ant-style path patterns. * *

Part of this mapping code has been kindly borrowed from Apache Ant. * - *

The mapping matches URLs using the following rules:

+ *

The mapping matches URLs using the following rules:
+ *

* - *

Some examples:

+ *

Examples

+ * * * @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 class AntPathMatcher implements PathMatcher { } /** - * 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 class AntPathMatcher implements PathMatcher { /** * Set the path separator to use for pattern parsing. - * Default is "/", as in Ant. + *

Default is "/", as in Ant. */ public void setPathSeparator(String pathSeparator) { this.pathSeparator = (pathSeparator != null ? pathSeparator : DEFAULT_PATH_SEPARATOR); @@ -102,7 +114,7 @@ public class AntPathMatcher implements PathMatcher { /** * Specify whether to trim tokenized paths and patterns. - * Default is {@code true}. + *

Default is {@code true}. */ public void setTrimTokens(boolean trimTokens) { this.trimTokens = trimTokens; @@ -116,7 +128,7 @@ public class AntPathMatcher implements PathMatcher { *

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 @@ public class AntPathMatcher implements PathMatcher { } /** - * 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 @@ public class AntPathMatcher implements PathMatcher { *

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), + *

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

This method may get overridden to implement a custom cache strategy. + * are coming in, with little chance for encountering a recurring pattern. + *

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 class AntPathMatcher implements PathMatcher { } /** - * Combines two patterns into a new pattern that is returned. - *

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

For example: - * - * - * - * + * Combine two patterns into a new pattern. + * + *

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

Examples

+ *
Pattern 1Pattern 2Result
/hotels{@code - * null}/hotels
{@code null}/hotels/hotels
/hotels/bookings/hotels/bookings
/hotelsbookings/hotels/bookings
/hotels/*/bookings/hotels/bookings
/hotels/**/bookings/hotels/**/bookings
/hotels{hotel}/hotels/{hotel}
/hotels/*{hotel}/hotels/{hotel}
+ * + * + * + * + * + * + * + * + * + * * - * - *
Pattern 1Pattern 2Result
{@code null}{@code null} 
/hotels{@code null}/hotels
{@code null}/hotels/hotels
/hotels/bookings/hotels/bookings
/hotelsbookings/hotels/bookings
/hotels/*/bookings/hotels/bookings
/hotels/**/bookings/hotels/**/bookings
/hotels{hotel}/hotels/{hotel}
/hotels/*{hotel}/hotels/{hotel}
/hotels/**{hotel}/hotels/**/{hotel}
/*.html/hotels.html/hotels.html
/*.html/hotels/hotels.html
/*.html/*.txtIllegalArgumentException
+ * /*.html/hotels.html/hotels.html + * /*.html/hotels/hotels.html + * /*.html/*.txt{@code IllegalArgumentException} + * + * * @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 class AntPathMatcher implements PathMatcher { } 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; + } } /** diff --git a/spring-core/src/test/java/org/springframework/util/AntPathMatcherTests.java b/spring-core/src/test/java/org/springframework/util/AntPathMatcherTests.java index 1ff59c73c1..6bef14d740 100644 --- a/spring-core/src/test/java/org/springframework/util/AntPathMatcherTests.java +++ b/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 class AntPathMatcherTests { 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 class AntPathMatcherTests { 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 class AntPathMatcherTests { 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 class AntPathMatcherTests { assertEquals("1.0.0", result.get("version")); } - // SPR-7787 - + /** + * SPR-7787 + */ @Test public void extractUriTemplateVarsRegexQualifiers() { Map result = pathMatcher.extractUriTemplateVariables( @@ -380,8 +381,9 @@ public class AntPathMatcherTests { assertEquals("1.0.0.{12}", result.get("version")); } - // SPR-8455 - + /** + * SPR-8455 + */ @Test public void extractUriTemplateVarsRegexCapturingGroups() { try { @@ -421,6 +423,8 @@ public class AntPathMatcherTests { 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 class AntPathMatcherTests { paths.clear(); } - // SPR-8687 - + /** + * SPR-8687 + */ @Test public void trimTokensOff() { pathMatcher.setTrimTokens(false); @@ -579,7 +584,7 @@ public class AntPathMatcherTests { } @Test - public void testDefaultCacheSetting() { + public void defaultCacheSetting() { match(); assertTrue(pathMatcher.stringMatcherCache.size() > 20); @@ -591,7 +596,7 @@ public class AntPathMatcherTests { } @Test - public void testCacheSetToTrue() { + public void cachePatternsSetToTrue() { pathMatcher.setCachePatterns(true); match(); assertTrue(pathMatcher.stringMatcherCache.size() > 20); @@ -604,14 +609,14 @@ public class AntPathMatcherTests { } @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.*")); diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/AbstractRequestCondition.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/AbstractRequestCondition.java index f67b1d20d4..aa86fe50aa 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/AbstractRequestCondition.java +++ b/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 abstract class AbstractRequestConditionFor 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. + *

For example {@code " || "} for URL patterns or {@code " && "} + * for param expressions. */ protected abstract String getToStringInfix(); diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfo.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfo.java index 92760b955d..90142c4603 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfo.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfo.java @@ -326,12 +326,24 @@ public final class RequestMappingInfo implements RequestCondition