diff --git a/spring-web/src/main/java/org/springframework/web/util/pattern/InternalPathPatternParser.java b/spring-web/src/main/java/org/springframework/web/util/pattern/InternalPathPatternParser.java index 1538e4df6a7..eb85e45b98d 100644 --- a/spring-web/src/main/java/org/springframework/web/util/pattern/InternalPathPatternParser.java +++ b/spring-web/src/main/java/org/springframework/web/util/pattern/InternalPathPatternParser.java @@ -236,7 +236,7 @@ class InternalPathPatternParser { /** * After processing a separator, a quick peek whether it is followed by - * (and only before the end of the pattern or the next separator). + * a double wildcard (and only as the last path element). */ private boolean peekDoubleWildcard() { if ((this.pos + 2) >= this.pathPatternLength) { @@ -245,6 +245,11 @@ class InternalPathPatternParser { if (this.pathPatternData[this.pos + 1] != '*' || this.pathPatternData[this.pos + 2] != '*') { return false; } + char separator = this.parser.getPathOptions().separator(); + if ((this.pos + 3) < this.pathPatternLength && this.pathPatternData[this.pos + 3] == separator) { + throw new PatternParseException(this.pos, this.pathPatternData, + PatternMessage.NO_MORE_DATA_EXPECTED_AFTER_CAPTURE_THE_REST); + } return (this.pos + 3 == this.pathPatternLength); } diff --git a/spring-web/src/main/java/org/springframework/web/util/pattern/PathPatternParser.java b/spring-web/src/main/java/org/springframework/web/util/pattern/PathPatternParser.java index d083514f092..faca696babf 100644 --- a/spring-web/src/main/java/org/springframework/web/util/pattern/PathPatternParser.java +++ b/spring-web/src/main/java/org/springframework/web/util/pattern/PathPatternParser.java @@ -16,9 +16,6 @@ package org.springframework.web.util.pattern; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; - import org.springframework.http.server.PathContainer; /** @@ -37,8 +34,6 @@ import org.springframework.http.server.PathContainer; */ public class PathPatternParser { - private static final Log logger = LogFactory.getLog(PathPatternParser.class); - private boolean matchOptionalTrailingSeparator = true; private boolean caseSensitive = true; @@ -114,11 +109,6 @@ public class PathPatternParser { * @throws PatternParseException in case of parse errors */ public PathPattern parse(String pathPattern) throws PatternParseException { - int wildcardIndex = pathPattern.indexOf("**" + this.pathOptions.separator()); - if (wildcardIndex != -1 && wildcardIndex != pathPattern.length() - 3) { - logger.warn("'**' patterns are not supported in the middle of patterns and will be rejected in the future. " + - "Consider using '*' instead for matching a single path segment."); - } return new InternalPathPatternParser(this).parse(pathPattern); } diff --git a/spring-web/src/main/java/org/springframework/web/util/pattern/PatternParseException.java b/spring-web/src/main/java/org/springframework/web/util/pattern/PatternParseException.java index cfd62eccf45..82435759637 100644 --- a/spring-web/src/main/java/org/springframework/web/util/pattern/PatternParseException.java +++ b/spring-web/src/main/java/org/springframework/web/util/pattern/PatternParseException.java @@ -100,7 +100,7 @@ public class PatternParseException extends IllegalArgumentException { CANNOT_HAVE_ADJACENT_CAPTURES("Adjacent captures are not allowed"), ILLEGAL_CHARACTER_AT_START_OF_CAPTURE_DESCRIPTOR("Char ''{0}'' not allowed at start of captured variable name"), ILLEGAL_CHARACTER_IN_CAPTURE_DESCRIPTOR("Char ''{0}'' is not allowed in a captured variable name"), - NO_MORE_DATA_EXPECTED_AFTER_CAPTURE_THE_REST("No more pattern data allowed after '{*...}' pattern element"), + NO_MORE_DATA_EXPECTED_AFTER_CAPTURE_THE_REST("No more pattern data allowed after '{*...}' or '**' pattern element"), BADLY_FORMED_CAPTURE_THE_REST("Expected form when capturing the rest of the path is simply '{*...}'"), MISSING_REGEX_CONSTRAINT("Missing regex constraint on capture"), ILLEGAL_DOUBLE_CAPTURE("Not allowed to capture ''{0}'' twice in the same pattern"), diff --git a/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternParserTests.java b/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternParserTests.java index 8b6c204c4f7..6f032000680 100644 --- a/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternParserTests.java +++ b/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternParserTests.java @@ -28,6 +28,7 @@ import org.springframework.web.util.pattern.PatternParseException.PatternMessage import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.assertj.core.api.Assertions.fail; /** @@ -128,12 +129,12 @@ public class PathPatternParserTests { pathPattern = checkStructure("/{var:\\\\}"); PathElement next = pathPattern.getHeadSection().next; assertThat(next.getClass().getName()).isEqualTo(CaptureVariablePathElement.class.getName()); - assertMatches(pathPattern,"/\\"); + assertMatches(pathPattern, "/\\"); pathPattern = checkStructure("/{var:\\/}"); next = pathPattern.getHeadSection().next; assertThat(next.getClass().getName()).isEqualTo(CaptureVariablePathElement.class.getName()); - assertNoMatch(pathPattern,"/aaa"); + assertNoMatch(pathPattern, "/aaa"); pathPattern = checkStructure("/{var:a{1,2}}"); next = pathPattern.getHeadSection().next; @@ -142,25 +143,25 @@ public class PathPatternParserTests { pathPattern = checkStructure("/{var:[^\\/]*}"); next = pathPattern.getHeadSection().next; assertThat(next.getClass().getName()).isEqualTo(CaptureVariablePathElement.class.getName()); - PathPattern.PathMatchInfo result = matchAndExtract(pathPattern,"/foo"); + PathPattern.PathMatchInfo result = matchAndExtract(pathPattern, "/foo"); assertThat(result.getUriVariables().get("var")).isEqualTo("foo"); pathPattern = checkStructure("/{var:\\[*}"); next = pathPattern.getHeadSection().next; assertThat(next.getClass().getName()).isEqualTo(CaptureVariablePathElement.class.getName()); - result = matchAndExtract(pathPattern,"/[[["); + result = matchAndExtract(pathPattern, "/[[["); assertThat(result.getUriVariables().get("var")).isEqualTo("[[["); pathPattern = checkStructure("/{var:[\\{]*}"); next = pathPattern.getHeadSection().next; assertThat(next.getClass().getName()).isEqualTo(CaptureVariablePathElement.class.getName()); - result = matchAndExtract(pathPattern,"/{{{"); + result = matchAndExtract(pathPattern, "/{{{"); assertThat(result.getUriVariables().get("var")).isEqualTo("{{{"); pathPattern = checkStructure("/{var:[\\}]*}"); next = pathPattern.getHeadSection().next; assertThat(next.getClass().getName()).isEqualTo(CaptureVariablePathElement.class.getName()); - result = matchAndExtract(pathPattern,"/}}}"); + result = matchAndExtract(pathPattern, "/}}}"); assertThat(result.getUriVariables().get("var")).isEqualTo("}}}"); pathPattern = checkStructure("*"); @@ -249,10 +250,10 @@ public class PathPatternParserTests { PathPattern pp = parse("/{abc:foo(bar)}"); assertThatIllegalArgumentException().isThrownBy(() -> pp.matchAndExtract(toPSC("/foo"))) - .withMessage("No capture groups allowed in the constraint regex: foo(bar)"); + .withMessage("No capture groups allowed in the constraint regex: foo(bar)"); assertThatIllegalArgumentException().isThrownBy(() -> pp.matchAndExtract(toPSC("/foobar"))) - .withMessage("No capture groups allowed in the constraint regex: foo(bar)"); + .withMessage("No capture groups allowed in the constraint regex: foo(bar)"); } @Test @@ -414,12 +415,12 @@ public class PathPatternParserTests { assertThat(patterns.get(1)).isEqualTo(p2); } - @Test // Should be updated with gh-24952 - public void doubleWildcardWithinPatternNotSupported() { + @Test + public void captureTheRestWithinPatternNotSupported() { PathPatternParser parser = new PathPatternParser(); - PathPattern pattern = parser.parse("/resources/**/details"); - assertThat(pattern.matches(PathContainer.parsePath("/resources/test/details"))).isTrue(); - assertThat(pattern.matches(PathContainer.parsePath("/resources/projects/spring/details"))).isFalse(); + assertThatThrownBy(() -> parser.parse("/resources/**/details")) + .isInstanceOf(PatternParseException.class) + .extracting("messageType").isEqualTo(PatternMessage.NO_MORE_DATA_EXPECTED_AFTER_CAPTURE_THE_REST); } @Test @@ -461,16 +462,16 @@ public class PathPatternParserTests { String... expectedInserts) { assertThatExceptionOfType(PatternParseException.class) - .isThrownBy(() -> pathPattern = parse(pattern)) - .satisfies(ex -> { - if (expectedPos >= 0) { - assertThat(ex.getPosition()).as(ex.toDetailedString()).isEqualTo(expectedPos); - } - assertThat(ex.getMessageType()).as(ex.toDetailedString()).isEqualTo(expectedMessage); - if (expectedInserts.length != 0) { - assertThat(ex.getInserts()).isEqualTo(expectedInserts); - } - }); + .isThrownBy(() -> pathPattern = parse(pattern)) + .satisfies(ex -> { + if (expectedPos >= 0) { + assertThat(ex.getPosition()).as(ex.toDetailedString()).isEqualTo(expectedPos); + } + assertThat(ex.getMessageType()).as(ex.toDetailedString()).isEqualTo(expectedMessage); + if (expectedInserts.length != 0) { + assertThat(ex.getInserts()).isEqualTo(expectedInserts); + } + }); } @SafeVarargs diff --git a/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternTests.java b/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternTests.java index ebf95ff215c..ceeafb2e662 100644 --- a/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternTests.java +++ b/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 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. @@ -54,8 +54,10 @@ public class PathPatternTests { public void hasPatternSyntax() { PathPatternParser parser = new PathPatternParser(); assertThat(parser.parse("/foo/*").hasPatternSyntax()).isTrue(); - assertThat(parser.parse("/foo/**/bar").hasPatternSyntax()).isTrue(); + assertThat(parser.parse("/foo/**").hasPatternSyntax()).isFalse(); + assertThat(parser.parse("/foo/{*elem}").hasPatternSyntax()).isFalse(); assertThat(parser.parse("/f?o").hasPatternSyntax()).isTrue(); + assertThat(parser.parse("/f*").hasPatternSyntax()).isTrue(); assertThat(parser.parse("/foo/{bar}/baz").hasPatternSyntax()).isTrue(); assertThat(parser.parse("/foo/bar").hasPatternSyntax()).isFalse(); } @@ -867,13 +869,10 @@ public class PathPatternTests { assertThat(pathMatcher.combine("", "/hotels")).isEqualTo("/hotels"); assertThat(pathMatcher.combine("/hotels/*", "booking")).isEqualTo("/hotels/booking"); assertThat(pathMatcher.combine("/hotels/*", "/booking")).isEqualTo("/hotels/booking"); - assertThat(pathMatcher.combine("/hotels/**", "booking")).isEqualTo("/hotels/**/booking"); - assertThat(pathMatcher.combine("/hotels/**", "/booking")).isEqualTo("/hotels/**/booking"); assertThat(pathMatcher.combine("/hotels", "/booking")).isEqualTo("/hotels/booking"); assertThat(pathMatcher.combine("/hotels", "booking")).isEqualTo("/hotels/booking"); assertThat(pathMatcher.combine("/hotels/", "booking")).isEqualTo("/hotels/booking"); assertThat(pathMatcher.combine("/hotels/*", "{hotel}")).isEqualTo("/hotels/{hotel}"); - assertThat(pathMatcher.combine("/hotels/**", "{hotel}")).isEqualTo("/hotels/**/{hotel}"); assertThat(pathMatcher.combine("/hotels", "{hotel}")).isEqualTo("/hotels/{hotel}"); assertThat(pathMatcher.combine("/hotels", "{hotel}.*")).isEqualTo("/hotels/{hotel}.*"); assertThat(pathMatcher.combine("/hotels/*/booking", "{booking}")).isEqualTo("/hotels/*/booking/{booking}");