Skip to content

Commit

Permalink
Enable PathPattern based matching for MVC actuators
Browse files Browse the repository at this point in the history 
Closes gh-24645
  • Loading branch information
@mbhave
mbhave committed Sep 8, 2021
1 parent c83aac6 commit 393081f
Show file tree
Hide file tree
Showing 7 changed files with 135 additions and 23 deletions.
4 changes: 3 additions & 1 deletion ...uate/autoconfigure/cloudfoundry/servlet/CloudFoundryWebEndpointServletHandlerMapping.java
View file
Expand Up @@ -38,6 +38,7 @@
import org.springframework.boot.actuate.endpoint.web.Link;
import org.springframework.boot.actuate.endpoint.web.WebOperation;
import org.springframework.boot.actuate.endpoint.web.servlet.AbstractWebMvcEndpointHandlerMapping;
import org.springframework.boot.autoconfigure.web.servlet.WebMvcAutoConfiguration;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.ResponseBody;
Expand All @@ -64,7 +65,8 @@ class CloudFoundryWebEndpointServletHandlerMapping extends AbstractWebMvcEndpoin
Collection<ExposableWebEndpoint> endpoints, EndpointMediaTypes endpointMediaTypes,
CorsConfiguration corsConfiguration, CloudFoundrySecurityInterceptor securityInterceptor,
EndpointLinksResolver linksResolver) {
super(endpointMapping, endpoints, endpointMediaTypes, corsConfiguration, true);
super(endpointMapping, endpoints, endpointMediaTypes, corsConfiguration, true,
WebMvcAutoConfiguration.pathPatternParser);
this.securityInterceptor = securityInterceptor;
this.linksResolver = linksResolver;
}
Expand Down
3 changes: 2 additions & 1 deletion ...uate/autoconfigure/endpoint/web/servlet/WebMvcEndpointManagementContextConfiguration.java
View file
Expand Up @@ -45,6 +45,7 @@
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication;
import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication.Type;
import org.springframework.boot.autoconfigure.web.servlet.WebMvcAutoConfiguration;
import org.springframework.boot.context.properties.EnableConfigurationProperties;
import org.springframework.context.annotation.Bean;
import org.springframework.core.env.Environment;
Expand Down Expand Up @@ -82,7 +83,7 @@ public WebMvcEndpointHandlerMapping webEndpointServletHandlerMapping(WebEndpoint
boolean shouldRegisterLinksMapping = shouldRegisterLinksMapping(webEndpointProperties, environment, basePath);
return new WebMvcEndpointHandlerMapping(endpointMapping, webEndpoints, endpointMediaTypes,
corsProperties.toCorsConfiguration(), new EndpointLinksResolver(allEndpoints, basePath),
shouldRegisterLinksMapping);
shouldRegisterLinksMapping, WebMvcAutoConfiguration.pathPatternParser);
}

private boolean shouldRegisterLinksMapping(WebEndpointProperties webEndpointProperties, Environment environment,
Expand Down
12 changes: 12 additions & 0 deletions ...gframework/boot/actuate/autoconfigure/integrationtest/WebMvcEndpointIntegrationTests.java
View file
Expand Up @@ -33,6 +33,7 @@
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.WebMvcEndpointHandlerMapping;
import org.springframework.boot.autoconfigure.ImportAutoConfiguration;
import org.springframework.boot.autoconfigure.context.PropertyPlaceholderAutoConfiguration;
import org.springframework.boot.autoconfigure.data.rest.RepositoryRestMvcAutoConfiguration;
Expand All @@ -56,6 +57,7 @@
import org.springframework.test.web.servlet.setup.MockMvcBuilders;
import org.springframework.test.web.servlet.setup.MockMvcConfigurer;

import static org.assertj.core.api.Assertions.assertThat;
import static org.hamcrest.Matchers.both;
import static org.hamcrest.Matchers.hasKey;
import static org.springframework.security.test.web.servlet.setup.SecurityMockMvcConfigurers.springSecurity;
Expand All @@ -78,6 +80,16 @@ void close() {
this.context.close();
}

@Test
void webMvcEndpointHandlerMappingIsConfiguredWithPathPatternParser() {
this.context = new AnnotationConfigServletWebApplicationContext();
this.context.register(DefaultConfiguration.class);
this.context.setServletContext(new MockServletContext());
this.context.refresh();
WebMvcEndpointHandlerMapping handlerMapping = this.context.getBean(WebMvcEndpointHandlerMapping.class);
assertThat(handlerMapping.getPatternParser()).isEqualTo(WebMvcAutoConfiguration.pathPatternParser);
}

@Test
void endpointsAreSecureByDefault() throws Exception {
this.context = new AnnotationConfigServletWebApplicationContext();
Expand Down
56 changes: 41 additions & 15 deletions ...ringframework/boot/actuate/endpoint/web/servlet/AbstractWebMvcEndpointHandlerMapping.java
View file
Expand Up @@ -67,7 +67,7 @@
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.UrlPathHelper;
import org.springframework.web.util.pattern.PathPatternParser;

/**
* A custom {@link HandlerMapping} that makes {@link ExposableWebEndpoint web endpoints}
Expand Down Expand Up @@ -95,7 +95,7 @@ public abstract class AbstractWebMvcEndpointHandlerMapping extends RequestMappin
private final Method handleMethod = ReflectionUtils.findMethod(OperationHandler.class, "handle",
HttpServletRequest.class, Map.class);

private static final RequestMappingInfo.BuilderConfiguration builderConfig = getBuilderConfig();
private RequestMappingInfo.BuilderConfiguration builderConfig = new RequestMappingInfo.BuilderConfiguration();

/**
* Creates a new {@code WebEndpointHandlerMapping} that provides mappings for the
Expand Down Expand Up @@ -123,14 +123,48 @@ 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);

}
super.afterPropertiesSet();
}

@Override
protected void initHandlerMethods() {
for (ExposableWebEndpoint endpoint : this.endpoints) {
Expand All @@ -151,7 +185,8 @@ protected HandlerMethod createHandlerMethod(Object handler, Method method) {

@Override
public RequestMatchResult match(HttpServletRequest request, String pattern) {
RequestMappingInfo info = RequestMappingInfo.paths(pattern).options(builderConfig).build();
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;
Expand All @@ -161,15 +196,6 @@ public RequestMatchResult match(HttpServletRequest request, String pattern) {
return new RequestMatchResult(patterns.iterator().next(), lookupPath, getPathMatcher());
}

@SuppressWarnings("deprecation")
private static RequestMappingInfo.BuilderConfiguration getBuilderConfig() {
RequestMappingInfo.BuilderConfiguration config = new RequestMappingInfo.BuilderConfiguration();
config.setPathMatcher(null);
config.setSuffixPatternMatch(false);
config.setTrailingSlashMatch(true);
return config;
}

private void registerMappingForOperation(ExposableWebEndpoint endpoint, WebOperation operation) {
WebOperationRequestPredicate predicate = operation.getRequestPredicate();
String path = predicate.getPath();
Expand Down Expand Up @@ -202,7 +228,7 @@ protected ServletWebOperation wrapServletWebOperation(ExposableWebEndpoint endpo
}

private RequestMappingInfo createRequestMappingInfo(WebOperationRequestPredicate predicate, String path) {
return RequestMappingInfo.paths(this.endpointMapping.createSubPath(path))
return RequestMappingInfo.paths(this.endpointMapping.createSubPath(path)).options(this.builderConfig)
.methods(RequestMethod.valueOf(predicate.getHttpMethod().name()))
.consumes(predicate.getConsumes().toArray(new String[0]))
.produces(predicate.getProduces().toArray(new String[0])).build();
Expand All @@ -211,7 +237,7 @@ private RequestMappingInfo createRequestMappingInfo(WebOperationRequestPredicate
private void registerLinksMapping() {
RequestMappingInfo mapping = RequestMappingInfo.paths(this.endpointMapping.createSubPath(""))
.methods(RequestMethod.GET).produces(this.endpointMediaTypes.getProduced().toArray(new String[0]))
.options(builderConfig).build();
.options(this.builderConfig).build();
LinksHandler linksHandler = getLinksHandler();
registerMapping(mapping, linksHandler, ReflectionUtils.findMethod(linksHandler.getClass(), "links",
HttpServletRequest.class, HttpServletResponse.class));
Expand Down Expand Up @@ -335,7 +361,7 @@ private Map<String, Object> getArguments(HttpServletRequest request, Map<String,
}

private Object getRemainingPathSegments(HttpServletRequest request) {
String[] pathTokens = tokenize(request, UrlPathHelper.PATH_ATTRIBUTE, true);
String[] pathTokens = tokenize(request, HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, true);
String[] patternTokens = tokenize(request, HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE, false);
int numberOfRemainingPathSegments = pathTokens.length - patternTokens.length + 1;
Assert.state(numberOfRemainingPathSegments >= 0, "Unable to extract remaining path segments");
Expand Down
22 changes: 22 additions & 0 deletions ...a/org/springframework/boot/actuate/endpoint/web/servlet/WebMvcEndpointHandlerMapping.java
View file
Expand Up @@ -31,6 +31,7 @@
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
Expand Down Expand Up @@ -62,6 +63,27 @@ 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();
Expand Down
54 changes: 49 additions & 5 deletions ...org/springframework/boot/actuate/endpoint/web/servlet/MvcWebEndpointIntegrationTests.java
View file
Expand Up @@ -56,8 +56,11 @@
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.
Expand Down Expand Up @@ -104,24 +107,37 @@ void readOperationsThatReturnAResourceSupportRangeRequests() {
});
}

@Test
void matchWhenPathPatternParserShouldThrowException() {
assertThatIllegalArgumentException().isThrownBy(() -> getMatchResult("/spring/", true));
}

@Test
void matchWhenRequestHasTrailingSlashShouldNotBeNull() {
assertThat(getMatchResult("/spring/")).isNotNull();
assertThat(getMatchResult("/spring/", false)).isNotNull();
}

@Test
void matchWhenRequestHasSuffixShouldBeNull() {
assertThat(getMatchResult("/spring.do")).isNull();
assertThat(getMatchResult("/spring.do", false)).isNull();
}

private RequestMatchResult getMatchResult(String servletPath) {
private RequestMatchResult getMatchResult(String servletPath, boolean isPatternParser) {
MockHttpServletRequest request = new MockHttpServletRequest();
request.setServletPath(servletPath);
AnnotationConfigServletWebServerApplicationContext context = createApplicationContext();
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);
}
Expand Down Expand Up @@ -156,7 +172,35 @@ 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 EndpointLinksResolver(endpointDiscoverer.getEndpoints()), StringUtils.hasText(endpointPath),
new PathPatternParser());
}

}

@Configuration(proxyBeanMethods = false)
@ImportAutoConfiguration({ JacksonAutoConfiguration.class, HttpMessageConvertersAutoConfiguration.class,
ServletWebServerFactoryAutoConfiguration.class, WebMvcAutoConfiguration.class,
DispatcherServletAutoConfiguration.class, ErrorMvcAutoConfiguration.class })
static class PathMatcherWebMvcConfiguration {

@Bean
TomcatServletWebServerFactory tomcat() {
return new TomcatServletWebServerFactory(0);
}

@Bean
WebMvcEndpointHandlerMapping webEndpointHandlerMapping(Environment environment,
WebEndpointDiscoverer endpointDiscoverer, EndpointMediaTypes endpointMediaTypes) {
CorsConfiguration corsConfiguration = new CorsConfiguration();
corsConfiguration.setAllowedOrigins(Arrays.asList("https://example.com"));
corsConfiguration.setAllowedMethods(Arrays.asList("GET", "POST"));
String endpointPath = environment.getProperty("endpointPath");
WebMvcEndpointHandlerMapping handlerMapping = new WebMvcEndpointHandlerMapping(
new EndpointMapping(endpointPath), endpointDiscoverer.getEndpoints(), endpointMediaTypes,
corsConfiguration, new EndpointLinksResolver(endpointDiscoverer.getEndpoints()),
StringUtils.hasText(endpointPath));
return handlerMapping;
}

}
Expand Down
7 changes: 6 additions & 1 deletion ...main/java/org/springframework/boot/autoconfigure/web/servlet/WebMvcAutoConfiguration.java
View file
Expand Up @@ -158,6 +158,11 @@ public class WebMvcAutoConfiguration {
*/
public static final String DEFAULT_SUFFIX = "";

/**
* Instance of {@link PathPatternParser} shared across MVC and actuator configuration.
*/
public static final PathPatternParser pathPatternParser = new PathPatternParser();

private static final String SERVLET_LOCATION = "/";

@Bean
Expand Down Expand Up @@ -246,7 +251,7 @@ public void configureAsyncSupport(AsyncSupportConfigurer configurer) {
public void configurePathMatch(PathMatchConfigurer configurer) {
if (this.mvcProperties.getPathmatch()
.getMatchingStrategy() == WebMvcProperties.MatchingStrategy.PATH_PATTERN_PARSER) {
configurer.setPatternParser(new PathPatternParser());
configurer.setPatternParser(pathPatternParser);
}
configurer.setUseSuffixPatternMatch(this.mvcProperties.getPathmatch().isUseSuffixPattern());
configurer.setUseRegisteredSuffixPatternMatch(
Expand Down

0 comments on commit 393081f

Please sign in to comment.