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

Author: rstoyanchev

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 StandaloneMockMvcBuilder setFlashMapManager(FlashMapManager flashMapManag
 	 * 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 @@ protected Map<String, Object> extendMvcSingletons(@Nullable ServletContext servl
 	/** 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 @@
  *
  * @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 @@
  *
  * @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 ResourceHandlerRegistry setOrder(int order) {
 	 * 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.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.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.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 @@ protected AbstractMessageConverterMethodProcessor(List<HttpMessageConverter<?>>
 		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 @@ private boolean safeExtension(HttpServletRequest request, @Nullable String exten
 				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) {
-		List<MediaType> mediaTypes = null;
-		try {
-			mediaTypes = this.pathStrategy.resolveMediaTypeKey(request, extension);
+	@Nullable
+	private MediaType resolveMediaType(ServletRequest request, String extension) {
+		MediaType result = null;
+		String rawMimeType = request.getServletContext().getMimeType("file." + extension);
+		if (StringUtils.hasText(rawMimeType)) {
+			result = MediaType.parseMediaType(rawMimeType);
 		}
-		catch (HttpMediaTypeNotAcceptableException ex) {
-			// Ignore
-		}
-		if (CollectionUtils.isEmpty(mediaTypes)) {
-			return false;
-		}
-		for (MediaType mediaType : mediaTypes) {
-			if (!safeMediaType(mediaType)) {
-				return false;
-			}
+		if (result == null || MediaType.APPLICATION_OCTET_STREAM.equals(result)) {
+			result = MediaTypeFactory.getMediaType("file." + extension).orElse(null);
 		}
-		return true;
+		return result;
 	}
 
 	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.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.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.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 PathExtensionContentNegotiationStrategy contentNegotiationStrategy;
+	private final Map<String, MediaType> mediaTypes = new HashMap<>(4);
 
 	@Nullable
 	private CorsConfiguration corsConfiguration;
@@ -262,20 +261,50 @@ public ResourceRegionHttpMessageConverter getResourceRegionHttpMessageConverter(
 	 * 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;
 	}
 
 	/**
 	 * 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 void afterPropertiesSet() throws Exception {
 			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 @@ protected void initAllowedLocations() {
 	}
 
 	/**
-	 * Initialize the content negotiation strategy depending on the {@code ContentNegotiationManager}
-	 * setup and the availability of a {@code ServletContext}.
-	 * @see ServletPathExtensionContentNegotiationStrategy
-	 * @see PathExtensionContentNegotiationStrategy
+	 * Initialize the strategy to use to determine the media type for a resource.
+	 * @deprecated as of 5.2.4 this method returns {@code null}, and if a
+	 * sub-class returns an actual instance,the instance is used only as a
+	 * 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() {
-		Map<String, MediaType> mediaTypes = null;
-		if (getContentNegotiationManager() != null) {
-			PathExtensionContentNegotiationStrategy strategy =
-					getContentNegotiationManager().getStrategy(PathExtensionContentNegotiationStrategy.class);
-			if (strategy != null) {
-				mediaTypes = new HashMap<>(strategy.getMediaTypes());
-			}
-		}
-		return (getServletContext() != null ?
-				new ServletPathExtensionContentNegotiationStrategy(getServletContext(), mediaTypes) :
-				new PathExtensionContentNegotiationStrategy(mediaTypes));
+	@Nullable
+	@Deprecated
+	@SuppressWarnings("deprecation")
+	protected org.springframework.web.accept.PathExtensionContentNegotiationStrategy initContentNegotiationStrategy() {
+		return null;
 	}
 
-
 	/**
 	 * Processes a resource request.
 	 * <p>Checks for the existence of the requested resource in the configured list of locations.
@@ -659,17 +692,40 @@ protected boolean isInvalidPath(String path) {
 
 	/**
 	 * Determine the media type for the given request and the resource matched
-	 * to it. This implementation tries to determine the MediaType based on the
-	 * file extension of the Resource via
-	 * {@link ServletPathExtensionContentNegotiationStrategy#getMediaTypeForResource}.
+	 * to it. This implementation tries to determine the MediaType using one of
+	 * the following lookups based on the resource filename and its path
+	 * 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 ?
-				this.contentNegotiationStrategy.getMediaTypeForResource(resource) : null);
+		MediaType result = 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;
 	}
 
 	/**