root
/
spring-framework
mirror of https://github.com/spring-projects/spring-framework.git
1
0
Fork 0
Code Issues Actions 2 Packages Projects Releases Wiki Activity

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
This commit is contained in:
Arjen Poutsma 2015-05-07 16:17:36 +02:00 committed by Sam Brannen
parent 5648fbfc31
commit 76beb36e4b
4 changed files with 116 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

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

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

@ -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<String, String> 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.*"));

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

@ -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 AbstractRequestCondition<T extends AbstractRequestConditio
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();

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

@ -326,12 +326,24 @@ public final class RequestMappingInfo implements RequestCondition<RequestMapping
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();
}