Drop support for String path matching for MVC endpoints

Closes gh-31700
This commit is contained in:
Madhura Bhave 2022-07-13 14:00:14 -07:00
parent 92c530c111
commit 7c56a45d3e
7 changed files with 10 additions and 124 deletions

View File

@ -63,8 +63,7 @@ class CloudFoundryWebEndpointServletHandlerMapping extends AbstractWebMvcEndpoin
Collection<ExposableWebEndpoint> endpoints, EndpointMediaTypes endpointMediaTypes, Collection<ExposableWebEndpoint> endpoints, EndpointMediaTypes endpointMediaTypes,
CorsConfiguration corsConfiguration, CloudFoundrySecurityInterceptor securityInterceptor, CorsConfiguration corsConfiguration, CloudFoundrySecurityInterceptor securityInterceptor,
EndpointLinksResolver linksResolver) { EndpointLinksResolver linksResolver) {
super(endpointMapping, endpoints, endpointMediaTypes, corsConfiguration, true, super(endpointMapping, endpoints, endpointMediaTypes, corsConfiguration, true);
AbstractWebMvcEndpointHandlerMapping.pathPatternParser);
this.securityInterceptor = securityInterceptor; this.securityInterceptor = securityInterceptor;
this.linksResolver = linksResolver; this.linksResolver = linksResolver;
} }

View File

@ -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.WebServerNamespace;
import org.springframework.boot.actuate.endpoint.web.annotation.ControllerEndpointsSupplier; 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.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.AdditionalHealthEndpointPathsWebMvcHandlerMapping;
import org.springframework.boot.actuate.endpoint.web.servlet.ControllerEndpointHandlerMapping; import org.springframework.boot.actuate.endpoint.web.servlet.ControllerEndpointHandlerMapping;
import org.springframework.boot.actuate.endpoint.web.servlet.WebMvcEndpointHandlerMapping; import org.springframework.boot.actuate.endpoint.web.servlet.WebMvcEndpointHandlerMapping;
@ -85,7 +84,7 @@ public class WebMvcEndpointManagementContextConfiguration {
boolean shouldRegisterLinksMapping = shouldRegisterLinksMapping(webEndpointProperties, environment, basePath); boolean shouldRegisterLinksMapping = shouldRegisterLinksMapping(webEndpointProperties, environment, basePath);
return new WebMvcEndpointHandlerMapping(endpointMapping, webEndpoints, endpointMediaTypes, return new WebMvcEndpointHandlerMapping(endpointMapping, webEndpoints, endpointMediaTypes,
corsProperties.toCorsConfiguration(), new EndpointLinksResolver(allEndpoints, basePath), corsProperties.toCorsConfiguration(), new EndpointLinksResolver(allEndpoints, basePath),
shouldRegisterLinksMapping, AbstractWebMvcEndpointHandlerMapping.pathPatternParser); shouldRegisterLinksMapping);
} }
private boolean shouldRegisterLinksMapping(WebEndpointProperties webEndpointProperties, Environment environment, private boolean shouldRegisterLinksMapping(WebEndpointProperties webEndpointProperties, Environment environment,

View File

@ -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.ControllerEndpoint;
import org.springframework.boot.actuate.endpoint.web.annotation.RestControllerEndpoint; 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.annotation.ServletEndpoint;
import org.springframework.boot.actuate.endpoint.web.servlet.AbstractWebMvcEndpointHandlerMapping;
import org.springframework.boot.actuate.endpoint.web.servlet.WebMvcEndpointHandlerMapping; import org.springframework.boot.actuate.endpoint.web.servlet.WebMvcEndpointHandlerMapping;
import org.springframework.boot.autoconfigure.ImportAutoConfiguration; import org.springframework.boot.autoconfigure.ImportAutoConfiguration;
import org.springframework.boot.autoconfigure.context.PropertyPlaceholderAutoConfiguration; 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.DefaultMockMvcBuilder;
import org.springframework.test.web.servlet.setup.MockMvcBuilders; import org.springframework.test.web.servlet.setup.MockMvcBuilders;
import org.springframework.test.web.servlet.setup.MockMvcConfigurer; 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.assertj.core.api.Assertions.assertThat;
import static org.hamcrest.Matchers.both; import static org.hamcrest.Matchers.both;
@ -87,7 +87,7 @@ class WebMvcEndpointIntegrationTests {
this.context.setServletContext(new MockServletContext()); this.context.setServletContext(new MockServletContext());
this.context.refresh(); this.context.refresh();
WebMvcEndpointHandlerMapping handlerMapping = this.context.getBean(WebMvcEndpointHandlerMapping.class); WebMvcEndpointHandlerMapping handlerMapping = this.context.getBean(WebMvcEndpointHandlerMapping.class);
assertThat(handlerMapping.getPatternParser()).isEqualTo(AbstractWebMvcEndpointHandlerMapping.pathPatternParser); assertThat(handlerMapping.getPatternParser()).isInstanceOf(PathPatternParser.class);
} }
@Test @Test

View File

@ -26,7 +26,6 @@ import java.util.Collections;
import java.util.LinkedHashMap; import java.util.LinkedHashMap;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Set;
import java.util.function.Function; import java.util.function.Function;
import jakarta.servlet.http.HttpServletRequest; 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.method.HandlerMethod;
import org.springframework.web.server.ResponseStatusException; import org.springframework.web.server.ResponseStatusException;
import org.springframework.web.servlet.HandlerMapping; 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.RequestMappingInfo;
import org.springframework.web.servlet.mvc.method.RequestMappingInfoHandlerMapping; 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} * 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 * @since 2.0.0
*/ */
public abstract class AbstractWebMvcEndpointHandlerMapping extends RequestMappingInfoHandlerMapping public abstract class AbstractWebMvcEndpointHandlerMapping extends RequestMappingInfoHandlerMapping
implements InitializingBean, MatchableHandlerMapping { implements InitializingBean {
private final EndpointMapping endpointMapping; private final EndpointMapping endpointMapping;
@ -102,11 +98,6 @@ public abstract class AbstractWebMvcEndpointHandlerMapping extends RequestMappin
private RequestMappingInfo.BuilderConfiguration builderConfig = new RequestMappingInfo.BuilderConfiguration(); 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 * Creates a new {@code WebEndpointHandlerMapping} that provides mappings for the
* operations of the given {@code webEndpoints}. * operations of the given {@code webEndpoints}.
@ -133,29 +124,11 @@ public abstract class AbstractWebMvcEndpointHandlerMapping extends RequestMappin
public AbstractWebMvcEndpointHandlerMapping(EndpointMapping endpointMapping, public AbstractWebMvcEndpointHandlerMapping(EndpointMapping endpointMapping,
Collection<ExposableWebEndpoint> endpoints, EndpointMediaTypes endpointMediaTypes, Collection<ExposableWebEndpoint> endpoints, EndpointMediaTypes endpointMediaTypes,
CorsConfiguration corsConfiguration, boolean shouldRegisterLinksMapping) { 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.endpointMapping = endpointMapping;
this.endpoints = endpoints; this.endpoints = endpoints;
this.endpointMediaTypes = endpointMediaTypes; this.endpointMediaTypes = endpointMediaTypes;
this.corsConfiguration = corsConfiguration; this.corsConfiguration = corsConfiguration;
this.shouldRegisterLinksMapping = shouldRegisterLinksMapping; this.shouldRegisterLinksMapping = shouldRegisterLinksMapping;
setPatternParser(pathPatternParser);
setOrder(-100); setOrder(-100);
} }
@ -163,15 +136,7 @@ public abstract class AbstractWebMvcEndpointHandlerMapping extends RequestMappin
@SuppressWarnings("deprecation") @SuppressWarnings("deprecation")
public void afterPropertiesSet() { public void afterPropertiesSet() {
this.builderConfig = new RequestMappingInfo.BuilderConfiguration(); this.builderConfig = new RequestMappingInfo.BuilderConfiguration();
if (getPatternParser() != null) {
this.builderConfig.setPatternParser(getPatternParser()); this.builderConfig.setPatternParser(getPatternParser());
}
else {
this.builderConfig.setPathMatcher(null);
this.builderConfig.setTrailingSlashMatch(true);
this.builderConfig.setSuffixPatternMatch(false);
}
super.afterPropertiesSet(); super.afterPropertiesSet();
} }
@ -193,19 +158,6 @@ public abstract class AbstractWebMvcEndpointHandlerMapping extends RequestMappin
return new WebMvcEndpointHandlerMethod(handlerMethod.getBean(), handlerMethod.getMethod()); 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) { private void registerMappingForOperation(ExposableWebEndpoint endpoint, WebOperation operation) {
WebOperationRequestPredicate predicate = operation.getRequestPredicate(); WebOperationRequestPredicate predicate = operation.getRequestPredicate();
String path = predicate.getPath(); String path = predicate.getPath();

View File

@ -67,7 +67,6 @@ public class ControllerEndpointHandlerMapping extends RequestMappingHandlerMappi
this.handlers = getHandlers(endpoints); this.handlers = getHandlers(endpoints);
this.corsConfiguration = corsConfiguration; this.corsConfiguration = corsConfiguration;
setOrder(-100); setOrder(-100);
setUseSuffixPatternMatch(false);
} }
private Map<Object, ExposableControllerEndpoint> getHandlers(Collection<ExposableControllerEndpoint> endpoints) { private Map<Object, ExposableControllerEndpoint> getHandlers(Collection<ExposableControllerEndpoint> endpoints) {

View File

@ -31,7 +31,6 @@ import org.springframework.boot.actuate.endpoint.web.Link;
import org.springframework.web.bind.annotation.ResponseBody; import org.springframework.web.bind.annotation.ResponseBody;
import org.springframework.web.cors.CorsConfiguration; import org.springframework.web.cors.CorsConfiguration;
import org.springframework.web.servlet.HandlerMapping; 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 * A custom {@link HandlerMapping} that makes web endpoints available over HTTP using
@ -63,27 +62,6 @@ public class WebMvcEndpointHandlerMapping extends AbstractWebMvcEndpointHandlerM
setOrder(-100); 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 @Override
protected LinksHandler getLinksHandler() { protected LinksHandler getLinksHandler() {
return new WebMvcLinksHandler(); return new WebMvcLinksHandler();

View File

@ -45,7 +45,6 @@ import org.springframework.context.annotation.Configuration;
import org.springframework.core.env.Environment; import org.springframework.core.env.Environment;
import org.springframework.http.HttpStatus; import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType; import org.springframework.http.MediaType;
import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
import org.springframework.security.core.authority.SimpleGrantedAuthority; import org.springframework.security.core.authority.SimpleGrantedAuthority;
import org.springframework.security.core.context.SecurityContext; 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.util.StringUtils;
import org.springframework.web.cors.CorsConfiguration; import org.springframework.web.cors.CorsConfiguration;
import org.springframework.web.filter.OncePerRequestFilter; 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.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
/** /**
* Integration tests for web endpoints exposed using Spring MVC. * Integration tests for web endpoints exposed using Spring MVC.
@ -107,44 +102,9 @@ class MvcWebEndpointIntegrationTests
} }
@Test @Test
void matchWhenPathPatternParserShouldThrowException() { void requestWithSuffixShouldNotMatch() {
assertThatIllegalArgumentException().isThrownBy(() -> getMatchResult("/spring/", true)); load(TestEndpointConfiguration.class, (client) -> client.options().uri("/test.do")
} .accept(MediaType.APPLICATION_JSON).exchange().expectStatus().isNotFound());
@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");
}
} }
@Override @Override
@ -172,8 +132,7 @@ class MvcWebEndpointIntegrationTests
String endpointPath = environment.getProperty("endpointPath"); String endpointPath = environment.getProperty("endpointPath");
return new WebMvcEndpointHandlerMapping(new EndpointMapping(endpointPath), return new WebMvcEndpointHandlerMapping(new EndpointMapping(endpointPath),
endpointDiscoverer.getEndpoints(), endpointMediaTypes, corsConfiguration, endpointDiscoverer.getEndpoints(), endpointMediaTypes, corsConfiguration,
new EndpointLinksResolver(endpointDiscoverer.getEndpoints()), StringUtils.hasText(endpointPath), new EndpointLinksResolver(endpointDiscoverer.getEndpoints()), StringUtils.hasText(endpointPath));
new PathPatternParser());
} }
} }