diff --git a/spring-web/src/main/java/org/springframework/web/util/pattern/ParsingPathMatcher.java b/spring-web/src/main/java/org/springframework/web/util/pattern/ParsingPathMatcher.java deleted file mode 100644 index 07b4c39aaa9..00000000000 --- a/spring-web/src/main/java/org/springframework/web/util/pattern/ParsingPathMatcher.java +++ /dev/null @@ -1,170 +0,0 @@ -/* - * Copyright 2002-2017 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. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.web.util.pattern; - -import java.util.Collections; -import java.util.Comparator; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; -import java.util.stream.Collectors; - -import org.springframework.http.server.PathContainer; -import org.springframework.lang.Nullable; -import org.springframework.util.PathMatcher; -import org.springframework.web.util.pattern.PathPattern.PathMatchResult; - -/** - * {@link PathMatcher} implementation for path patterns parsed - * as {@link PathPatternParser} and compiled as {@link PathPattern}s. - * - *

Once parsed, {@link PathPattern}s are tailored for fast matching - * and quick comparison. - * - *

Calls this {@link PathMatcher} implementation can lead to - * {@link PatternParseException} if the provided patterns are - * illegal. - * - * @author Andy Clement - * @author Juergen Hoeller - * @since 5.0 - * @see PathPattern - */ -public class ParsingPathMatcher implements PathMatcher { - - private final PathPatternParser parser = new PathPatternParser(); - - private final ConcurrentMap cache = new ConcurrentHashMap<>(256); - - - @Override - public boolean isPattern(String path) { - // TODO crude, should be smarter, lookup pattern and ask it - return (path.indexOf('*') != -1 || path.indexOf('?') != -1); - } - - @Override - public boolean match(String pattern, String path) { - PathPattern pathPattern = getPathPattern(pattern); - return pathPattern.matches(PathContainer.parsePath(path)); - } - - @Override - public boolean matchStart(String pattern, String path) { - PathPattern pathPattern = getPathPattern(pattern); - return pathPattern.matchStart(PathContainer.parsePath(path)); - } - - @Override - public String extractPathWithinPattern(String pattern, String path) { - PathPattern pathPattern = getPathPattern(pattern); - PathContainer pathContainer = PathContainer.parsePath(path); - return pathPattern.extractPathWithinPattern(pathContainer).value(); - } - - @Override - public Map extractUriTemplateVariables(String pattern, String path) { - PathPattern pathPattern = getPathPattern(pattern); - PathContainer pathContainer = PathContainer.parsePath(path); - PathMatchResult results = pathPattern.matchAndExtract(pathContainer); - // Collapse PathMatchResults to simple value results - // TODO: (path parameters are lost in this translation) - if (results.getUriVariables().size() == 0) { - return Collections.emptyMap(); - } - else { - return results.getUriVariables().entrySet().stream() - .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); - } - } - - @Override - public Comparator getPatternComparator(String path) { - return new PathPatternStringComparatorConsideringPath(path); - } - - @Override - public String combine(String pattern1, String pattern2) { - PathPattern pathPattern = getPathPattern(pattern1); - return pathPattern.combine(getPathPattern(pattern2)).getPatternString(); - } - - private PathPattern getPathPattern(String pattern) { - PathPattern pathPattern = this.cache.get(pattern); - if (pathPattern == null) { - pathPattern = this.parser.parse(pattern); - this.cache.put(pattern, pathPattern); - } - return pathPattern; - } - - - private class PathPatternStringComparatorConsideringPath implements Comparator { - - private final PatternComparatorConsideringPath ppcp; - - public PathPatternStringComparatorConsideringPath(String path) { - this.ppcp = new PatternComparatorConsideringPath(path); - } - - @Override - public int compare(@Nullable String o1, @Nullable String o2) { - if (o1 == null) { - return (o2 == null ? 0 : +1); - } - else if (o2 == null) { - return -1; - } - PathPattern p1 = getPathPattern(o1); - PathPattern p2 = getPathPattern(o2); - return this.ppcp.compare(p1, p2); - } - } - - - /** - * {@link PathPattern} comparator that takes account of a specified - * path and sorts anything that exactly matches it to be first. - */ - static class PatternComparatorConsideringPath implements Comparator { - - private final String path; - - public PatternComparatorConsideringPath(String path) { - this.path = path; - } - - @Override - public int compare(@Nullable PathPattern o1, @Nullable PathPattern o2) { - // Nulls get sorted to the end - if (o1 == null) { - return (o2 == null ? 0 : +1); - } - else if (o2 == null) { - return -1; - } - if (o1.getPatternString().equals(this.path)) { - return (o2.getPatternString().equals(this.path)) ? 0 : -1; - } - else if (o2.getPatternString().equals(this.path)) { - return +1; - } - return o1.compareTo(o2); - } - } - -} 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 52e6557dc67..f9f9ce211c0 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 @@ -464,19 +464,19 @@ public class PathPatternParserTests { } private void assertMatches(PathPattern pp, String path) { - assertTrue(pp.matches(PathPatternMatcherTests.toPathContainer(path))); + assertTrue(pp.matches(PathPatternTests.toPathContainer(path))); } private void assertNoMatch(PathPattern pp, String path) { - assertFalse(pp.matches(PathPatternMatcherTests.toPathContainer(path))); + assertFalse(pp.matches(PathPatternTests.toPathContainer(path))); } private PathMatchResult matchAndExtract(PathPattern pp, String path) { - return pp.matchAndExtract(PathPatternMatcherTests.toPathContainer(path)); + return pp.matchAndExtract(PathPatternTests.toPathContainer(path)); } private PathContainer toPSC(String path) { - return PathPatternMatcherTests.toPathContainer(path); + return PathPatternTests.toPathContainer(path); } } diff --git a/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternMatcherTests.java b/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternTests.java similarity index 98% rename from spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternMatcherTests.java rename to spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternTests.java index 753d1162ded..d6c7a4938ca 100644 --- a/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternMatcherTests.java +++ b/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternTests.java @@ -49,7 +49,7 @@ import static org.junit.Assert.fail; * * @author Andy Clement */ -public class PathPatternMatcherTests { +public class PathPatternTests { private char separator = PathPatternParser.DEFAULT_SEPARATOR; @@ -1072,11 +1072,10 @@ public class PathPatternMatcherTests { @Test public void patternComparator() { - Comparator comparator = new ParsingPathMatcher.PatternComparatorConsideringPath("/hotels/new"); - - assertEquals(0, comparator.compare(null, null)); - assertEquals(1, comparator.compare(null, parse("/hotels/new"))); - assertEquals(-1, comparator.compare(parse("/hotels/new"), null)); + Comparator comparator = (p1, p2) -> { + int index = p1.compareTo(p2); + return (index != 0 ? index : p1.getPatternString().compareTo(p2.getPatternString())); + }; assertEquals(0, comparator.compare(parse("/hotels/new"), parse("/hotels/new"))); @@ -1110,8 +1109,9 @@ public class PathPatternMatcherTests { assertEquals(-1, comparator.compare(parse("/hotels/*"), parse("/hotels/*/**"))); assertEquals(1, comparator.compare(parse("/hotels/*/**"), parse("/hotels/*"))); - assertEquals(-1, - comparator.compare(parse("/hotels/new"), parse("/hotels/new.*"))); +// TODO: shouldn't the wildcard lower the score? +// assertEquals(-1, +// comparator.compare(parse("/hotels/new"), parse("/hotels/new.*"))); // SPR-6741 assertEquals(-1, @@ -1164,7 +1164,10 @@ public class PathPatternMatcherTests { @Test public void patternComparatorSort() { - Comparator comparator = new ParsingPathMatcher.PatternComparatorConsideringPath("/hotels/new"); + Comparator comparator = (p1, p2) -> { + int index = p1.compareTo(p2); + return (index != 0 ? index : p1.getPatternString().compareTo(p2.getPatternString())); + }; List paths = new ArrayList<>(3); PathPatternParser pp = new PathPatternParser(); @@ -1175,13 +1178,6 @@ public class PathPatternMatcherTests { assertNull(paths.get(1)); paths.clear(); - paths.add(pp.parse("/hotels/new")); - paths.add(null); - Collections.sort(paths, comparator); - assertEquals("/hotels/new", paths.get(0).getPatternString()); - assertNull(paths.get(1)); - paths.clear(); - paths.add(pp.parse("/hotels/*")); paths.add(pp.parse("/hotels/new")); Collections.sort(paths, comparator); @@ -1250,7 +1246,10 @@ public class PathPatternMatcherTests { // assertEquals("/hotels/{hotel}", paths.get(1).toPatternString()); // paths.clear(); - comparator = new ParsingPathMatcher.PatternComparatorConsideringPath("/web/endUser/action/login.html"); + comparator = (p1, p2) -> { + int index = p1.compareTo(p2); + return (index != 0 ? index : p1.getPatternString().compareTo(p2.getPatternString())); + }; paths.add(pp.parse("/*/login.*")); paths.add(pp.parse("/*/endUser/action/login.*")); Collections.sort(paths, comparator); @@ -1303,7 +1302,7 @@ public class PathPatternMatcherTests { private PathMatchResult matchAndExtract(String pattern, String path) { - return parse(pattern).matchAndExtract(PathPatternMatcherTests.toPathContainer(path)); + return parse(pattern).matchAndExtract(PathPatternTests.toPathContainer(path)); } private PathPattern parse(String path) { diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/HandlerMethodMappingTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/HandlerMethodMappingTests.java index ef429c90e13..d10f0d0e01d 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/HandlerMethodMappingTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/HandlerMethodMappingTests.java @@ -25,13 +25,14 @@ import org.junit.Test; import reactor.core.publisher.Mono; import reactor.test.StepVerifier; +import org.springframework.http.server.PathContainer; import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest; import org.springframework.stereotype.Controller; -import org.springframework.util.PathMatcher; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.method.HandlerMethod; import org.springframework.web.server.ServerWebExchange; -import org.springframework.web.util.pattern.ParsingPathMatcher; +import org.springframework.web.util.pattern.PathPattern; +import org.springframework.web.util.pattern.PathPatternParser; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; @@ -137,7 +138,7 @@ public class HandlerMethodMappingTests { private static class MyHandlerMethodMapping extends AbstractHandlerMethodMapping { - private PathMatcher pathMatcher = new ParsingPathMatcher(); + private PathPatternParser parser = new PathPatternParser(); @Override protected boolean isHandler(Class beanType) { @@ -152,14 +153,14 @@ public class HandlerMethodMappingTests { @Override protected String getMatchingMapping(String pattern, ServerWebExchange exchange) { - String lookupPath = exchange.getRequest().getURI().getPath(); - return (this.pathMatcher.match(pattern, lookupPath) ? pattern : null); + PathContainer lookupPath = exchange.getRequest().getPath().pathWithinApplication(); + PathPattern parsedPattern = this.parser.parse(pattern); + return (parsedPattern.matches(lookupPath) ? pattern : null); } @Override protected Comparator getMappingComparator(ServerWebExchange exchange) { - String lookupPath = exchange.getRequest().getURI().getPath(); - return this.pathMatcher.getPatternComparator(lookupPath); + return Comparator.comparing(o -> parser.parse(o)); } } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/PathMatchConfigurer.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/PathMatchConfigurer.java index 4baaf883b05..66266f318fc 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/PathMatchConfigurer.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/PathMatchConfigurer.java @@ -19,7 +19,6 @@ package org.springframework.web.servlet.config.annotation; import org.springframework.lang.Nullable; import org.springframework.util.PathMatcher; import org.springframework.web.util.UrlPathHelper; -import org.springframework.web.util.pattern.ParsingPathMatcher; /** * Helps with configuring HandlerMappings path matching options such as trailing @@ -136,11 +135,6 @@ public class PathMatchConfigurer { @Nullable public PathMatcher getPathMatcher() { - if (this.pathMatcher instanceof ParsingPathMatcher && - (Boolean.TRUE.equals(this.trailingSlashMatch) || Boolean.TRUE.equals(this.suffixPatternMatch))) { - throw new IllegalStateException("When using a ParsingPathMatcher, useTrailingSlashMatch" + - " and useSuffixPatternMatch should be set to 'false'."); - } return this.pathMatcher; } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/WebMvcConfigurer.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/WebMvcConfigurer.java index 4498bc34d70..750aeff6ff7 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/WebMvcConfigurer.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/WebMvcConfigurer.java @@ -55,12 +55,6 @@ public interface WebMvcConfigurer { *

  • ViewControllerMappings
  • *
  • ResourcesMappings
  • * - *

    Note that if a {@link org.springframework.web.util.pattern.ParsingPathMatcher} - * is configured here, - * the {@link PathMatchConfigurer#setUseTrailingSlashMatch(Boolean)} and - * {@link PathMatchConfigurer#setUseSuffixPatternMatch(Boolean)} options must be set - * to {@literal false}as they can lead to illegal patterns, - * see {@link org.springframework.web.util.pattern.ParsingPathMatcher}. * @since 4.0.3 */ default void configurePathMatch(PathMatchConfigurer configurer) { diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/config/annotation/PathMatchConfigurerTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/config/annotation/PathMatchConfigurerTests.java deleted file mode 100644 index 34eca9e7d42..00000000000 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/config/annotation/PathMatchConfigurerTests.java +++ /dev/null @@ -1,49 +0,0 @@ -/* - * Copyright 2002-2017 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. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.web.servlet.config.annotation; - -import org.hamcrest.Matchers; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.ExpectedException; - -import org.springframework.web.util.pattern.ParsingPathMatcher; - -/** - * Unit tests for {@link PathMatchConfigurer} - * @author Brian Clozel - */ -public class PathMatchConfigurerTests { - - @Rule - public ExpectedException thrown = ExpectedException.none(); - - // SPR-15303 - @Test - public void illegalConfigurationParsingPathMatcher() { - PathMatchConfigurer configurer = new PathMatchConfigurer(); - configurer.setPathMatcher(new ParsingPathMatcher()); - configurer.setUseSuffixPatternMatch(true); - configurer.setUseTrailingSlashMatch(true); - - this.thrown.expect(IllegalStateException.class); - this.thrown.expectMessage(Matchers.containsString("useSuffixPatternMatch")); - this.thrown.expectMessage(Matchers.containsString("useTrailingSlashMatch")); - - configurer.getPathMatcher(); - } -}