From 7c56a45d3efd3462caeadda4fcd5f075cea8ce0b Mon Sep 17 00:00:00 2001 From: Madhura Bhave Date: Wed, 13 Jul 2022 14:00:14 -0700 Subject: [PATCH] Drop support for String path matching for MVC endpoints Closes gh-31700 --- ...undryWebEndpointServletHandlerMapping.java | 3 +- ...ndpointManagementContextConfiguration.java | 3 +- .../WebMvcEndpointIntegrationTests.java | 4 +- .../AbstractWebMvcEndpointHandlerMapping.java | 52 +------------------ .../ControllerEndpointHandlerMapping.java | 1 - .../servlet/WebMvcEndpointHandlerMapping.java | 22 -------- .../MvcWebEndpointIntegrationTests.java | 49 ++--------------- 7 files changed, 10 insertions(+), 124 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/cloudfoundry/servlet/CloudFoundryWebEndpointServletHandlerMapping.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/cloudfoundry/servlet/CloudFoundryWebEndpointServletHandlerMapping.java index a0bef7334a5..fd8fe0e74cb 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/cloudfoundry/servlet/CloudFoundryWebEndpointServletHandlerMapping.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/cloudfoundry/servlet/CloudFoundryWebEndpointServletHandlerMapping.java @@ -63,8 +63,7 @@ class CloudFoundryWebEndpointServletHandlerMapping extends AbstractWebMvcEndpoin Collection 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/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/endpoint/web/servlet/WebMvcEndpointManagementContextConfiguration.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/endpoint/web/servlet/WebMvcEndpointManagementContextConfiguration.java index b51477ef9f4..69ddfd704fa 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/endpoint/web/servlet/WebMvcEndpointManagementContextConfiguration.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/endpoint/web/servlet/WebMvcEndpointManagementContextConfiguration.java @@ -37,7 +37,6 @@ import org.springframework.boot.actuate.endpoint.web.WebEndpointsSupplier; 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 class WebMvcEndpointManagementContextConfiguration { 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/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/integrationtest/WebMvcEndpointIntegrationTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/integrationtest/WebMvcEndpointIntegrationTests.java index 42468aa7438..ca6eea6b823 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/integrationtest/WebMvcEndpointIntegrationTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/integrationtest/WebMvcEndpointIntegrationTests.java @@ -32,7 +32,6 @@ import org.springframework.boot.actuate.endpoint.web.EndpointServlet; 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.MockMvc; 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 @@ class WebMvcEndpointIntegrationTests { 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/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/servlet/AbstractWebMvcEndpointHandlerMapping.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/servlet/AbstractWebMvcEndpointHandlerMapping.java index 38b0413b917..3bc469d87c7 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/servlet/AbstractWebMvcEndpointHandlerMapping.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/servlet/AbstractWebMvcEndpointHandlerMapping.java @@ -26,7 +26,6 @@ import java.util.Collections; 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.cors.CorsConfiguration; 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 @@ import org.springframework.web.util.pattern.PathPatternParser; * @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,29 +124,11 @@ public abstract class AbstractWebMvcEndpointHandlerMapping extends RequestMappin public AbstractWebMvcEndpointHandlerMapping(EndpointMapping endpointMapping, Collection 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 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); } @@ -163,15 +136,7 @@ public abstract class AbstractWebMvcEndpointHandlerMapping extends RequestMappin @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 @@ public abstract class AbstractWebMvcEndpointHandlerMapping extends RequestMappin 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 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/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/servlet/ControllerEndpointHandlerMapping.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/servlet/ControllerEndpointHandlerMapping.java index fffa49cea81..dc8c65163d6 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/servlet/ControllerEndpointHandlerMapping.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/servlet/ControllerEndpointHandlerMapping.java @@ -67,7 +67,6 @@ public class ControllerEndpointHandlerMapping extends RequestMappingHandlerMappi this.handlers = getHandlers(endpoints); this.corsConfiguration = corsConfiguration; setOrder(-100); - setUseSuffixPatternMatch(false); } private Map getHandlers(Collection endpoints) { diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/servlet/WebMvcEndpointHandlerMapping.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/servlet/WebMvcEndpointHandlerMapping.java index b6000f9c158..a6f3cfb96d4 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/servlet/WebMvcEndpointHandlerMapping.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/servlet/WebMvcEndpointHandlerMapping.java @@ -31,7 +31,6 @@ import org.springframework.boot.actuate.endpoint.web.Link; 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 class WebMvcEndpointHandlerMapping extends AbstractWebMvcEndpointHandlerM 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 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/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/web/servlet/MvcWebEndpointIntegrationTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/web/servlet/MvcWebEndpointIntegrationTests.java index 95ddd8a643a..d793e7d52e7 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/web/servlet/MvcWebEndpointIntegrationTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/web/servlet/MvcWebEndpointIntegrationTests.java @@ -45,7 +45,6 @@ import org.springframework.context.annotation.Configuration; 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.security.web.servletapi.SecurityContextHolderAwareReq 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 @@ class MvcWebEndpointIntegrationTests } @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 @@ class MvcWebEndpointIntegrationTests 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)); } }