From 0557404715f205e13001aec19c3d0d3d1ca3a0ed Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Thu, 1 Jun 2017 18:22:05 +0200 Subject: [PATCH] Throw exception for illegal PathMatch configurations With the new `ParsingPathMatcher` implementation, new patterns are now allowed, such as `"/foo/{*bar}". The `"{*bar}"` segment will capture everything until the end of the given path. Adding other elements after that segment is illegal and will throw exceptions. One can configure on a `PathMatchConfigurer` various options like `useTrailingSlashMatch` and `useSuffixPatternMatch`; those options, when enabled, will try to append suffixes like `".*"` and `"/"` to existing path patterns. In case of a "capture the rest" pattern segment, those options won't be honored. This is why this commit ensures that an exception is thrown at the start of the application if an illegal configuration is applied to the `PathMatchConfigurer`. Issue: SPR-15303, SPR-15558 --- .../web/util/pattern/ParsingPathMatcher.java | 4 ++ .../reactive/config/PathMatchConfigurer.java | 9 +++- .../reactive/config/WebFluxConfigurer.java | 9 +++- .../config/PathMatchConfigurerTests.java | 49 +++++++++++++++++++ .../annotation/PathMatchConfigurer.java | 7 +++ .../config/annotation/WebMvcConfigurer.java | 8 +++ .../annotation/PathMatchConfigurerTests.java | 49 +++++++++++++++++++ 7 files changed, 133 insertions(+), 2 deletions(-) create mode 100644 spring-webflux/src/test/java/org/springframework/web/reactive/config/PathMatchConfigurerTests.java create mode 100644 spring-webmvc/src/test/java/org/springframework/web/servlet/config/annotation/PathMatchConfigurerTests.java 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 index e78ce3901650..1f3d82a44343 100644 --- 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 @@ -30,6 +30,10 @@ *

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 diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/config/PathMatchConfigurer.java b/spring-webflux/src/main/java/org/springframework/web/reactive/config/PathMatchConfigurer.java index 9f256437fef9..20e73f21bbd0 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/config/PathMatchConfigurer.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/config/PathMatchConfigurer.java @@ -18,6 +18,7 @@ import org.springframework.util.PathMatcher; import org.springframework.web.server.support.HttpRequestPathHelper; +import org.springframework.web.util.pattern.ParsingPathMatcher; /** * Assist with configuring {@code HandlerMapping}'s with path matching options. @@ -105,7 +106,13 @@ protected HttpRequestPathHelper getPathHelper() { return this.pathHelper; } - protected PathMatcher getPathMatcher() { + public PathMatcher getPathMatcher() { + if(this.pathMatcher != null + && this.pathMatcher.getClass().isAssignableFrom(ParsingPathMatcher.class) + && (this.trailingSlashMatch || this.suffixPatternMatch)) { + throw new IllegalStateException("When using a ParsingPathMatcher, useTrailingSlashMatch" + + " and useSuffixPatternMatch should be set to 'false'."); + } return this.pathMatcher; } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/config/WebFluxConfigurer.java b/spring-webflux/src/main/java/org/springframework/web/reactive/config/WebFluxConfigurer.java index 43519bdc69fc..3560202e783b 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/config/WebFluxConfigurer.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/config/WebFluxConfigurer.java @@ -63,7 +63,14 @@ default void addCorsMappings(CorsRegistry registry) { /** * Configure path matching options. - *

The given configurer assists with configuring + * + *

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}. + * * {@code HandlerMapping}s with path matching options. * @param configurer the {@link PathMatchConfigurer} instance */ diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/config/PathMatchConfigurerTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/config/PathMatchConfigurerTests.java new file mode 100644 index 000000000000..4b8fa4f7f07a --- /dev/null +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/config/PathMatchConfigurerTests.java @@ -0,0 +1,49 @@ +/* + * 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.reactive.config; + +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(); + } +} 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 35fa13e31720..bdcdb309948b 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 @@ -18,6 +18,7 @@ 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 @@ -123,6 +124,12 @@ public UrlPathHelper getUrlPathHelper() { } public PathMatcher getPathMatcher() { + if(this.pathMatcher != null + && this.pathMatcher.getClass().isAssignableFrom(ParsingPathMatcher.class) + && (this.trailingSlashMatch || 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 750aeff6ff74..551487248599 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,6 +55,14 @@ 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 new file mode 100644 index 000000000000..34eca9e7d429 --- /dev/null +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/config/annotation/PathMatchConfigurerTests.java @@ -0,0 +1,49 @@ +/* + * 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(); + } +}