Skip actuator path extension content negotiation

Allow `PathExtensionContentNegotiationStrategy` to be bypassed by
actuator endpoints. Prior to this commit calling `/loggers/com.aaa.cab`
would return a HTTP 406 response due to `.cab` being a known extension.

Fixes gh-8765

Author: wilkinsona

spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/AbstractEndpointHandlerMapping.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2016 the original author or authors.
+ * Copyright 2012-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.
@@ -26,17 +26,21 @@
 import java.util.Set;
 
 import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
 
 import org.springframework.boot.actuate.endpoint.Endpoint;
+import org.springframework.boot.autoconfigure.web.WebMvcAutoConfiguration;
 import org.springframework.context.ApplicationContext;
 import org.springframework.util.Assert;
 import org.springframework.util.ObjectUtils;
 import org.springframework.util.StringUtils;
+import org.springframework.web.accept.PathExtensionContentNegotiationStrategy;
 import org.springframework.web.cors.CorsConfiguration;
 import org.springframework.web.cors.CorsUtils;
 import org.springframework.web.servlet.HandlerExecutionChain;
 import org.springframework.web.servlet.HandlerInterceptor;
 import org.springframework.web.servlet.HandlerMapping;
+import org.springframework.web.servlet.handler.HandlerInterceptorAdapter;
 import org.springframework.web.servlet.mvc.condition.PatternsRequestCondition;
 import org.springframework.web.servlet.mvc.method.RequestMappingInfo;
 import org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping;
@@ -204,6 +208,11 @@ protected HandlerExecutionChain getCorsHandlerExecutionChain(
 		return addSecurityInterceptor(chain);
 	}
 
+	@Override
+	protected void extendInterceptors(List<Object> interceptors) {
+		interceptors.add(new SkipPathExtensionContentNegotiation());
+	}
+
 	private HandlerExecutionChain addSecurityInterceptor(HandlerExecutionChain chain) {
 		List<HandlerInterceptor> interceptors = new ArrayList<HandlerInterceptor>();
 		if (chain.getInterceptors() != null) {
@@ -279,4 +288,22 @@ protected CorsConfiguration initCorsConfiguration(Object handler, Method method,
 		return this.corsConfiguration;
 	}
 
+	/**
+	 * {@link HandlerInterceptorAdapter} to ensure that
+	 * {@link PathExtensionContentNegotiationStrategy} is skipped for actuator endpoints.
+	 */
+	private static final class SkipPathExtensionContentNegotiation
+			extends HandlerInterceptorAdapter {
+
+		@Override
+		public boolean preHandle(HttpServletRequest request, HttpServletResponse response,
+				Object handler) throws Exception {
+			request.setAttribute(
+					WebMvcAutoConfiguration.SKIP_PATH_EXTENSION_CONTENT_NEGOTIATION_ATTRIBUTE,
+					Boolean.TRUE);
+			return true;
+		}
+
+	}
+
 }

spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/EndpointWebMvcAutoConfigurationTests.java

@@ -193,7 +193,7 @@ public void onDifferentPort() throws Exception {
 				.getBean(ManagementContextResolver.class).getApplicationContext();
 		List<?> interceptors = (List<?>) ReflectionTestUtils.getField(
 				managementContext.getBean(EndpointHandlerMapping.class), "interceptors");
-		assertThat(interceptors).hasSize(1);
+		assertThat(interceptors).hasSize(2);
 	}
 
 	@Test
@@ -211,7 +211,7 @@ public void onDifferentPortWithSpecificContainer() throws Exception {
 				.getBean(ManagementContextResolver.class).getApplicationContext();
 		List<?> interceptors = (List<?>) ReflectionTestUtils.getField(
 				managementContext.getBean(EndpointHandlerMapping.class), "interceptors");
-		assertThat(interceptors).hasSize(1);
+		assertThat(interceptors).hasSize(2);
 		EmbeddedServletContainerFactory parentContainerFactory = this.applicationContext
 				.getBean(EmbeddedServletContainerFactory.class);
 		EmbeddedServletContainerFactory managementContainerFactory = managementContext
@@ -536,7 +536,7 @@ public void managementSpecificSslUsingDifferentPort() throws Exception {
 				.getBean(ManagementContextResolver.class).getApplicationContext();
 		List<?> interceptors = (List<?>) ReflectionTestUtils.getField(
 				managementContext.getBean(EndpointHandlerMapping.class), "interceptors");
-		assertThat(interceptors).hasSize(1);
+		assertThat(interceptors).hasSize(2);
 		ManagementServerProperties managementServerProperties = this.applicationContext
 				.getBean(ManagementServerProperties.class);
 		assertThat(managementServerProperties.getSsl()).isNotNull();
@@ -577,7 +577,7 @@ public void managementServerCanDisableSslWhenUsingADifferentPort() throws Except
 				.getBean(ManagementContextResolver.class).getApplicationContext();
 		List<?> interceptors = (List<?>) ReflectionTestUtils.getField(
 				managementContext.getBean(EndpointHandlerMapping.class), "interceptors");
-		assertThat(interceptors).hasSize(1);
+		assertThat(interceptors).hasSize(2);
 		ManagementServerProperties managementServerProperties = this.applicationContext
 				.getBean(ManagementServerProperties.class);
 		assertThat(managementServerProperties.getSsl()).isNotNull();

spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/AbstractEndpointHandlerMappingTests.java

@@ -60,7 +60,8 @@ public void securityInterceptorIfNullShouldNotBeAdded() throws Exception {
 				Collections.singletonList(endpoint));
 		mapping.setApplicationContext(this.context);
 		mapping.afterPropertiesSet();
-		assertThat(mapping.getHandler(request("POST", "/a")).getInterceptors()).isNull();
+		assertThat(mapping.getHandler(request("POST", "/a")).getInterceptors())
+				.hasSize(1);
 	}
 
 	@Test
@@ -75,8 +76,8 @@ public void securityInterceptorShouldBePresentAfterCorsInterceptorForCorsRequest
 		mapping.afterPropertiesSet();
 		MockHttpServletRequest request = request("POST", "/a");
 		request.addHeader("Origin", "http://example.com");
-		assertThat(mapping.getHandler(request).getInterceptors().length).isEqualTo(2);
-		assertThat(mapping.getHandler(request).getInterceptors()[1])
+		assertThat(mapping.getHandler(request).getInterceptors().length).isEqualTo(3);
+		assertThat(mapping.getHandler(request).getInterceptors()[2])
 				.isEqualTo(securityInterceptor);
 	}
 

spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/LoggersMvcEndpointTests.java

@@ -178,6 +178,16 @@ public void setLoggerWithWrongLogLevel() throws Exception {
 		verifyZeroInteractions(this.loggingSystem);
 	}
 
+	@Test
+	public void logLevelForLoggerWithNameThatCouldBeMistakenForAPathExtension()
+			throws Exception {
+		given(this.loggingSystem.getLoggerConfiguration("com.png"))
+				.willReturn(new LoggerConfiguration("com.png", null, LogLevel.DEBUG));
+		this.mvc.perform(get("/loggers/com.png")).andExpect(status().isOk())
+				.andExpect(content().string(equalTo(
+						"{\"configuredLevel\":null," + "\"effectiveLevel\":\"DEBUG\"}")));
+	}
+
 	@Configuration
 	@Import({ JacksonAutoConfiguration.class,
 			HttpMessageConvertersAutoConfiguration.class,

spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/MetricsMvcEndpointTests.java

@@ -125,6 +125,13 @@ public void specificMetric() throws Exception {
 				.andExpect(content().string(equalTo("{\"foo\":1}")));
 	}
 
+	@Test
+	public void specificMetricWithNameThatCouldBeMistakenForAPathExtension()
+			throws Exception {
+		this.mvc.perform(get("/metrics/bar.png")).andExpect(status().isOk())
+				.andExpect(content().string(equalTo("{\"bar.png\":1}")));
+	}
+
 	@Test
 	public void specificMetricWhenDisabled() throws Exception {
 		this.context.getBean(MetricsEndpoint.class).setEnabled(false);
@@ -138,7 +145,8 @@ public void specificMetricThatDoesNotExist() throws Exception {
 
 	@Test
 	public void regexAll() throws Exception {
-		String expected = "\"foo\":1,\"group1.a\":1,\"group1.b\":1,\"group2.a\":1,\"group2_a\":1";
+		String expected = "\"foo\":1,\"bar.png\":1,\"group1.a\":1,\"group1.b\":1"
+				+ ",\"group2.a\":1,\"group2_a\":1";
 		this.mvc.perform(get("/metrics/.*")).andExpect(status().isOk())
 				.andExpect(content().string(containsString(expected)));
 	}
@@ -178,6 +186,7 @@ public MetricsEndpoint endpoint() {
 				public Collection<Metric<?>> metrics() {
 					ArrayList<Metric<?>> metrics = new ArrayList<Metric<?>>();
 					metrics.add(new Metric<Integer>("foo", 1));
+					metrics.add(new Metric<Integer>("bar.png", 1));
 					metrics.add(new Metric<Integer>("group1.a", 1));
 					metrics.add(new Metric<Integer>("group1.b", 1));
 					metrics.add(new Metric<Integer>("group2.a", 1));

spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/WebMvcAutoConfiguration.java

@@ -20,6 +20,7 @@
 import java.util.Collections;
 import java.util.Date;
 import java.util.List;
+import java.util.ListIterator;
 import java.util.Map;
 import java.util.Map.Entry;
 
@@ -67,8 +68,13 @@
 import org.springframework.validation.DefaultMessageCodesResolver;
 import org.springframework.validation.MessageCodesResolver;
 import org.springframework.validation.Validator;
+import org.springframework.web.HttpMediaTypeNotAcceptableException;
 import org.springframework.web.accept.ContentNegotiationManager;
+import org.springframework.web.accept.ContentNegotiationStrategy;
+import org.springframework.web.accept.PathExtensionContentNegotiationStrategy;
 import org.springframework.web.bind.support.ConfigurableWebBindingInitializer;
+import org.springframework.web.context.request.NativeWebRequest;
+import org.springframework.web.context.request.RequestAttributes;
 import org.springframework.web.context.request.RequestContextListener;
 import org.springframework.web.filter.HiddenHttpMethodFilter;
 import org.springframework.web.filter.HttpPutFormContentFilter;
@@ -125,9 +131,16 @@
 		ValidationAutoConfiguration.class })
 public class WebMvcAutoConfiguration {
 
-	public static String DEFAULT_PREFIX = "";
+	public static final String DEFAULT_PREFIX = "";
 
-	public static String DEFAULT_SUFFIX = "";
+	public static final String DEFAULT_SUFFIX = "";
+
+	/**
+	 * Attribute that can be added to the web request when the
+	 * {@link PathExtensionContentNegotiationStrategy} should be be skipped.
+	 */
+	public static final String SKIP_PATH_EXTENSION_CONTENT_NEGOTIATION_ATTRIBUTE = PathExtensionContentNegotiationStrategy.class
+			.getName() + ".SKIP";
 
 	@Bean
 	@ConditionalOnMissingBean(HiddenHttpMethodFilter.class)
@@ -404,8 +417,7 @@ public Validator mvcValidator() {
 					getClass().getClassLoader())) {
 				return super.mvcValidator();
 			}
-			return WebMvcValidator.get(getApplicationContext(),
-					getValidator());
+			return WebMvcValidator.get(getApplicationContext(), getValidator());
 		}
 
 		@Override
@@ -453,6 +465,22 @@ protected void configureHandlerExceptionResolvers(
 			}
 		}
 
+		@Bean
+		@Override
+		public ContentNegotiationManager mvcContentNegotiationManager() {
+			ContentNegotiationManager manager = super.mvcContentNegotiationManager();
+			List<ContentNegotiationStrategy> strategies = manager.getStrategies();
+			ListIterator<ContentNegotiationStrategy> iterator = strategies.listIterator();
+			while (iterator.hasNext()) {
+				ContentNegotiationStrategy strategy = iterator.next();
+				if (strategy instanceof PathExtensionContentNegotiationStrategy) {
+					iterator.set(new OptionalPathExtensionContentNegotiationStrategy(
+							strategy));
+				}
+			}
+			return manager;
+		}
+
 	}
 
 	@Configuration
@@ -550,4 +578,32 @@ private List<MediaType> getAcceptedMediaTypes(HttpServletRequest request) {
 
 	}
 
+	/**
+	 * Decorator to make {@link PathExtensionContentNegotiationStrategy} optional
+	 * depending on a request attribute.
+	 */
+	static class OptionalPathExtensionContentNegotiationStrategy
+			implements ContentNegotiationStrategy {
+
+		private final ContentNegotiationStrategy delegate;
+
+		OptionalPathExtensionContentNegotiationStrategy(
+				ContentNegotiationStrategy delegate) {
+			this.delegate = delegate;
+		}
+
+		@Override
+		public List<MediaType> resolveMediaTypes(NativeWebRequest webRequest)
+				throws HttpMediaTypeNotAcceptableException {
+			Object skip = webRequest.getAttribute(
+					SKIP_PATH_EXTENSION_CONTENT_NEGOTIATION_ATTRIBUTE,
+					RequestAttributes.SCOPE_REQUEST);
+			if (skip != null && Boolean.parseBoolean(skip.toString())) {
+				return Collections.emptyList();
+			}
+			return this.delegate.resolveMediaTypes(webRequest);
+		}
+
+	}
+
 }