Spring/spring-framework
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Remove trailing slash matching in PathPatternParser

Browse Source 
See gh-34036
This commit is contained in:
rstoyanchev
2024-12-13 14:51:22 +00:00
parent 32db1a1680
commit 2e6046a655
8 changed files with 8 additions and 302 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
spring-web/src/main/java/org/springframework/web/util/pattern/CaptureVariablePathElement.java
View File

@@ -100,11 +100,6 @@ class CaptureVariablePathElement extends PathElement {
else {
// Needs to be at least one character #SPR15264
match = (pathIndex == matchingContext.pathLength);
if (!match && matchingContext.isMatchOptionalTrailingSeparator()) {
match = //(nextPos > candidateIndex) &&
(pathIndex + 1) == matchingContext.pathLength &&
matchingContext.isSeparator(pathIndex);
}
}
}
else {

9
spring-web/src/main/java/org/springframework/web/util/pattern/LiteralPathElement.java
View File

@@ -78,14 +78,7 @@ class LiteralPathElement extends PathElement {
return true;
}
else {
if (pathIndex == matchingContext.pathLength) {
return true;
}
else {
return (matchingContext.isMatchOptionalTrailingSeparator() &&
(pathIndex + 1) == matchingContext.pathLength &&
matchingContext.isSeparator(pathIndex));
}
return (pathIndex == matchingContext.pathLength);
}
}
else {

18
spring-web/src/main/java/org/springframework/web/util/pattern/PathPattern.java
View File

@@ -112,9 +112,6 @@ public class PathPattern implements Comparable<PathPattern> {
/** The options to use to parse a pattern. */
private final PathContainer.Options pathOptions;
/** If this pattern has no trailing slash, allow candidates to include one and still match successfully. */
private final boolean matchOptionalTrailingSeparator;
/** Will this match candidates in a case-sensitive way? (case sensitivity at parse time). */
private final boolean caseSensitive;
@@ -152,12 +149,10 @@ public class PathPattern implements Comparable<PathPattern> {
private boolean catchAll = false;
@SuppressWarnings("deprecation")
PathPattern(String patternText, PathPatternParser parser, @Nullable PathElement head) {
this.patternString = patternText;
this.parser = parser;
this.pathOptions = parser.getPathOptions();
this.matchOptionalTrailingSeparator = parser.isMatchOptionalTrailingSeparator();
this.caseSensitive = parser.isCaseSensitive();
this.head = head;
@@ -202,8 +197,7 @@ public class PathPattern implements Comparable<PathPattern> {
*/
public boolean matches(PathContainer pathContainer) {
if (this.head == null) {
return !hasLength(pathContainer) ||
(this.matchOptionalTrailingSeparator && pathContainerIsJustSeparator(pathContainer));
return !hasLength(pathContainer);
}
else if (!hasLength(pathContainer)) {
if (this.head instanceof WildcardTheRestPathElement || this.head instanceof CaptureTheRestPathElement) {
@@ -226,9 +220,7 @@ public class PathPattern implements Comparable<PathPattern> {
@Nullable
public PathMatchInfo matchAndExtract(PathContainer pathContainer) {
if (this.head == null) {
return (hasLength(pathContainer) &&
!(this.matchOptionalTrailingSeparator && pathContainerIsJustSeparator(pathContainer)) ?
null : PathMatchInfo.EMPTY);
return (hasLength(pathContainer) && !pathContainerIsJustSeparator(pathContainer) ? null : PathMatchInfo.EMPTY);
}
else if (!hasLength(pathContainer)) {
if (this.head instanceof WildcardTheRestPathElement || this.head instanceof CaptureTheRestPathElement) {
@@ -647,7 +639,7 @@ public class PathPattern implements Comparable<PathPattern> {
* candidate currently being considered for a match but also some accumulators for
* extracted variables.
*/
class MatchingContext {
static class MatchingContext {
final PathContainer candidate;
@@ -681,10 +673,6 @@ public class PathPattern implements Comparable<PathPattern> {
this.determineRemainingPath = true;
}
public boolean isMatchOptionalTrailingSeparator() {
return matchOptionalTrailingSeparator;
}
public void set(String key, String value, MultiValueMap<String,String> parameters) {
if (this.extractedUriVariables == null) {
this.extractedUriVariables = new HashMap<>();

37
spring-web/src/main/java/org/springframework/web/util/pattern/PathPatternParser.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2023 the original author or authors.
* Copyright 2002-2024 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.
@@ -35,40 +35,11 @@ import org.springframework.util.StringUtils;
*/
public class PathPatternParser {
private boolean matchOptionalTrailingSeparator = false;
private boolean caseSensitive = true;
private PathContainer.Options pathOptions = PathContainer.Options.HTTP_PATH;
/**
* Configure whether a {@link PathPattern} produced by this parser should
* automatically match request paths with a trailing slash.
* <p>If set to {@code true} a {@code PathPattern} without a trailing slash
* will also match request paths with a trailing slash. If set to
* {@code false} a {@code PathPattern} will only match request paths with
* a trailing slash.
* <p>The default was changed in 6.0 from {@code true} to {@code false} in
* order to support the deprecation of the property.
* @deprecated transparent support for trailing slashes is deprecated as of
* 6.0 in favor of configuring explicit redirects through a proxy,
* Servlet/web filter, or a controller.
*/
@Deprecated(since = "6.0")
public void setMatchOptionalTrailingSeparator(boolean matchOptionalTrailingSeparator) {
this.matchOptionalTrailingSeparator = matchOptionalTrailingSeparator;
}
/**
* Whether optional trailing slashing match is enabled.
* @deprecated as of 6.0 together with {@link #setMatchOptionalTrailingSeparator(boolean)}.
*/
@Deprecated(since = "6.0")
public boolean isMatchOptionalTrailingSeparator() {
return this.matchOptionalTrailingSeparator;
}
/**
* Configure whether path pattern matching should be case-sensitive.
* <p>The default is {@code true}.
@@ -141,12 +112,6 @@ public class PathPatternParser {
*/
public static final PathPatternParser defaultInstance = new PathPatternParser() {
@SuppressWarnings("deprecation")
@Override
public void setMatchOptionalTrailingSeparator(boolean matchOptionalTrailingSeparator) {
raiseError();
}
@Override
public void setCaseSensitive(boolean caseSensitive) {
raiseError();

5
spring-web/src/main/java/org/springframework/web/util/pattern/RegexPathElement.java
View File

@@ -143,11 +143,6 @@ class RegexPathElement extends PathElement {
// If pattern is capturing variables there must be some actual data to bind to them
matches = (pathIndex + 1 >= matchingContext.pathLength) &&
(this.variableNames.isEmpty() || !textToMatch.isEmpty());
if (!matches && matchingContext.isMatchOptionalTrailingSeparator()) {
matches = (this.variableNames.isEmpty() || !textToMatch.isEmpty()) &&
(pathIndex + 2 >= matchingContext.pathLength) &&
matchingContext.isSeparator(pathIndex + 1);
}
}
}
else {

9
spring-web/src/main/java/org/springframework/web/util/pattern/SingleCharWildcardedPathElement.java
View File

@@ -99,14 +99,7 @@ class SingleCharWildcardedPathElement extends PathElement {
return true;
}
else {
if (pathIndex == matchingContext.pathLength) {
return true;
}
else {
return (matchingContext.isMatchOptionalTrailingSeparator() &&
(pathIndex + 1) == matchingContext.pathLength &&
matchingContext.isSeparator(pathIndex));
}
return (pathIndex == matchingContext.pathLength);
}
}
else {

12
spring-web/src/main/java/org/springframework/web/util/pattern/WildcardPathElement.java
View File

@@ -60,16 +60,8 @@ class WildcardPathElement extends PathElement {
return true;
}
else {
if (pathIndex == matchingContext.pathLength) {
// and the path data has run out too
return true;
}
else {
return (matchingContext.isMatchOptionalTrailingSeparator() && // if optional slash is on...
segmentData != null && !segmentData.isEmpty() && // and there is at least one character to match the *...
(pathIndex + 1) == matchingContext.pathLength && // and the next path element is the end of the candidate...
matchingContext.isSeparator(pathIndex)); // and the final element is a separator
}
// and the path data has run out too
return (pathIndex == matchingContext.pathLength);
}
}
else {

215
spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternTests.java
View File

@@ -80,7 +80,6 @@ class PathPatternTests {
@Test
void basicMatching() {
checkMatches("", "");
checkMatches("", "/");
checkMatches("", null);
checkNoMatch("/abc", "/");
checkMatches("/", "/");
@@ -99,211 +98,6 @@ class PathPatternTests {
assertThat(pp.matches(toPathContainer(path))).isFalse();
}
@SuppressWarnings("deprecation")
@Test
void optionalTrailingSeparators() {
PathPattern pp;
// LiteralPathElement
pp = parse("/resource");
assertMatches(pp,"/resource");
assertMatches(pp,"/resource");
assertMatches(pp,"/resource/");
assertNoMatch(pp,"/resource//");
pp = parse("/resource/");
assertNoMatch(pp,"/resource");
assertMatches(pp,"/resource/");
assertNoMatch(pp,"/resource//");
pp = parse("res?urce");
assertNoMatch(pp,"resource//");
// SingleCharWildcardPathElement
pp = parse("/res?urce");
assertMatches(pp,"/resource");
assertMatches(pp,"/resource/");
assertNoMatch(pp,"/resource//");
pp = parse("/res?urce/");
assertNoMatch(pp,"/resource");
assertMatches(pp,"/resource/");
assertNoMatch(pp,"/resource//");
// CaptureVariablePathElement
pp = parse("/{var}");
assertMatches(pp,"/resource");
assertThat(pp.matchAndExtract(toPathContainer("/resource")).getUriVariables()).containsEntry("var", "resource");
assertMatches(pp,"/resource/");
assertThat(pp.matchAndExtract(toPathContainer("/resource/")).getUriVariables()).containsEntry(
"var",
"resource"
);
assertNoMatch(pp,"/resource//");
pp = parse("/{var}/");
assertNoMatch(pp,"/resource");
assertMatches(pp,"/resource/");
assertThat(pp.matchAndExtract(toPathContainer("/resource/")).getUriVariables()).containsEntry(
"var",
"resource"
);
assertNoMatch(pp,"/resource//");
// CaptureTheRestPathElement
pp = parse("/{*var}");
assertMatches(pp,"/resource");
assertThat(pp.matchAndExtract(toPathContainer("/resource")).getUriVariables()).containsEntry(
"var",
"/resource"
);
assertMatches(pp,"/resource/");
assertThat(pp.matchAndExtract(toPathContainer("/resource/")).getUriVariables()).containsEntry(
"var",
"/resource/"
);
assertMatches(pp,"/resource//");
assertThat(pp.matchAndExtract(toPathContainer("/resource//")).getUriVariables()).containsEntry(
"var",
"/resource//"
);
assertMatches(pp,"//resource//");
assertThat(pp.matchAndExtract(toPathContainer("//resource//")).getUriVariables()).containsEntry(
"var",
"//resource//"
);
// WildcardTheRestPathElement
pp = parse("/**");
assertMatches(pp,"/resource");
assertMatches(pp,"/resource/");
assertMatches(pp,"/resource//");
assertMatches(pp,"//resource//");
// WildcardPathElement
pp = parse("/*");
assertMatches(pp,"/resource");
assertMatches(pp,"/resource/");
assertNoMatch(pp,"/resource//");
pp = parse("/*/");
assertNoMatch(pp,"/resource");
assertMatches(pp,"/resource/");
assertNoMatch(pp,"/resource//");
// RegexPathElement
pp = parse("/{var1}_{var2}");
assertMatches(pp,"/res1_res2");
assertThat(pp.matchAndExtract(toPathContainer("/res1_res2")).getUriVariables()).containsEntry("var1", "res1");
assertThat(pp.matchAndExtract(toPathContainer("/res1_res2")).getUriVariables()).containsEntry("var2", "res2");
assertMatches(pp,"/res1_res2/");
assertThat(pp.matchAndExtract(toPathContainer("/res1_res2/")).getUriVariables()).containsEntry("var1", "res1");
assertThat(pp.matchAndExtract(toPathContainer("/res1_res2/")).getUriVariables()).containsEntry("var2", "res2");
assertNoMatch(pp,"/res1_res2//");
pp = parse("/{var1}_{var2}/");
assertNoMatch(pp,"/res1_res2");
assertMatches(pp,"/res1_res2/");
assertThat(pp.matchAndExtract(toPathContainer("/res1_res2/")).getUriVariables()).containsEntry("var1", "res1");
assertThat(pp.matchAndExtract(toPathContainer("/res1_res2/")).getUriVariables()).containsEntry("var2", "res2");
assertNoMatch(pp,"/res1_res2//");
pp = parse("/{var1}*");
assertMatches(pp,"/a");
assertMatches(pp,"/a/");
assertNoMatch(pp,"/"); // no characters for var1
assertNoMatch(pp,"//"); // no characters for var1
// Now with trailing matching turned OFF
PathPatternParser parser = new PathPatternParser();
parser.setMatchOptionalTrailingSeparator(false);
// LiteralPathElement
pp = parser.parse("/resource");
assertMatches(pp,"/resource");
assertNoMatch(pp,"/resource/");
assertNoMatch(pp,"/resource//");
pp = parser.parse("/resource/");
assertNoMatch(pp,"/resource");
assertMatches(pp,"/resource/");
assertNoMatch(pp,"/resource//");
// SingleCharWildcardPathElement
pp = parser.parse("/res?urce");
assertMatches(pp,"/resource");
assertNoMatch(pp,"/resource/");
assertNoMatch(pp,"/resource//");
pp = parser.parse("/res?urce/");
assertNoMatch(pp,"/resource");
assertMatches(pp,"/resource/");
assertNoMatch(pp,"/resource//");
// CaptureVariablePathElement
pp = parser.parse("/{var}");
assertMatches(pp,"/resource");
assertThat(pp.matchAndExtract(toPathContainer("/resource")).getUriVariables()).containsEntry("var", "resource");
assertNoMatch(pp,"/resource/");
assertNoMatch(pp,"/resource//");
pp = parser.parse("/{var}/");
assertNoMatch(pp,"/resource");
assertMatches(pp,"/resource/");
assertThat(pp.matchAndExtract(toPathContainer("/resource/")).getUriVariables()).containsEntry(
"var",
"resource"
);
assertNoMatch(pp,"/resource//");
// CaptureTheRestPathElement
pp = parser.parse("/{*var}");
assertMatches(pp,"/resource");
assertThat(pp.matchAndExtract(toPathContainer("/resource")).getUriVariables()).containsEntry(
"var",
"/resource"
);
assertMatches(pp,"/resource/");
assertThat(pp.matchAndExtract(toPathContainer("/resource/")).getUriVariables()).containsEntry(
"var",
"/resource/"
);
assertMatches(pp,"/resource//");
assertThat(pp.matchAndExtract(toPathContainer("/resource//")).getUriVariables()).containsEntry(
"var",
"/resource//"
);
assertMatches(pp,"//resource//");
assertThat(pp.matchAndExtract(toPathContainer("//resource//")).getUriVariables()).containsEntry(
"var",
"//resource//"
);
// WildcardTheRestPathElement
pp = parser.parse("/**");
assertMatches(pp,"/resource");
assertMatches(pp,"/resource/");
assertMatches(pp,"/resource//");
assertMatches(pp,"//resource//");
// WildcardPathElement
pp = parser.parse("/*");
assertMatches(pp,"/resource");
assertNoMatch(pp,"/resource/");
assertNoMatch(pp,"/resource//");
pp = parser.parse("/*/");
assertNoMatch(pp,"/resource");
assertMatches(pp,"/resource/");
assertNoMatch(pp,"/resource//");
// RegexPathElement
pp = parser.parse("/{var1}_{var2}");
assertMatches(pp,"/res1_res2");
assertThat(pp.matchAndExtract(toPathContainer("/res1_res2")).getUriVariables()).containsEntry("var1", "res1");
assertThat(pp.matchAndExtract(toPathContainer("/res1_res2")).getUriVariables()).containsEntry("var2", "res2");
assertNoMatch(pp,"/res1_res2/");
assertNoMatch(pp,"/res1_res2//");
pp = parser.parse("/{var1}_{var2}/");
assertNoMatch(pp,"/res1_res2");
assertMatches(pp,"/res1_res2/");
assertThat(pp.matchAndExtract(toPathContainer("/res1_res2/")).getUriVariables()).containsEntry("var1", "res1");
assertThat(pp.matchAndExtract(toPathContainer("/res1_res2/")).getUriVariables()).containsEntry("var2", "res2");
assertNoMatch(pp,"/res1_res2//");
pp = parser.parse("/{var1}*");
assertMatches(pp,"/a");
assertNoMatch(pp,"/a/");
assertNoMatch(pp,"/"); // no characters for var1
assertNoMatch(pp,"//"); // no characters for var1
}
@Test
void pathRemainderBasicCases_spr15336() {
// Cover all PathElement kinds
@@ -493,12 +287,9 @@ class PathPatternTests {
checkMatches("*a*", "testa");
checkMatches("a/*", "a/");
checkNoMatch("a/*", "a//"); // no data for *
checkMatches("a/*", "a/a/"); // trailing slash, so is allowed
PathPatternParser ppp = new PathPatternParser();
ppp.setMatchOptionalTrailingSeparator(false);
assertThat(ppp.parse("a/*").matches(toPathContainer("a//"))).isFalse();
checkMatches("a/*", "a/a");
checkMatches("a/*", "a/a/"); // trailing slash is optional
checkMatches("/resource/**", "/resource");
checkNoMatch("/resource/**", "/resourceX");
checkNoMatch("/resource/**", "/resourceX/foobar");
@@ -554,7 +345,6 @@ class PathPatternTests {
checkMatches("test*aaa", "testblaaaa");
checkNoMatch("test*", "tst");
checkNoMatch("test*", "tsttest");
checkMatches("test*", "test/"); // trailing slash is optional
checkMatches("test*", "test"); // trailing slash is optional
checkNoMatch("test*", "test/t");
checkNoMatch("test/*", "test");
@@ -805,7 +595,6 @@ class PathPatternTests {
// Only patterns not capturing variables cannot match against just /
PathPatternParser ppp = new PathPatternParser();
ppp.setMatchOptionalTrailingSeparator(true);
pp = ppp.parse("/****");
assertMatches(pp,"/abcdef");
assertMatches(pp,"/");
@@ -1185,10 +974,8 @@ class PathPatternTests {
return parse(pattern).matchAndExtract(PathPatternTests.toPathContainer(path));
}
@SuppressWarnings("deprecation")
private PathPattern parse(String path) {
PathPatternParser pp = new PathPatternParser();
pp.setMatchOptionalTrailingSeparator(true);
return pp.parse(path);
}
@@ -1199,10 +986,8 @@ class PathPatternTests {
return PathContainer.parsePath(path);
}
@SuppressWarnings("deprecation")
private void checkMatches(String uriTemplate, String path) {
PathPatternParser parser = new PathPatternParser();
parser.setMatchOptionalTrailingSeparator(true);
PathPattern p = parser.parse(uriTemplate);
PathContainer pc = toPathContainer(path);
assertThat(p.matches(pc)).isTrue();