From 57f868fcbd4a7d087b3d10a400058a5607a8cb02 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Wed, 24 Jun 2020 08:07:01 +0100 Subject: [PATCH] Direct matches by URL path work again This commit fixes a recent regression as a result of 5225a5741197a7e2123437709c2468e4537b0469 with the determination of non-pattern vs pattern URLs. That in turn affects the ability to perform direct matches by URL path. There is also a fix in PathPattern to recognize "catch-all" patterns as pattern syntax. See gh-24945 --- .../org/springframework/web/util/pattern/PathPattern.java | 2 +- .../springframework/web/util/pattern/PathPatternTests.java | 4 ++-- .../servlet/mvc/condition/PathPatternsRequestCondition.java | 2 +- .../web/servlet/mvc/condition/PatternsRequestCondition.java | 2 +- .../mvc/condition/PathPatternsRequestConditionTests.java | 6 ++++++ .../mvc/condition/PatternsRequestConditionTests.java | 6 ++++++ 6 files changed, 17 insertions(+), 5 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/util/pattern/PathPattern.java b/spring-web/src/main/java/org/springframework/web/util/pattern/PathPattern.java index 0b69eed122e..4c36f65b937 100644 --- a/spring-web/src/main/java/org/springframework/web/util/pattern/PathPattern.java +++ b/spring-web/src/main/java/org/springframework/web/util/pattern/PathPattern.java @@ -190,7 +190,7 @@ public class PathPattern implements Comparable { * @since 5.2 */ public boolean hasPatternSyntax() { - return (this.score > 0 || this.patternString.indexOf('?') != -1); + return (this.score > 0 || this.catchAll || this.patternString.indexOf('?') != -1); } /** 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 ceeafb2e662..b90836aab4a 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 @@ -54,8 +54,8 @@ public class PathPatternTests { public void hasPatternSyntax() { PathPatternParser parser = new PathPatternParser(); assertThat(parser.parse("/foo/*").hasPatternSyntax()).isTrue(); - assertThat(parser.parse("/foo/**").hasPatternSyntax()).isFalse(); - assertThat(parser.parse("/foo/{*elem}").hasPatternSyntax()).isFalse(); + assertThat(parser.parse("/foo/**").hasPatternSyntax()).isTrue(); + assertThat(parser.parse("/foo/{*elem}").hasPatternSyntax()).isTrue(); assertThat(parser.parse("/f?o").hasPatternSyntax()).isTrue(); assertThat(parser.parse("/f*").hasPatternSyntax()).isTrue(); assertThat(parser.parse("/foo/{bar}/baz").hasPatternSyntax()).isTrue(); diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/PathPatternsRequestCondition.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/PathPatternsRequestCondition.java index 00a0b204c5c..a89b4ed3cb6 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/PathPatternsRequestCondition.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/PathPatternsRequestCondition.java @@ -129,7 +129,7 @@ public final class PathPatternsRequestCondition extends AbstractRequestCondition } Set result = Collections.emptySet(); for (PathPattern pattern : this.patterns) { - if (pattern.hasPatternSyntax()) { + if (!pattern.hasPatternSyntax()) { result = (result.isEmpty() ? new HashSet<>(1) : result); result.add(pattern.getPatternString()); } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/PatternsRequestCondition.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/PatternsRequestCondition.java index 71eeff6144c..d04b26942f4 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/PatternsRequestCondition.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/PatternsRequestCondition.java @@ -218,7 +218,7 @@ public class PatternsRequestCondition extends AbstractRequestCondition result = Collections.emptySet(); for (String pattern : this.patterns) { - if (this.pathMatcher.isPattern(pattern)) { + if (!this.pathMatcher.isPattern(pattern)) { result = (result.isEmpty() ? new HashSet<>(1) : result); result.add(pattern); } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/PathPatternsRequestConditionTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/PathPatternsRequestConditionTests.java index bc82d1f0b93..52071bd97aa 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/PathPatternsRequestConditionTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/PathPatternsRequestConditionTests.java @@ -48,6 +48,12 @@ public class PathPatternsRequestConditionTests { .isEqualTo(""); } + @Test + void getDirectUrls() { + PathPatternsRequestCondition condition = createCondition("/something", "/else/**"); + assertThat(condition.getDirectPaths()).containsExactly("/something"); + } + @Test void combineEmptySets() { PathPatternsRequestCondition c1 = createCondition(); diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/PatternsRequestConditionTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/PatternsRequestConditionTests.java index 7957ccb184c..5150c0325bb 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/PatternsRequestConditionTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/PatternsRequestConditionTests.java @@ -47,6 +47,12 @@ class PatternsRequestConditionTests { .isEqualTo(""); } + @Test + void getDirectUrls() { + PatternsRequestCondition condition = new PatternsRequestCondition("/something", "/else/**"); + assertThat(condition.getDirectPaths()).containsExactly("/something"); + } + @Test void combineEmptySets() { PatternsRequestCondition c1 = new PatternsRequestCondition();