Drop support for String path matching for MVC endpoints

Closes spring-projectsgh-31700
hyk001 · Jul 13, 2022 · 7c56a45 · 7c56a45
1 parent 92c530c
commit 7c56a45
Show file tree

Hide file tree

Showing 7 changed files with 10 additions and 124 deletions.
diff --git a/...uate/autoconfigure/cloudfoundry/servlet/CloudFoundryWebEndpointServletHandlerMapping.java b/...uate/autoconfigure/cloudfoundry/servlet/CloudFoundryWebEndpointServletHandlerMapping.java
@@ -63,8 +63,7 @@ class CloudFoundryWebEndpointServletHandlerMapping extends AbstractWebMvcEndpoin
 			Collection<ExposableWebEndpoint> endpoints, EndpointMediaTypes endpointMediaTypes,
 			CorsConfiguration corsConfiguration, CloudFoundrySecurityInterceptor securityInterceptor,
 			EndpointLinksResolver linksResolver) {
-		super(endpointMapping, endpoints, endpointMediaTypes, corsConfiguration, true,
-				AbstractWebMvcEndpointHandlerMapping.pathPatternParser);
+		super(endpointMapping, endpoints, endpointMediaTypes, corsConfiguration, true);
 		this.securityInterceptor = securityInterceptor;
 		this.linksResolver = linksResolver;
 	}

diff --git a/...uate/autoconfigure/endpoint/web/servlet/WebMvcEndpointManagementContextConfiguration.java b/...uate/autoconfigure/endpoint/web/servlet/WebMvcEndpointManagementContextConfiguration.java
@@ -37,7 +37,6 @@
 import org.springframework.boot.actuate.endpoint.web.WebServerNamespace;
 import org.springframework.boot.actuate.endpoint.web.annotation.ControllerEndpointsSupplier;
 import org.springframework.boot.actuate.endpoint.web.annotation.ServletEndpointsSupplier;
-import org.springframework.boot.actuate.endpoint.web.servlet.AbstractWebMvcEndpointHandlerMapping;
 import org.springframework.boot.actuate.endpoint.web.servlet.AdditionalHealthEndpointPathsWebMvcHandlerMapping;
 import org.springframework.boot.actuate.endpoint.web.servlet.ControllerEndpointHandlerMapping;
 import org.springframework.boot.actuate.endpoint.web.servlet.WebMvcEndpointHandlerMapping;
@@ -85,7 +84,7 @@ public WebMvcEndpointHandlerMapping webEndpointServletHandlerMapping(WebEndpoint
 		boolean shouldRegisterLinksMapping = shouldRegisterLinksMapping(webEndpointProperties, environment, basePath);
 		return new WebMvcEndpointHandlerMapping(endpointMapping, webEndpoints, endpointMediaTypes,
 				corsProperties.toCorsConfiguration(), new EndpointLinksResolver(allEndpoints, basePath),
-				shouldRegisterLinksMapping, AbstractWebMvcEndpointHandlerMapping.pathPatternParser);
+				shouldRegisterLinksMapping);
 	}
 
 	private boolean shouldRegisterLinksMapping(WebEndpointProperties webEndpointProperties, Environment environment,

diff --git a/...gframework/boot/actuate/autoconfigure/integrationtest/WebMvcEndpointIntegrationTests.java b/...gframework/boot/actuate/autoconfigure/integrationtest/WebMvcEndpointIntegrationTests.java
@@ -32,7 +32,6 @@
 import org.springframework.boot.actuate.endpoint.web.annotation.ControllerEndpoint;
 import org.springframework.boot.actuate.endpoint.web.annotation.RestControllerEndpoint;
 import org.springframework.boot.actuate.endpoint.web.annotation.ServletEndpoint;
-import org.springframework.boot.actuate.endpoint.web.servlet.AbstractWebMvcEndpointHandlerMapping;
 import org.springframework.boot.actuate.endpoint.web.servlet.WebMvcEndpointHandlerMapping;
 import org.springframework.boot.autoconfigure.ImportAutoConfiguration;
 import org.springframework.boot.autoconfigure.context.PropertyPlaceholderAutoConfiguration;
@@ -56,6 +55,7 @@
 import org.springframework.test.web.servlet.setup.DefaultMockMvcBuilder;
 import org.springframework.test.web.servlet.setup.MockMvcBuilders;
 import org.springframework.test.web.servlet.setup.MockMvcConfigurer;
+import org.springframework.web.util.pattern.PathPatternParser;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.hamcrest.Matchers.both;
@@ -87,7 +87,7 @@ void webMvcEndpointHandlerMappingIsConfiguredWithPathPatternParser() {
 		this.context.setServletContext(new MockServletContext());
 		this.context.refresh();
 		WebMvcEndpointHandlerMapping handlerMapping = this.context.getBean(WebMvcEndpointHandlerMapping.class);
-		assertThat(handlerMapping.getPatternParser()).isEqualTo(AbstractWebMvcEndpointHandlerMapping.pathPatternParser);
+		assertThat(handlerMapping.getPatternParser()).isInstanceOf(PathPatternParser.class);
 	}
 
 	@Test

diff --git a/...ringframework/boot/actuate/endpoint/web/servlet/AbstractWebMvcEndpointHandlerMapping.java b/...ringframework/boot/actuate/endpoint/web/servlet/AbstractWebMvcEndpointHandlerMapping.java
@@ -26,7 +26,6 @@
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 import java.util.function.Function;
 
 import jakarta.servlet.http.HttpServletRequest;
@@ -68,11 +67,8 @@
 import org.springframework.web.method.HandlerMethod;
 import org.springframework.web.server.ResponseStatusException;
 import org.springframework.web.servlet.HandlerMapping;
-import org.springframework.web.servlet.handler.MatchableHandlerMapping;
-import org.springframework.web.servlet.handler.RequestMatchResult;
 import org.springframework.web.servlet.mvc.method.RequestMappingInfo;
 import org.springframework.web.servlet.mvc.method.RequestMappingInfoHandlerMapping;
-import org.springframework.web.util.pattern.PathPatternParser;
 
 /**
  * A custom {@link HandlerMapping} that makes {@link ExposableWebEndpoint web endpoints}
@@ -85,7 +81,7 @@
  * @since 2.0.0
  */
 public abstract class AbstractWebMvcEndpointHandlerMapping extends RequestMappingInfoHandlerMapping
-		implements InitializingBean, MatchableHandlerMapping {
+		implements InitializingBean {
 
 	private final EndpointMapping endpointMapping;
 
@@ -102,11 +98,6 @@ public abstract class AbstractWebMvcEndpointHandlerMapping extends RequestMappin
 
 	private RequestMappingInfo.BuilderConfiguration builderConfig = new RequestMappingInfo.BuilderConfiguration();
 
-	/**
-	 * Instance of {@link PathPatternParser} shared across actuator configuration.
-	 */
-	public static final PathPatternParser pathPatternParser = new PathPatternParser();
-
 	/**
 	 * Creates a new {@code WebEndpointHandlerMapping} that provides mappings for the
 	 * operations of the given {@code webEndpoints}.
@@ -133,45 +124,19 @@ public AbstractWebMvcEndpointHandlerMapping(EndpointMapping endpointMapping,
 	public AbstractWebMvcEndpointHandlerMapping(EndpointMapping endpointMapping,
 			Collection<ExposableWebEndpoint> endpoints, EndpointMediaTypes endpointMediaTypes,
 			CorsConfiguration corsConfiguration, boolean shouldRegisterLinksMapping) {
-		this(endpointMapping, endpoints, endpointMediaTypes, corsConfiguration, shouldRegisterLinksMapping, null);
-	}
-
-	/**
-	 * Creates a new {@code AbstractWebMvcEndpointHandlerMapping} that provides mappings
-	 * for the operations of the given endpoints.
-	 * @param endpointMapping the base mapping for all endpoints
-	 * @param endpoints the web endpoints
-	 * @param endpointMediaTypes media types consumed and produced by the endpoints
-	 * @param corsConfiguration the CORS configuration for the endpoints or {@code null}
-	 * @param shouldRegisterLinksMapping whether the links endpoint should be registered
-	 * @param pathPatternParser the path pattern parser
-	 */
-	public AbstractWebMvcEndpointHandlerMapping(EndpointMapping endpointMapping,
-			Collection<ExposableWebEndpoint> endpoints, EndpointMediaTypes endpointMediaTypes,
-			CorsConfiguration corsConfiguration, boolean shouldRegisterLinksMapping,
-			PathPatternParser pathPatternParser) {
 		this.endpointMapping = endpointMapping;
 		this.endpoints = endpoints;
 		this.endpointMediaTypes = endpointMediaTypes;
 		this.corsConfiguration = corsConfiguration;
 		this.shouldRegisterLinksMapping = shouldRegisterLinksMapping;
-		setPatternParser(pathPatternParser);
 		setOrder(-100);
 	}
 
 	@Override
 	@SuppressWarnings("deprecation")
 	public void afterPropertiesSet() {
 		this.builderConfig = new RequestMappingInfo.BuilderConfiguration();
-		if (getPatternParser() != null) {
-			this.builderConfig.setPatternParser(getPatternParser());
-		}
-		else {
-			this.builderConfig.setPathMatcher(null);
-			this.builderConfig.setTrailingSlashMatch(true);
-			this.builderConfig.setSuffixPatternMatch(false);
-
-		}
+		this.builderConfig.setPatternParser(getPatternParser());
 		super.afterPropertiesSet();
 	}
 
@@ -193,19 +158,6 @@ protected HandlerMethod createHandlerMethod(Object handler, Method method) {
 		return new WebMvcEndpointHandlerMethod(handlerMethod.getBean(), handlerMethod.getMethod());
 	}
 
-	@Override
-	public RequestMatchResult match(HttpServletRequest request, String pattern) {
-		Assert.isNull(getPatternParser(), "This HandlerMapping uses PathPatterns.");
-		RequestMappingInfo info = RequestMappingInfo.paths(pattern).options(this.builderConfig).build();
-		RequestMappingInfo matchingInfo = info.getMatchingCondition(request);
-		if (matchingInfo == null) {
-			return null;
-		}
-		Set<String> patterns = matchingInfo.getPatternsCondition().getPatterns();
-		String lookupPath = getUrlPathHelper().getLookupPathForRequest(request);
-		return new RequestMatchResult(patterns.iterator().next(), lookupPath, getPathMatcher());
-	}
-
 	private void registerMappingForOperation(ExposableWebEndpoint endpoint, WebOperation operation) {
 		WebOperationRequestPredicate predicate = operation.getRequestPredicate();
 		String path = predicate.getPath();

diff --git a/...g/springframework/boot/actuate/endpoint/web/servlet/ControllerEndpointHandlerMapping.java b/...g/springframework/boot/actuate/endpoint/web/servlet/ControllerEndpointHandlerMapping.java
@@ -67,7 +67,6 @@ public ControllerEndpointHandlerMapping(EndpointMapping endpointMapping,
 		this.handlers = getHandlers(endpoints);
 		this.corsConfiguration = corsConfiguration;
 		setOrder(-100);
-		setUseSuffixPatternMatch(false);
 	}
 
 	private Map<Object, ExposableControllerEndpoint> getHandlers(Collection<ExposableControllerEndpoint> endpoints) {

diff --git a/...a/org/springframework/boot/actuate/endpoint/web/servlet/WebMvcEndpointHandlerMapping.java b/...a/org/springframework/boot/actuate/endpoint/web/servlet/WebMvcEndpointHandlerMapping.java
@@ -31,7 +31,6 @@
 import org.springframework.web.bind.annotation.ResponseBody;
 import org.springframework.web.cors.CorsConfiguration;
 import org.springframework.web.servlet.HandlerMapping;
-import org.springframework.web.util.pattern.PathPatternParser;
 
 /**
  * A custom {@link HandlerMapping} that makes web endpoints available over HTTP using
@@ -63,27 +62,6 @@ public WebMvcEndpointHandlerMapping(EndpointMapping endpointMapping, Collection<
 		setOrder(-100);
 	}
 
-	/**
-	 * Creates a new {@code WebMvcEndpointHandlerMapping} instance that provides mappings
-	 * for the given endpoints.
-	 * @param endpointMapping the base mapping for all endpoints
-	 * @param endpoints the web endpoints
-	 * @param endpointMediaTypes media types consumed and produced by the endpoints
-	 * @param corsConfiguration the CORS configuration for the endpoints or {@code null}
-	 * @param linksResolver resolver for determining links to available endpoints
-	 * @param shouldRegisterLinksMapping whether the links endpoint should be registered
-	 * @param pathPatternParser the path pattern parser
-	 */
-	public WebMvcEndpointHandlerMapping(EndpointMapping endpointMapping, Collection<ExposableWebEndpoint> endpoints,
-			EndpointMediaTypes endpointMediaTypes, CorsConfiguration corsConfiguration,
-			EndpointLinksResolver linksResolver, boolean shouldRegisterLinksMapping,
-			PathPatternParser pathPatternParser) {
-		super(endpointMapping, endpoints, endpointMediaTypes, corsConfiguration, shouldRegisterLinksMapping,
-				pathPatternParser);
-		this.linksResolver = linksResolver;
-		setOrder(-100);
-	}
-
 	@Override
 	protected LinksHandler getLinksHandler() {
 		return new WebMvcLinksHandler();

diff --git a/...org/springframework/boot/actuate/endpoint/web/servlet/MvcWebEndpointIntegrationTests.java b/...org/springframework/boot/actuate/endpoint/web/servlet/MvcWebEndpointIntegrationTests.java
@@ -45,7 +45,6 @@
 import org.springframework.core.env.Environment;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.MediaType;
-import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
 import org.springframework.security.core.authority.SimpleGrantedAuthority;
 import org.springframework.security.core.context.SecurityContext;
@@ -54,12 +53,8 @@
 import org.springframework.util.StringUtils;
 import org.springframework.web.cors.CorsConfiguration;
 import org.springframework.web.filter.OncePerRequestFilter;
-import org.springframework.web.servlet.handler.RequestMatchResult;
-import org.springframework.web.util.ServletRequestPathUtils;
-import org.springframework.web.util.pattern.PathPatternParser;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
 
 /**
  * Integration tests for web endpoints exposed using Spring MVC.
@@ -107,44 +102,9 @@ void readOperationsThatReturnAResourceSupportRangeRequests() {
 	}
 
 	@Test
-	void matchWhenPathPatternParserShouldThrowException() {
-		assertThatIllegalArgumentException().isThrownBy(() -> getMatchResult("/spring/", true));
-	}
-
-	@Test
-	void matchWhenRequestHasTrailingSlashShouldNotBeNull() {
-		assertThat(getMatchResult("/spring/", false)).isNotNull();
-	}
-
-	@Test
-	void matchWhenRequestHasSuffixShouldBeNull() {
-		assertThat(getMatchResult("/spring.do", false)).isNull();
-	}
-
-	private RequestMatchResult getMatchResult(String servletPath, boolean isPatternParser) {
-		MockHttpServletRequest request = new MockHttpServletRequest();
-		request.setServletPath(servletPath);
-		try (AnnotationConfigServletWebServerApplicationContext context = new AnnotationConfigServletWebServerApplicationContext()) {
-			if (isPatternParser) {
-				context.register(WebMvcConfiguration.class);
-			}
-			else {
-				context.register(PathMatcherWebMvcConfiguration.class);
-			}
-			context.register(TestEndpointConfiguration.class);
-			context.refresh();
-			WebMvcEndpointHandlerMapping bean = context.getBean(WebMvcEndpointHandlerMapping.class);
-			try {
-				// Setup request attributes
-				ServletRequestPathUtils.parseAndCache(request);
-				// Trigger initLookupPath
-				bean.getHandler(request);
-			}
-			catch (Exception ex) {
-				throw new RuntimeException(ex);
-			}
-			return bean.match(request, "/spring");
-		}
+	void requestWithSuffixShouldNotMatch() {
+		load(TestEndpointConfiguration.class, (client) -> client.options().uri("/test.do")
+				.accept(MediaType.APPLICATION_JSON).exchange().expectStatus().isNotFound());
 	}
 
 	@Override
@@ -172,8 +132,7 @@ WebMvcEndpointHandlerMapping webEndpointHandlerMapping(Environment environment,
 			String endpointPath = environment.getProperty("endpointPath");
 			return new WebMvcEndpointHandlerMapping(new EndpointMapping(endpointPath),
 					endpointDiscoverer.getEndpoints(), endpointMediaTypes, corsConfiguration,
-					new EndpointLinksResolver(endpointDiscoverer.getEndpoints()), StringUtils.hasText(endpointPath),
-					new PathPatternParser());
+					new EndpointLinksResolver(endpointDiscoverer.getEndpoints()), StringUtils.hasText(endpointPath));
 		}
 
 	}