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

spring-web/src/main/java/org/springframework/web/util/pattern/ParsingPathMatcher.java

@@ -30,6 +30,10 @@
  * <p>Once parsed, {@link PathPattern}s are tailored for fast matching
  * and quick comparison.
  *
+ * <p>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

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;
 	}
 

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.
-	 * <p>The given configurer assists with configuring
+	 * 
+	 * <p>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
 	 */

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();
+	}
+}

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;
 	}
 

spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/WebMvcConfigurer.java

@@ -55,6 +55,14 @@ public interface WebMvcConfigurer {
 	 * <li>ViewControllerMappings</li>
 	 * <li>ResourcesMappings</li>
 	 * </ul>
+	 *
+	 * <p>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) {

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();
+	}
+}