Deprecate path extension strategies

This commit deprecates PathExtensionContentNegotiationStrategy and
ServletPathExtensionContentNegotiationStrategy and also updates code
that depends on them internally to remove that dependence.

See gh-24179

spring-test/src/main/java/org/springframework/test/web/servlet/setup/StandaloneMockMvcBuilder.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2020 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.
@@ -310,7 +310,11 @@ public class StandaloneMockMvcBuilder extends AbstractMockMvcBuilder<StandaloneM
 	 * Whether to use suffix pattern match (".*") when matching patterns to
 	 * requests. If enabled a method mapped to "/users" also matches to "/users.*".
 	 * <p>The default value is {@code true}.
+	 * @deprecated as of 5.2.4. See class-level note in
+	 * {@link RequestMappingHandlerMapping} on the deprecation of path extension
+	 * config options.
 	 */
+	@Deprecated
 	public StandaloneMockMvcBuilder setUseSuffixPatternMatch(boolean useSuffixPatternMatch) {
 		this.useSuffixPatternMatch = useSuffixPatternMatch;
 		return this;
@@ -442,6 +446,7 @@ public class StandaloneMockMvcBuilder extends AbstractMockMvcBuilder<StandaloneM
 	/** Using the MVC Java configuration as the starting point for the "standalone" setup. */
 	private class StandaloneConfiguration extends WebMvcConfigurationSupport {
 
+		@SuppressWarnings("deprecation")
 		public RequestMappingHandlerMapping getHandlerMapping(
 				FormattingConversionService mvcConversionService,
 				ResourceUrlProvider mvcResourceUrlProvider) {

spring-web/src/main/java/org/springframework/web/accept/PathExtensionContentNegotiationStrategy.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2020 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.
@@ -41,7 +41,11 @@ import org.springframework.web.util.UrlPathHelper;
  *
  * @author Rossen Stoyanchev
  * @since 3.2
+ * @deprecated as of 5.2.4. See class-level note in
+ * {@link ContentNegotiationManagerFactoryBean} on the deprecation of path
+ * extension config options.
  */
+@Deprecated
 public class PathExtensionContentNegotiationStrategy extends AbstractMappingContentNegotiationStrategy {
 
 	private UrlPathHelper urlPathHelper = new UrlPathHelper();

spring-web/src/main/java/org/springframework/web/accept/ServletPathExtensionContentNegotiationStrategy.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2020 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.
@@ -34,7 +34,11 @@ import org.springframework.web.context.request.NativeWebRequest;
  *
  * @author Rossen Stoyanchev
  * @since 3.2
+ * @deprecated as of 5.2.4. See class-level note in
+ * {@link ContentNegotiationManagerFactoryBean} on the deprecation of path
+ * extension config options.
  */
+@Deprecated
 public class ServletPathExtensionContentNegotiationStrategy extends PathExtensionContentNegotiationStrategy {
 
 	private final ServletContext servletContext;

spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/ResourceHandlerRegistry.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2020 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.
@@ -151,6 +151,7 @@ public class ResourceHandlerRegistry {
 	 * of no registrations.
 	 */
 	@Nullable
+	@SuppressWarnings("deprecation")
 	protected AbstractHandlerMapping getHandlerMapping() {
 		if (this.registrations.isEmpty()) {
 			return null;

spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java

@@ -26,6 +26,7 @@ import java.util.List;
 import java.util.Locale;
 import java.util.Set;
 
+import javax.servlet.ServletRequest;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
@@ -43,6 +44,7 @@ import org.springframework.http.HttpOutputMessage;
 import org.springframework.http.HttpRange;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.MediaType;
+import org.springframework.http.MediaTypeFactory;
 import org.springframework.http.converter.GenericHttpMessageConverter;
 import org.springframework.http.converter.HttpMessageConverter;
 import org.springframework.http.converter.HttpMessageNotWritableException;
@@ -54,7 +56,6 @@ import org.springframework.util.CollectionUtils;
 import org.springframework.util.StringUtils;
 import org.springframework.web.HttpMediaTypeNotAcceptableException;
 import org.springframework.web.accept.ContentNegotiationManager;
-import org.springframework.web.accept.PathExtensionContentNegotiationStrategy;
 import org.springframework.web.context.request.NativeWebRequest;
 import org.springframework.web.context.request.ServletWebRequest;
 import org.springframework.web.method.support.HandlerMethodReturnValueHandler;
@@ -102,8 +103,6 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe
 
 	private final ContentNegotiationManager contentNegotiationManager;
 
-	private final PathExtensionContentNegotiationStrategy pathStrategy;
-
 	private final Set<String> safeExtensions = new HashSet<>();
 
 
@@ -133,17 +132,10 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe
 		super(converters, requestResponseBodyAdvice);
 
 		this.contentNegotiationManager = (manager != null ? manager : new ContentNegotiationManager());
-		this.pathStrategy = initPathStrategy(this.contentNegotiationManager);
 		this.safeExtensions.addAll(this.contentNegotiationManager.getAllFileExtensions());
 		this.safeExtensions.addAll(WHITELISTED_EXTENSIONS);
 	}
 
-	private static PathExtensionContentNegotiationStrategy initPathStrategy(ContentNegotiationManager manager) {
-		Class<PathExtensionContentNegotiationStrategy> clazz = PathExtensionContentNegotiationStrategy.class;
-		PathExtensionContentNegotiationStrategy strategy = manager.getStrategy(clazz);
-		return (strategy != null ? strategy : new PathExtensionContentNegotiationStrategy());
-	}
-
 
 	/**
 	 * Creates a new {@link HttpOutputMessage} from the given {@link NativeWebRequest}.
@@ -481,26 +473,21 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe
 				return true;
 			}
 		}
-		return safeMediaTypesForExtension(new ServletWebRequest(request), extension);
+		MediaType mediaType = resolveMediaType(request, extension);
+		return (mediaType != null && (safeMediaType(mediaType)));
 	}
 
-	private boolean safeMediaTypesForExtension(NativeWebRequest request, String extension) {
+	@Nullable
-		List<MediaType> mediaTypes = null;
+	private MediaType resolveMediaType(ServletRequest request, String extension) {
-		try {
+		MediaType result = null;
-			mediaTypes = this.pathStrategy.resolveMediaTypeKey(request, extension);
+		String rawMimeType = request.getServletContext().getMimeType("file." + extension);
+		if (StringUtils.hasText(rawMimeType)) {
+			result = MediaType.parseMediaType(rawMimeType);
 		}
-		catch (HttpMediaTypeNotAcceptableException ex) {
+		if (result == null || MediaType.APPLICATION_OCTET_STREAM.equals(result)) {
-			// Ignore
+			result = MediaTypeFactory.getMediaType("file." + extension).orElse(null);
 		}
-		if (CollectionUtils.isEmpty(mediaTypes)) {
+		return result;
-			return false;
-		}
-		for (MediaType mediaType : mediaTypes) {
-			if (!safeMediaType(mediaType)) {
-				return false;
-			}
-		}
-		return true;
 	}
 
 	private boolean safeMediaType(MediaType mediaType) {

spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2020 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.
@@ -24,6 +24,7 @@ import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 import java.util.stream.Collectors;
 
@@ -43,6 +44,7 @@ import org.springframework.http.HttpHeaders;
 import org.springframework.http.HttpMethod;
 import org.springframework.http.HttpRange;
 import org.springframework.http.MediaType;
+import org.springframework.http.MediaTypeFactory;
 import org.springframework.http.converter.ResourceHttpMessageConverter;
 import org.springframework.http.converter.ResourceRegionHttpMessageConverter;
 import org.springframework.http.server.ServletServerHttpRequest;
@@ -56,8 +58,6 @@ import org.springframework.util.StringUtils;
 import org.springframework.util.StringValueResolver;
 import org.springframework.web.HttpRequestHandler;
 import org.springframework.web.accept.ContentNegotiationManager;
-import org.springframework.web.accept.PathExtensionContentNegotiationStrategy;
-import org.springframework.web.accept.ServletPathExtensionContentNegotiationStrategy;
 import org.springframework.web.context.request.ServletWebRequest;
 import org.springframework.web.cors.CorsConfiguration;
 import org.springframework.web.cors.CorsConfigurationSource;
@@ -129,8 +129,7 @@ public class ResourceHttpRequestHandler extends WebContentGenerator
 	@Nullable
 	private ContentNegotiationManager contentNegotiationManager;
 
-	@Nullable
+	private final Map<String, MediaType> mediaTypes = new HashMap<>(4);
-	private PathExtensionContentNegotiationStrategy contentNegotiationStrategy;
 
 	@Nullable
 	private CorsConfiguration corsConfiguration;
@@ -262,7 +261,11 @@ public class ResourceHttpRequestHandler extends WebContentGenerator
 	 * media types for resources being served. If the manager contains a path
 	 * extension strategy it will be checked for registered file extension.
 	 * @since 4.3
+	 * @deprecated as of 5.2.4 in favor of using {@link #setMediaTypes(Map)}
+	 * with mappings possibly obtained from
+	 * {@link ContentNegotiationManager#getMediaTypeMappings()}.
 	 */
+	@Deprecated
 	public void setContentNegotiationManager(@Nullable ContentNegotiationManager contentNegotiationManager) {
 		this.contentNegotiationManager = contentNegotiationManager;
 	}
@@ -270,12 +273,38 @@ public class ResourceHttpRequestHandler extends WebContentGenerator
 	/**
 	 * Return the configured content negotiation manager.
 	 * @since 4.3
+	 * @deprecated as of 5.2.4.
 	 */
 	@Nullable
+	@Deprecated
 	public ContentNegotiationManager getContentNegotiationManager() {
 		return this.contentNegotiationManager;
 	}
 
+	/**
+	 * Add mappings between file extensions, extracted from the filename of a
+	 * static {@link Resource}, and corresponding media type  to set on the
+	 * response.
+	 * <p>Use of this method is typically not necessary since mappings are
+	 * otherwise determined via
+	 * {@link javax.servlet.ServletContext#getMimeType(String)} or via
+	 * {@link MediaTypeFactory#getMediaType(Resource)}.
+	 * @param mediaTypes media type mappings
+	 * @since 5.2.4
+	 */
+	public void setMediaTypes(Map<String, MediaType> mediaTypes) {
+		mediaTypes.forEach((ext, mediaType) ->
+				this.mediaTypes.put(ext.toLowerCase(Locale.ENGLISH), mediaType));
+	}
+
+	/**
+	 * Return the {@link #setMediaTypes(Map) configured} media types.
+	 * @since 5.2.4
+	 */
+	public Map<String, MediaType> getMediaTypes() {
+		return this.mediaTypes;
+	}
+
 	/**
 	 * Specify the CORS configuration for resources served by this handler.
 	 * <p>By default this is not set in which allows cross-origin requests.
@@ -344,7 +373,17 @@ public class ResourceHttpRequestHandler extends WebContentGenerator
 			this.resourceRegionHttpMessageConverter = new ResourceRegionHttpMessageConverter();
 		}
 
-		this.contentNegotiationStrategy = initContentNegotiationStrategy();
+		ContentNegotiationManager manager = getContentNegotiationManager();
+		if (manager != null) {
+			setMediaTypes(manager.getMediaTypeMappings());
+		}
+
+		@SuppressWarnings("deprecation")
+		org.springframework.web.accept.PathExtensionContentNegotiationStrategy strategy =
+				initContentNegotiationStrategy();
+		if (strategy != null) {
+			setMediaTypes(strategy.getMediaTypes());
+		}
 	}
 
 	private void resolveResourceLocations() {
@@ -412,26 +451,20 @@ public class ResourceHttpRequestHandler extends WebContentGenerator
 	}
 
 	/**
-	 * Initialize the content negotiation strategy depending on the {@code ContentNegotiationManager}
+	 * Initialize the strategy to use to determine the media type for a resource.
-	 * setup and the availability of a {@code ServletContext}.
+	 * @deprecated as of 5.2.4 this method returns {@code null}, and if a
-	 * @see ServletPathExtensionContentNegotiationStrategy
+	 * sub-class returns an actual instance,the instance is used only as a
-	 * @see PathExtensionContentNegotiationStrategy
+	 * source of media type mappings, if it contains any. Please, use
+	 * {@link #setMediaTypes(Map)} instead, or if you need to change behavior,
+	 * you can override {@link #getMediaType(HttpServletRequest, Resource)}.
 	 */
-	protected PathExtensionContentNegotiationStrategy initContentNegotiationStrategy() {
+	@Nullable
-		Map<String, MediaType> mediaTypes = null;
+	@Deprecated
-		if (getContentNegotiationManager() != null) {
+	@SuppressWarnings("deprecation")
-			PathExtensionContentNegotiationStrategy strategy =
+	protected org.springframework.web.accept.PathExtensionContentNegotiationStrategy initContentNegotiationStrategy() {
-					getContentNegotiationManager().getStrategy(PathExtensionContentNegotiationStrategy.class);
+		return null;
-			if (strategy != null) {
-				mediaTypes = new HashMap<>(strategy.getMediaTypes());
-			}
-		}
-		return (getServletContext() != null ?
-				new ServletPathExtensionContentNegotiationStrategy(getServletContext(), mediaTypes) :
-				new PathExtensionContentNegotiationStrategy(mediaTypes));
 	}
 
-
 	/**
 	 * Processes a resource request.
 	 * <p>Checks for the existence of the requested resource in the configured list of locations.
@@ -659,17 +692,40 @@ public class ResourceHttpRequestHandler extends WebContentGenerator
 
 	/**
 	 * Determine the media type for the given request and the resource matched
-	 * to it. This implementation tries to determine the MediaType based on the
+	 * to it. This implementation tries to determine the MediaType using one of
-	 * file extension of the Resource via
+	 * the following lookups based on the resource filename and its path
-	 * {@link ServletPathExtensionContentNegotiationStrategy#getMediaTypeForResource}.
+	 * extension:
+	 * <ol>
+	 * <li>{@link javax.servlet.ServletContext#getMimeType(String)}
+	 * <li>{@link #getMediaTypes()}
+	 * <li>{@link MediaTypeFactory#getMediaType(String)}
+	 * </ol>
 	 * @param request the current request
 	 * @param resource the resource to check
 	 * @return the corresponding media type, or {@code null} if none found
 	 */
 	@Nullable
 	protected MediaType getMediaType(HttpServletRequest request, Resource resource) {
-		return (this.contentNegotiationStrategy != null ?
+		MediaType result = null;
-				this.contentNegotiationStrategy.getMediaTypeForResource(resource) : null);
+		String mimeType = request.getServletContext().getMimeType(resource.getFilename());
+		if (StringUtils.hasText(mimeType)) {
+			result = MediaType.parseMediaType(mimeType);
+		}
+		if (result == null || MediaType.APPLICATION_OCTET_STREAM.equals(result)) {
+			MediaType mediaType = null;
+			String filename = resource.getFilename();
+			String ext = StringUtils.getFilenameExtension(filename);
+			if (ext != null) {
+				mediaType = this.mediaTypes.get(ext.toLowerCase(Locale.ENGLISH));
+			}
+			if (mediaType == null) {
+				mediaType = MediaTypeFactory.getMediaType(filename).orElse(null);
+			}
+			if (mediaType != null) {
+				result = mediaType;
+			}
+		}
+		return result;
 	}
 
 	/**

spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2020 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.
@@ -74,13 +74,15 @@ public class ResourceHttpRequestHandlerTests {
 		paths.add(new ClassPathResource("testalternatepath/", getClass()));
 		paths.add(new ClassPathResource("META-INF/resources/webjars/"));
 
+		TestServletContext servletContext = new TestServletContext();
+
 		this.handler = new ResourceHttpRequestHandler();
 		this.handler.setLocations(paths);
 		this.handler.setCacheSeconds(3600);
-		this.handler.setServletContext(new TestServletContext());
+		this.handler.setServletContext(servletContext);
 		this.handler.afterPropertiesSet();
 
-		this.request = new MockHttpServletRequest("GET", "");
+		this.request = new MockHttpServletRequest(servletContext, "GET", "");
 		this.response = new MockHttpServletResponse();
 	}
 
@@ -283,15 +285,12 @@ public class ResourceHttpRequestHandlerTests {
 
 	@Test  // SPR-14368
 	public void getResourceWithMediaTypeResolvedThroughServletContext() throws Exception {
+
 		MockServletContext servletContext = new MockServletContext() {
 			@Override
 			public String getMimeType(String filePath) {
 				return "foo/bar";
 			}
-			@Override
-			public String getVirtualServerName() {
-				return "";
-			}
 		};
 
 		List<Resource> paths = Collections.singletonList(new ClassPathResource("test/", getClass()));
@@ -300,8 +299,9 @@ public class ResourceHttpRequestHandlerTests {
 		handler.setLocations(paths);
 		handler.afterPropertiesSet();
 
-		this.request.setAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, "foo.css");
+		MockHttpServletRequest request = new MockHttpServletRequest(servletContext, "GET", "");
-		handler.handleRequest(this.request, this.response);
+		request.setAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, "foo.css");
+		handler.handleRequest(request, this.response);
 
 		assertThat(this.response.getContentType()).isEqualTo("foo/bar");
 		assertThat(this.response.getContentAsString()).isEqualTo("h1 { color:red; }");

src/docs/asciidoc/web/webmvc.adoc

@@ -1683,6 +1683,18 @@ the issues that come with file extensions. Alternatively, if you must use file e
 restricting them to a list of explicitly registered extensions through the
 `mediaTypes` property of <<mvc-config-content-negotiation,ContentNegotiationConfigurer>>.
 
+[INFO]
+====
+Starting in 5.2.4, path extension related options for request mapping in
+{api-spring-framework}/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMapping.java[RequestMappingHandlerMapping]
+and for content negotiation in
+{api-spring-framework}/org.springframework.web.accept/ContentNegotiationManagerFactoryBean.java[ContentNegotiationManagerFactoryBean]
+are deprecated. See Spring Framework issue
+https://github.com/spring-projects/spring-framework/issues/24179[#24179] and related
+issues for further plans.
+====
+
+
 
 [[mvc-ann-requestmapping-rfd]]
 ==== Suffix Match and RFD
@@ -5779,13 +5791,11 @@ The following example shows how to customize path matching in Java configuration
 		@Override
 		public void configurePathMatch(PathMatchConfigurer configurer) {
 			configurer
-				.setUseSuffixPatternMatch(true)
 				.setUseTrailingSlashMatch(false)
 				.setUseRegisteredSuffixPatternMatch(true)
 				.setPathMatcher(antPathMatcher())
 				.setUrlPathHelper(urlPathHelper())
-				.addPathPrefix("/api",
+				.addPathPrefix("/api", HandlerTypePredicate.forAnnotation(RestController.class));
-						HandlerTypePredicate.forAnnotation(RestController.class));
 		}
 
 		@Bean
@@ -5813,8 +5823,7 @@ The following example shows how to customize path matching in Java configuration
 				.setUseRegisteredSuffixPatternMatch(true)
 				.setPathMatcher(antPathMatcher())
 				.setUrlPathHelper(urlPathHelper())
-				.addPathPrefix("/api",
+				.addPathPrefix("/api", HandlerTypePredicate.forAnnotation(RestController::class.java))
-						HandlerTypePredicate.forAnnotation(RestController::class.java))
 		}
 
 		@Bean
@@ -5835,7 +5844,6 @@ The following example shows how to achieve the same configuration in XML:
 ----
 	<mvc:annotation-driven>
 		<mvc:path-matching
-			suffix-pattern="true"
 			trailing-slash="false"
 			registered-suffixes-only="true"
 			path-helper="pathHelper"