Merge path extension related deprecation changes

Closes 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/ContentNegotiationManager.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 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.
@@ -20,13 +20,17 @@
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.LinkedHashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
+import java.util.function.Function;
 
 import org.springframework.http.MediaType;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
+import org.springframework.util.CollectionUtils;
 import org.springframework.web.HttpMediaTypeNotAcceptableException;
 import org.springframework.web.context.request.NativeWebRequest;
 
@@ -132,11 +136,7 @@ public List<MediaType> resolveMediaTypes(NativeWebRequest request) throws HttpMe
 
 	@Override
 	public List<String> resolveFileExtensions(MediaType mediaType) {
-		Set<String> result = new LinkedHashSet<>();
-		for (MediaTypeFileExtensionResolver resolver : this.resolvers) {
-			result.addAll(resolver.resolveFileExtensions(mediaType));
-		}
-		return new ArrayList<>(result);
+		return doResolveExtensions(resolver -> resolver.resolveFileExtensions(mediaType));
 	}
 
 	/**
@@ -152,11 +152,44 @@ public List<String> resolveFileExtensions(MediaType mediaType) {
 	 */
 	@Override
 	public List<String> getAllFileExtensions() {
-		Set<String> result = new LinkedHashSet<>();
+		return doResolveExtensions(MediaTypeFileExtensionResolver::getAllFileExtensions);
+	}
+
+	private List<String> doResolveExtensions(Function<MediaTypeFileExtensionResolver, List<String>> extractor) {
+		List<String> result = null;
+		for (MediaTypeFileExtensionResolver resolver : this.resolvers) {
+			List<String> extensions = extractor.apply(resolver);
+			if (CollectionUtils.isEmpty(extensions)) {
+				continue;
+			}
+			result = (result != null ? result : new ArrayList<>(4));
+			for (String extension : extensions) {
+				if (!result.contains(extension)) {
+					result.add(extension);
+				}
+			}
+		}
+		return (result != null ? result : Collections.emptyList());
+	}
+
+	/**
+	 * Return all registered lookup key to media type mappings by iterating
+	 * {@link MediaTypeFileExtensionResolver}s.
+	 * @since 5.2.4
+	 */
+	public Map<String, MediaType> getMediaTypeMappings() {
+		Map<String, MediaType> result = null;
 		for (MediaTypeFileExtensionResolver resolver : this.resolvers) {
-			result.addAll(resolver.getAllFileExtensions());
+			if (resolver instanceof MappingMediaTypeFileExtensionResolver) {
+				Map<String, MediaType> map = ((MappingMediaTypeFileExtensionResolver) resolver).getMediaTypes();
+				if (CollectionUtils.isEmpty(map)) {
+					continue;
+				}
+				result = (result != null ? result : new HashMap<>(4));
+				result.putAll(map);
+			}
 		}
-		return new ArrayList<>(result);
+		return (result != null ? result : Collections.emptyMap());
 	}
 
 }

spring-web/src/main/java/org/springframework/web/accept/ContentNegotiationManagerFactoryBean.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.
@@ -81,15 +81,18 @@
  * </tr>
  * </table>
  *
- * <p>As of 5.0 you can set the exact strategies to use via
+ * <p>Alternatively you can avoid use of the above convenience builder
+ * methods and set the exact strategies to use via
  * {@link #setStrategies(List)}.
  *
- * <p><strong>Note:</strong> if you must use URL-based content type resolution,
- * the use of a query parameter is simpler and preferable to the use of a path
- * extension since the latter can cause issues with URI variables, path
- * parameters, and URI decoding. Consider setting {@link #setFavorPathExtension}
- * to {@literal false} or otherwise set the strategies to use explicitly via
- * {@link #setStrategies(List)}.
+ * <p><strong>Note:</strong> As of 5.2.4,
+ * {@link #setFavorPathExtension(boolean) favorPathExtension} and
+ * {@link #setIgnoreUnknownPathExtensions(boolean) ignoreUnknownPathExtensions}
+ * are deprecated in order to discourage use of path extensions for content
+ * negotiation as well as for request mapping (with similar deprecations in
+ * {@link org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping
+ * RequestMappingHandlerMapping}). For further context, please read issue
+ * <a href="https://github.com/spring-projects/spring-framework/issues/24179">#24719</a>.
  *
  * @author Rossen Stoyanchev
  * @author Brian Clozel
@@ -145,52 +148,61 @@ public void setStrategies(@Nullable List<ContentNegotiationStrategy> strategies)
 	 * <p>By default this is set to {@code true} in which case a request
 	 * for {@code /hotels.pdf} will be interpreted as a request for
 	 * {@code "application/pdf"} regardless of the 'Accept' header.
+	 * @deprecated as of 5.2.4. See class-level note on the deprecation of path
+	 * extension config options.
 	 */
+	@Deprecated
 	public void setFavorPathExtension(boolean favorPathExtension) {
 		this.favorPathExtension = favorPathExtension;
 	}
 
 	/**
-	 * Add a mapping from a key, extracted from a path extension or a query
-	 * parameter, to a MediaType. This is required in order for the parameter
-	 * strategy to work. Any extensions explicitly registered here are also
-	 * whitelisted for the purpose of Reflected File Download attack detection
-	 * (see Spring Framework reference documentation for more details on RFD
-	 * attack protection).
-	 * <p>The path extension strategy will also try to use
+	 * Add a mapping from a key to a MediaType where the key are normalized to
+	 * lowercase and may have been extracted from a path extension, a filename
+	 * extension, or passed as a query parameter.
+	 * <p>The {@link #setFavorParameter(boolean) parameter strategy} requires
+	 * such mappings in order to work while the {@link #setFavorPathExtension(boolean)
+	 * path extension strategy} can fall back on lookups via
 	 * {@link ServletContext#getMimeType} and
-	 * {@link org.springframework.http.MediaTypeFactory} to resolve path extensions.
+	 * {@link org.springframework.http.MediaTypeFactory}.
+	 * <p><strong>Note:</strong> Mappings registered here may be accessed via
+	 * {@link ContentNegotiationManager#getMediaTypeMappings()} and may be used
+	 * not only in the parameter and path extension strategies. For example,
+	 * with the Spring MVC config, e.g. {@code @EnableWebMvc} or
+	 * {@code <mvc:annotation-driven>}, the media type mappings are also plugged
+	 * in to:
+	 * <ul>
+	 * <li>Determine the media type of static resources served with
+	 * {@code ResourceHttpRequestHandler}.
+	 * <li>Determine the media type of views rendered with
+	 * {@code ContentNegotiatingViewResolver}.
+	 * <li>Whitelist extensions for RFD attack detection (check the Spring
+	 * Framework reference docs for details).
+	 * </ul>
 	 * @param mediaTypes media type mappings
 	 * @see #addMediaType(String, MediaType)
 	 * @see #addMediaTypes(Map)
 	 */
 	public void setMediaTypes(Properties mediaTypes) {
 		if (!CollectionUtils.isEmpty(mediaTypes)) {
-			mediaTypes.forEach((key, value) -> {
-				String extension = ((String) key).toLowerCase(Locale.ENGLISH);
-				MediaType mediaType = MediaType.valueOf((String) value);
-				this.mediaTypes.put(extension, mediaType);
-			});
+			mediaTypes.forEach((key, value) ->
+					addMediaType((String) key, MediaType.valueOf((String) value)));
 		}
 	}
 
 	/**
-	 * An alternative to {@link #setMediaTypes} for use in Java code.
-	 * @see #setMediaTypes
-	 * @see #addMediaTypes
+	 * An alternative to {@link #setMediaTypes} for programmatic registrations.
 	 */
-	public void addMediaType(String fileExtension, MediaType mediaType) {
-		this.mediaTypes.put(fileExtension, mediaType);
+	public void addMediaType(String key, MediaType mediaType) {
+		this.mediaTypes.put(key.toLowerCase(Locale.ENGLISH), mediaType);
 	}
 
 	/**
-	 * An alternative to {@link #setMediaTypes} for use in Java code.
-	 * @see #setMediaTypes
-	 * @see #addMediaType
+	 * An alternative to {@link #setMediaTypes} for programmatic registrations.
 	 */
 	public void addMediaTypes(@Nullable Map<String, MediaType> mediaTypes) {
 		if (mediaTypes != null) {
-			this.mediaTypes.putAll(mediaTypes);
+			mediaTypes.forEach(this::addMediaType);
 		}
 	}
 
@@ -199,7 +211,10 @@ public void addMediaTypes(@Nullable Map<String, MediaType> mediaTypes) {
 	 * to any media type. Setting this to {@code false} will result in an
 	 * {@code HttpMediaTypeNotAcceptableException} if there is no match.
 	 * <p>By default this is set to {@code true}.
+	 * @deprecated as of 5.2.4. See class-level note on the deprecation of path
+	 * extension config options.
 	 */
+	@Deprecated
 	public void setIgnoreUnknownPathExtensions(boolean ignore) {
 		this.ignoreUnknownPathExtensions = ignore;
 	}
@@ -303,9 +318,10 @@ public void afterPropertiesSet() {
 	}
 
 	/**
-	 * Actually build the {@link ContentNegotiationManager}.
+	 * Create and initialize a {@link ContentNegotiationManager} instance.
 	 * @since 5.0
 	 */
+	@SuppressWarnings("deprecation")
 	public ContentNegotiationManager build() {
 		List<ContentNegotiationStrategy> strategies = new ArrayList<>();
 
@@ -327,7 +343,6 @@ public ContentNegotiationManager build() {
 				}
 				strategies.add(strategy);
 			}
-
 			if (this.favorParameter) {
 				ParameterContentNegotiationStrategy strategy = new ParameterContentNegotiationStrategy(this.mediaTypes);
 				strategy.setParameterName(this.parameterName);
@@ -339,17 +354,24 @@ public ContentNegotiationManager build() {
 				}
 				strategies.add(strategy);
 			}
-
 			if (!this.ignoreAcceptHeader) {
 				strategies.add(new HeaderContentNegotiationStrategy());
 			}
-
 			if (this.defaultNegotiationStrategy != null) {
 				strategies.add(this.defaultNegotiationStrategy);
 			}
 		}
 
 		this.contentNegotiationManager = new ContentNegotiationManager(strategies);
+
+		// Ensure media type mappings are available via ContentNegotiationManager#getMediaTypeMappings()
+		// independent of path extension or parameter strategies.
+
+		if (!CollectionUtils.isEmpty(this.mediaTypes) && !this.favorPathExtension && !this.favorParameter) {
+			this.contentNegotiationManager.addFileExtensionResolvers(
+					new MappingMediaTypeFileExtensionResolver(this.mediaTypes));
+		}
+
 		return this.contentNegotiationManager;
 	}
 

spring-web/src/main/java/org/springframework/web/accept/MappingMediaTypeFileExtensionResolver.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.
@@ -18,9 +18,11 @@
 
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.CopyOnWriteArrayList;
@@ -53,7 +55,7 @@ public class MappingMediaTypeFileExtensionResolver implements MediaTypeFileExten
 	 */
 	public MappingMediaTypeFileExtensionResolver(@Nullable Map<String, MediaType> mediaTypes) {
 		if (mediaTypes != null) {
-			List<String> allFileExtensions = new ArrayList<>();
+			Set<String> allFileExtensions = new HashSet<>(mediaTypes.size());
 			mediaTypes.forEach((extension, mediaType) -> {
 				String lowerCaseExtension = extension.toLowerCase(Locale.ENGLISH);
 				this.mediaTypes.put(lowerCaseExtension, mediaType);

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;