diff --git a/spring-test/src/main/java/org/springframework/test/web/servlet/setup/StandaloneMockMvcBuilder.java b/spring-test/src/main/java/org/springframework/test/web/servlet/setup/StandaloneMockMvcBuilder.java index 46209caddd..554d7e85da 100644 --- a/spring-test/src/main/java/org/springframework/test/web/servlet/setup/StandaloneMockMvcBuilder.java +++ b/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 AbstractMockMvcBuilderThe 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 resolveFileExtensions(MediaType mediaType) { - Set 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 class ContentNegotiationManager implements ContentNegotiationStrategy, Me */ @Override public List getAllFileExtensions() { - Set result = new LinkedHashSet<>(); + return doResolveExtensions(MediaTypeFileExtensionResolver::getAllFileExtensions); + } + + private List doResolveExtensions(Function> extractor) { + List result = null; for (MediaTypeFileExtensionResolver resolver : this.resolvers) { - result.addAll(resolver.getAllFileExtensions()); + List 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 new ArrayList<>(result); + 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 getMediaTypeMappings() { + Map result = null; + for (MediaTypeFileExtensionResolver resolver : this.resolvers) { + if (resolver instanceof MappingMediaTypeFileExtensionResolver) { + Map map = ((MappingMediaTypeFileExtensionResolver) resolver).getMediaTypes(); + if (CollectionUtils.isEmpty(map)) { + continue; + } + result = (result != null ? result : new HashMap<>(4)); + result.putAll(map); + } + } + return (result != null ? result : Collections.emptyMap()); } } diff --git a/spring-web/src/main/java/org/springframework/web/accept/ContentNegotiationManagerFactoryBean.java b/spring-web/src/main/java/org/springframework/web/accept/ContentNegotiationManagerFactoryBean.java index f49cff6395..42f5159141 100644 --- a/spring-web/src/main/java/org/springframework/web/accept/ContentNegotiationManagerFactoryBean.java +++ b/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 @@ import org.springframework.web.context.ServletContextAware; * * * - *

As of 5.0 you can set the exact strategies to use via + *

Alternatively you can avoid use of the above convenience builder + * methods and set the exact strategies to use via * {@link #setStrategies(List)}. * - *

Note: 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)}. + *

Note: 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 + * #24719. * * @author Rossen Stoyanchev * @author Brian Clozel @@ -145,52 +148,61 @@ public class ContentNegotiationManagerFactoryBean *

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). - *

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. + *

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}. + *

Note: 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 }, the media type mappings are also plugged + * in to: + *

    + *
  • Determine the media type of static resources served with + * {@code ResourceHttpRequestHandler}. + *
  • Determine the media type of views rendered with + * {@code ContentNegotiatingViewResolver}. + *
  • Whitelist extensions for RFD attack detection (check the Spring + * Framework reference docs for details). + *
* @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 mediaTypes) { if (mediaTypes != null) { - this.mediaTypes.putAll(mediaTypes); + mediaTypes.forEach(this::addMediaType); } } @@ -199,7 +211,10 @@ public class ContentNegotiationManagerFactoryBean * to any media type. Setting this to {@code false} will result in an * {@code HttpMediaTypeNotAcceptableException} if there is no match. *

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 class ContentNegotiationManagerFactoryBean } /** - * Actually build the {@link ContentNegotiationManager}. + * Create and initialize a {@link ContentNegotiationManager} instance. * @since 5.0 */ + @SuppressWarnings("deprecation") public ContentNegotiationManager build() { List strategies = new ArrayList<>(); @@ -327,7 +343,6 @@ public class ContentNegotiationManagerFactoryBean } strategies.add(strategy); } - if (this.favorParameter) { ParameterContentNegotiationStrategy strategy = new ParameterContentNegotiationStrategy(this.mediaTypes); strategy.setParameterName(this.parameterName); @@ -339,17 +354,24 @@ public class ContentNegotiationManagerFactoryBean } 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; } diff --git a/spring-web/src/main/java/org/springframework/web/accept/MappingMediaTypeFileExtensionResolver.java b/spring-web/src/main/java/org/springframework/web/accept/MappingMediaTypeFileExtensionResolver.java index 25147843b7..a0ee49a569 100644 --- a/spring-web/src/main/java/org/springframework/web/accept/MappingMediaTypeFileExtensionResolver.java +++ b/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 @@ package org.springframework.web.accept; 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 mediaTypes) { if (mediaTypes != null) { - List allFileExtensions = new ArrayList<>(); + Set allFileExtensions = new HashSet<>(mediaTypes.size()); mediaTypes.forEach((extension, mediaType) -> { String lowerCaseExtension = extension.toLowerCase(Locale.ENGLISH); this.mediaTypes.put(lowerCaseExtension, mediaType); diff --git a/spring-web/src/main/java/org/springframework/web/accept/PathExtensionContentNegotiationStrategy.java b/spring-web/src/main/java/org/springframework/web/accept/PathExtensionContentNegotiationStrategy.java index 2bee9a7b53..3fc337b377 100644 --- a/spring-web/src/main/java/org/springframework/web/accept/PathExtensionContentNegotiationStrategy.java +++ b/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(); diff --git a/spring-web/src/main/java/org/springframework/web/accept/ServletPathExtensionContentNegotiationStrategy.java b/spring-web/src/main/java/org/springframework/web/accept/ServletPathExtensionContentNegotiationStrategy.java index f47af3ca2f..f2a5495896 100644 --- a/spring-web/src/main/java/org/springframework/web/accept/ServletPathExtensionContentNegotiationStrategy.java +++ b/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; diff --git a/spring-web/src/test/java/org/springframework/web/accept/ContentNegotiationManagerFactoryBeanTests.java b/spring-web/src/test/java/org/springframework/web/accept/ContentNegotiationManagerFactoryBeanTests.java index 1b5ac13d79..2c0169c997 100644 --- a/spring-web/src/test/java/org/springframework/web/accept/ContentNegotiationManagerFactoryBeanTests.java +++ b/spring-web/src/test/java/org/springframework/web/accept/ContentNegotiationManagerFactoryBeanTests.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. @@ -21,6 +21,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Properties; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -70,21 +71,29 @@ public class ContentNegotiationManagerFactoryBeanTests { this.servletRequest.setRequestURI("/flower.gif"); - assertThat(manager.resolveMediaTypes(this.webRequest)).as("Should be able to resolve file extensions by default").isEqualTo(Collections.singletonList(MediaType.IMAGE_GIF)); + assertThat(manager.resolveMediaTypes(this.webRequest)) + .as("Should be able to resolve file extensions by default") + .isEqualTo(Collections.singletonList(MediaType.IMAGE_GIF)); this.servletRequest.setRequestURI("/flower.foobarbaz"); - assertThat(manager.resolveMediaTypes(this.webRequest)).as("Should ignore unknown extensions by default").isEqualTo(ContentNegotiationStrategy.MEDIA_TYPE_ALL_LIST); + assertThat(manager.resolveMediaTypes(this.webRequest)) + .as("Should ignore unknown extensions by default") + .isEqualTo(ContentNegotiationStrategy.MEDIA_TYPE_ALL_LIST); this.servletRequest.setRequestURI("/flower"); this.servletRequest.setParameter("format", "gif"); - assertThat(manager.resolveMediaTypes(this.webRequest)).as("Should not resolve request parameters by default").isEqualTo(ContentNegotiationStrategy.MEDIA_TYPE_ALL_LIST); + assertThat(manager.resolveMediaTypes(this.webRequest)) + .as("Should not resolve request parameters by default") + .isEqualTo(ContentNegotiationStrategy.MEDIA_TYPE_ALL_LIST); this.servletRequest.setRequestURI("/flower"); this.servletRequest.addHeader("Accept", MediaType.IMAGE_GIF_VALUE); - assertThat(manager.resolveMediaTypes(this.webRequest)).as("Should resolve Accept header by default").isEqualTo(Collections.singletonList(MediaType.IMAGE_GIF)); + assertThat(manager.resolveMediaTypes(this.webRequest)) + .as("Should resolve Accept header by default") + .isEqualTo(Collections.singletonList(MediaType.IMAGE_GIF)); } @Test @@ -101,29 +110,33 @@ public class ContentNegotiationManagerFactoryBeanTests { this.servletRequest.setRequestURI("/flower"); this.servletRequest.addParameter("format", "bar"); - assertThat(manager.resolveMediaTypes(this.webRequest)).isEqualTo(Collections.singletonList(new MediaType("application", "bar"))); + assertThat(manager.resolveMediaTypes(this.webRequest)) + .isEqualTo(Collections.singletonList(new MediaType("application", "bar"))); } @Test public void favorPath() throws Exception { this.factoryBean.setFavorPathExtension(true); - this.factoryBean.addMediaTypes(Collections.singletonMap("bar", new MediaType("application", "bar"))); + this.factoryBean.addMediaType("bar", new MediaType("application", "bar")); this.factoryBean.afterPropertiesSet(); ContentNegotiationManager manager = this.factoryBean.getObject(); this.servletRequest.setRequestURI("/flower.foo"); - assertThat(manager.resolveMediaTypes(this.webRequest)).isEqualTo(Collections.singletonList(new MediaType("application", "foo"))); + assertThat(manager.resolveMediaTypes(this.webRequest)) + .isEqualTo(Collections.singletonList(new MediaType("application", "foo"))); this.servletRequest.setRequestURI("/flower.bar"); - assertThat(manager.resolveMediaTypes(this.webRequest)).isEqualTo(Collections.singletonList(new MediaType("application", "bar"))); + assertThat(manager.resolveMediaTypes(this.webRequest)) + .isEqualTo(Collections.singletonList(new MediaType("application", "bar"))); this.servletRequest.setRequestURI("/flower.gif"); - assertThat(manager.resolveMediaTypes(this.webRequest)).isEqualTo(Collections.singletonList(MediaType.IMAGE_GIF)); + assertThat(manager.resolveMediaTypes(this.webRequest)) + .isEqualTo(Collections.singletonList(MediaType.IMAGE_GIF)); } @Test // SPR-10170 - public void favorPathWithIgnoreUnknownPathExtensionTurnedOff() throws Exception { + public void favorPathWithIgnoreUnknownPathExtensionTurnedOff() { this.factoryBean.setFavorPathExtension(true); this.factoryBean.setIgnoreUnknownPathExtensions(false); this.factoryBean.afterPropertiesSet(); @@ -139,10 +152,7 @@ public class ContentNegotiationManagerFactoryBeanTests { @Test public void favorParameter() throws Exception { this.factoryBean.setFavorParameter(true); - - Map mediaTypes = new HashMap<>(); - mediaTypes.put("json", MediaType.APPLICATION_JSON); - this.factoryBean.addMediaTypes(mediaTypes); + this.factoryBean.addMediaType("json", MediaType.APPLICATION_JSON); this.factoryBean.afterPropertiesSet(); ContentNegotiationManager manager = this.factoryBean.getObject(); @@ -150,11 +160,12 @@ public class ContentNegotiationManagerFactoryBeanTests { this.servletRequest.setRequestURI("/flower"); this.servletRequest.addParameter("format", "json"); - assertThat(manager.resolveMediaTypes(this.webRequest)).isEqualTo(Collections.singletonList(MediaType.APPLICATION_JSON)); + assertThat(manager.resolveMediaTypes(this.webRequest)) + .isEqualTo(Collections.singletonList(MediaType.APPLICATION_JSON)); } @Test // SPR-10170 - public void favorParameterWithUnknownMediaType() throws HttpMediaTypeNotAcceptableException { + public void favorParameterWithUnknownMediaType() { this.factoryBean.setFavorParameter(true); this.factoryBean.afterPropertiesSet(); ContentNegotiationManager manager = this.factoryBean.getObject(); @@ -162,8 +173,52 @@ public class ContentNegotiationManagerFactoryBeanTests { this.servletRequest.setRequestURI("/flower"); this.servletRequest.setParameter("format", "invalid"); - assertThatExceptionOfType(HttpMediaTypeNotAcceptableException.class).isThrownBy(() -> - manager.resolveMediaTypes(this.webRequest)); + assertThatExceptionOfType(HttpMediaTypeNotAcceptableException.class) + .isThrownBy(() -> manager.resolveMediaTypes(this.webRequest)); + } + + @Test + public void mediaTypeMappingsWithoutPathAndParameterStrategies() { + + this.factoryBean.setFavorPathExtension(false); + this.factoryBean.setFavorParameter(false); + + Properties properties = new Properties(); + properties.put("JSon", "application/json"); + + this.factoryBean.setMediaTypes(properties); + this.factoryBean.addMediaType("pdF", MediaType.APPLICATION_PDF); + this.factoryBean.addMediaTypes(Collections.singletonMap("xML", MediaType.APPLICATION_XML)); + + ContentNegotiationManager manager = this.factoryBean.build(); + assertThat(manager.getMediaTypeMappings()) + .hasSize(3) + .containsEntry("json", MediaType.APPLICATION_JSON) + .containsEntry("pdf", MediaType.APPLICATION_PDF) + .containsEntry("xml", MediaType.APPLICATION_XML); + } + + @Test + public void fileExtensions() { + + this.factoryBean.setFavorPathExtension(false); + this.factoryBean.setFavorParameter(false); + + Properties properties = new Properties(); + properties.put("json", "application/json"); + properties.put("pdf", "application/pdf"); + properties.put("xml", "application/xml"); + this.factoryBean.setMediaTypes(properties); + + this.factoryBean.addMediaType("jsON", MediaType.APPLICATION_JSON); + this.factoryBean.addMediaType("pdF", MediaType.APPLICATION_PDF); + + this.factoryBean.addMediaTypes(Collections.singletonMap("JSon", MediaType.APPLICATION_JSON)); + this.factoryBean.addMediaTypes(Collections.singletonMap("xML", MediaType.APPLICATION_XML)); + + ContentNegotiationManager manager = this.factoryBean.build(); + assertThat(manager.getAllFileExtensions()).containsExactlyInAnyOrder("json", "xml", "pdf"); + } @Test @@ -175,7 +230,8 @@ public class ContentNegotiationManagerFactoryBeanTests { this.servletRequest.setRequestURI("/flower"); this.servletRequest.addHeader("Accept", MediaType.IMAGE_GIF_VALUE); - assertThat(manager.resolveMediaTypes(this.webRequest)).isEqualTo(ContentNegotiationStrategy.MEDIA_TYPE_ALL_LIST); + assertThat(manager.resolveMediaTypes(this.webRequest)) + .isEqualTo(ContentNegotiationStrategy.MEDIA_TYPE_ALL_LIST); } @Test @@ -210,10 +266,12 @@ public class ContentNegotiationManagerFactoryBeanTests { this.factoryBean.afterPropertiesSet(); ContentNegotiationManager manager = this.factoryBean.getObject(); - assertThat(manager.resolveMediaTypes(this.webRequest)).isEqualTo(Collections.singletonList(MediaType.APPLICATION_JSON)); + assertThat(manager.resolveMediaTypes(this.webRequest)) + .isEqualTo(Collections.singletonList(MediaType.APPLICATION_JSON)); this.servletRequest.addHeader("Accept", MediaType.ALL_VALUE); - assertThat(manager.resolveMediaTypes(this.webRequest)).isEqualTo(Collections.singletonList(MediaType.APPLICATION_JSON)); + assertThat(manager.resolveMediaTypes(this.webRequest)) + .isEqualTo(Collections.singletonList(MediaType.APPLICATION_JSON)); } diff --git a/spring-web/src/test/java/org/springframework/web/accept/MappingMediaTypeFileExtensionResolverTests.java b/spring-web/src/test/java/org/springframework/web/accept/MappingMediaTypeFileExtensionResolverTests.java index 873f521a9e..7f1907dba5 100644 --- a/spring-web/src/test/java/org/springframework/web/accept/MappingMediaTypeFileExtensionResolverTests.java +++ b/spring-web/src/test/java/org/springframework/web/accept/MappingMediaTypeFileExtensionResolverTests.java @@ -17,6 +17,7 @@ package org.springframework.web.accept; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; @@ -34,12 +35,14 @@ import static org.assertj.core.api.Assertions.assertThat; */ public class MappingMediaTypeFileExtensionResolverTests { - private final Map mapping = Collections.singletonMap("json", MediaType.APPLICATION_JSON); - private final MappingMediaTypeFileExtensionResolver resolver = new MappingMediaTypeFileExtensionResolver(this.mapping); + private static final Map DEFAULT_MAPPINGS = + Collections.singletonMap("json", MediaType.APPLICATION_JSON); + @Test public void resolveExtensions() { - List extensions = this.resolver.resolveFileExtensions(MediaType.APPLICATION_JSON); + List extensions = new MappingMediaTypeFileExtensionResolver(DEFAULT_MAPPINGS) + .resolveFileExtensions(MediaType.APPLICATION_JSON); assertThat(extensions).hasSize(1); assertThat(extensions.get(0)).isEqualTo("json"); @@ -47,20 +50,24 @@ public class MappingMediaTypeFileExtensionResolverTests { @Test public void resolveExtensionsNoMatch() { - List extensions = this.resolver.resolveFileExtensions(MediaType.TEXT_HTML); - - assertThat(extensions).isEmpty(); + assertThat(new MappingMediaTypeFileExtensionResolver(DEFAULT_MAPPINGS) + .resolveFileExtensions(MediaType.TEXT_HTML)).isEmpty(); } - /** - * Unit test for SPR-13747 - ensures that reverse lookup of media type from media - * type key is case-insensitive. - */ - @Test + @Test // SPR-13747 public void lookupMediaTypeCaseInsensitive() { - MediaType mediaType = this.resolver.lookupMediaType("JSON"); - - assertThat(mediaType).isEqualTo(MediaType.APPLICATION_JSON); + assertThat(new MappingMediaTypeFileExtensionResolver(DEFAULT_MAPPINGS).lookupMediaType("JSON")) + .isEqualTo(MediaType.APPLICATION_JSON); } + @Test + public void allFileExtensions() { + Map mappings = new HashMap<>(); + mappings.put("json", MediaType.APPLICATION_JSON); + mappings.put("JsOn", MediaType.APPLICATION_JSON); + mappings.put("jSoN", MediaType.APPLICATION_JSON); + + MappingMediaTypeFileExtensionResolver resolver = new MappingMediaTypeFileExtensionResolver(mappings); + assertThat(resolver.getAllFileExtensions()).containsExactly("json"); + } } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/ContentNegotiationConfigurer.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/ContentNegotiationConfigurer.java index 6b0c2522ea..b1ce073255 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/ContentNegotiationConfigurer.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/ContentNegotiationConfigurer.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. @@ -32,7 +32,6 @@ import org.springframework.web.accept.ContentNegotiationStrategy; import org.springframework.web.accept.FixedContentNegotiationStrategy; import org.springframework.web.accept.HeaderContentNegotiationStrategy; import org.springframework.web.accept.ParameterContentNegotiationStrategy; -import org.springframework.web.accept.PathExtensionContentNegotiationStrategy; /** * Creates a {@code ContentNegotiationManager} and configures it with @@ -52,7 +51,8 @@ import org.springframework.web.accept.PathExtensionContentNegotiationStrategy; * * {@link #favorPathExtension} * true - * {@link PathExtensionContentNegotiationStrategy} + * {@link org.springframework.web.accept.PathExtensionContentNegotiationStrategy + * PathExtensionContentNegotiationStrategy} * Enabled * * @@ -129,7 +129,11 @@ public class ContentNegotiationConfigurer { *

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 in + * {@link ContentNegotiationManagerFactoryBean} on the deprecation of path + * extension config options. */ + @Deprecated public ContentNegotiationConfigurer favorPathExtension(boolean favorPathExtension) { this.factory.setFavorPathExtension(favorPathExtension); return this; @@ -183,7 +187,11 @@ public class ContentNegotiationConfigurer { * to any media type. Setting this to {@code false} will result in an * {@code HttpMediaTypeNotAcceptableException} if there is no match. *

By default this is set to {@code true}. + * @deprecated as of 5.2.4. See class-level note in + * {@link ContentNegotiationManagerFactoryBean} on the deprecation of path + * extension config options. */ + @Deprecated public ContentNegotiationConfigurer ignoreUnknownPathExtensions(boolean ignore) { this.factory.setIgnoreUnknownPathExtensions(ignore); return this; diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/PathMatchConfigurer.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/PathMatchConfigurer.java index fc20853ebe..50d6716eb9 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/PathMatchConfigurer.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/PathMatchConfigurer.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. @@ -22,6 +22,7 @@ import java.util.function.Predicate; import org.springframework.lang.Nullable; import org.springframework.util.PathMatcher; +import org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping; import org.springframework.web.util.UrlPathHelper; /** @@ -37,7 +38,7 @@ import org.springframework.web.util.UrlPathHelper; * * @author Brian Clozel * @since 4.0.3 - * @see org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping + * @see RequestMappingHandlerMapping * @see org.springframework.web.servlet.handler.SimpleUrlHandlerMapping */ public class PathMatchConfigurer { @@ -46,10 +47,10 @@ public class PathMatchConfigurer { private Boolean suffixPatternMatch; @Nullable - private Boolean trailingSlashMatch; + private Boolean registeredSuffixPatternMatch; @Nullable - private Boolean registeredSuffixPatternMatch; + private Boolean trailingSlashMatch; @Nullable private UrlPathHelper urlPathHelper; @@ -66,22 +67,16 @@ public class PathMatchConfigurer { * requests. If enabled a method mapped to "/users" also matches to "/users.*". *

By default this is set to {@code true}. * @see #registeredSuffixPatternMatch + * @deprecated as of 5.2.4. See class-level note in + * {@link RequestMappingHandlerMapping} on the deprecation of path extension + * config options. */ + @Deprecated public PathMatchConfigurer setUseSuffixPatternMatch(Boolean suffixPatternMatch) { this.suffixPatternMatch = suffixPatternMatch; return this; } - /** - * Whether to match to URLs irrespective of the presence of a trailing slash. - * If enabled a method mapped to "/users" also matches to "/users/". - *

The default value is {@code true}. - */ - public PathMatchConfigurer setUseTrailingSlashMatch(Boolean trailingSlashMatch) { - this.trailingSlashMatch = trailingSlashMatch; - return this; - } - /** * Whether suffix pattern matching should work only against path extensions * explicitly registered when you @@ -90,12 +85,26 @@ public class PathMatchConfigurer { * avoid issues such as when a "." appears in the path for other reasons. *

By default this is set to "false". * @see WebMvcConfigurer#configureContentNegotiation + * @deprecated as of 5.2.4. See class-level note in + * {@link RequestMappingHandlerMapping} on the deprecation of path extension + * config options. */ + @Deprecated public PathMatchConfigurer setUseRegisteredSuffixPatternMatch(Boolean registeredSuffixPatternMatch) { this.registeredSuffixPatternMatch = registeredSuffixPatternMatch; return this; } + /** + * Whether to match to URLs irrespective of the presence of a trailing slash. + * If enabled a method mapped to "/users" also matches to "/users/". + *

The default value is {@code true}. + */ + public PathMatchConfigurer setUseTrailingSlashMatch(Boolean trailingSlashMatch) { + this.trailingSlashMatch = trailingSlashMatch; + return this; + } + /** * Set the UrlPathHelper to use for resolution of lookup paths. *

Use this to override the default UrlPathHelper with a custom subclass, @@ -137,21 +146,35 @@ public class PathMatchConfigurer { } + /** + * Whether to use registered suffixes for pattern matching. + * @deprecated as of 5.2.4. See class-level note in + * {@link RequestMappingHandlerMapping} on the deprecation of path extension + * config options. + */ @Nullable + @Deprecated public Boolean isUseSuffixPatternMatch() { return this.suffixPatternMatch; } + /** + * Whether to use registered suffixes for pattern matching. + * @deprecated as of 5.2.4. See class-level note in + * {@link RequestMappingHandlerMapping} on the deprecation of path extension + * config options. + */ + @Nullable + @Deprecated + public Boolean isUseRegisteredSuffixPatternMatch() { + return this.registeredSuffixPatternMatch; + } + @Nullable public Boolean isUseTrailingSlashMatch() { return this.trailingSlashMatch; } - @Nullable - public Boolean isUseRegisteredSuffixPatternMatch() { - return this.registeredSuffixPatternMatch; - } - @Nullable public UrlPathHelper getUrlPathHelper() { return this.urlPathHelper; diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/ResourceHandlerRegistry.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/ResourceHandlerRegistry.java index 772d8c1489..55a92e5a20 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/ResourceHandlerRegistry.java +++ b/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; diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/WebMvcConfigurationSupport.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/WebMvcConfigurationSupport.java index d8f93bf4e5..1000fe12b7 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/WebMvcConfigurationSupport.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/WebMvcConfigurationSupport.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. @@ -275,6 +275,7 @@ public class WebMvcConfigurationSupport implements ApplicationContextAware, Serv * requests to annotated controllers. */ @Bean + @SuppressWarnings("deprecation") public RequestMappingHandlerMapping requestMappingHandlerMapping( @Qualifier("mvcContentNegotiationManager") ContentNegotiationManager contentNegotiationManager, @Qualifier("mvcConversionService") FormattingConversionService conversionService, diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/PatternsRequestCondition.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/PatternsRequestCondition.java index 1c1fd482c5..f5df906880 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/PatternsRequestCondition.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/PatternsRequestCondition.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. @@ -33,6 +33,7 @@ import org.springframework.util.AntPathMatcher; import org.springframework.util.PathMatcher; import org.springframework.util.StringUtils; import org.springframework.web.servlet.HandlerMapping; +import org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping; import org.springframework.web.util.UrlPathHelper; /** @@ -58,23 +59,42 @@ public final class PatternsRequestCondition extends AbstractRequestCondition fileExtensions) { diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfo.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfo.java index fe79fd341e..7d72839996 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfo.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfo.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. @@ -35,6 +35,7 @@ import org.springframework.web.servlet.mvc.condition.ProducesRequestCondition; import org.springframework.web.servlet.mvc.condition.RequestCondition; import org.springframework.web.servlet.mvc.condition.RequestConditionHolder; import org.springframework.web.servlet.mvc.condition.RequestMethodsRequestCondition; +import org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping; import org.springframework.web.util.UrlPathHelper; /** @@ -505,6 +506,7 @@ public final class RequestMappingInfo implements RequestConditionBy default this is set to 'true'. * @see #setRegisteredSuffixPatternMatch(boolean) + * @deprecated as of 5.2.4. See class-level note in + * {@link RequestMappingHandlerMapping} on the deprecation of path + * extension config options. */ + @Deprecated public void setSuffixPatternMatch(boolean suffixPatternMatch) { this.suffixPatternMatch = suffixPatternMatch; } /** * Return whether to apply suffix pattern matching in PatternsRequestCondition. + * @deprecated as of 5.2.4. See class-level note in + * {@link RequestMappingHandlerMapping} on the deprecation of path + * extension config options. */ + @Deprecated public boolean useSuffixPatternMatch() { return this.suffixPatternMatch; } @@ -618,7 +628,12 @@ public final class RequestMappingInfo implements RequestCondition getFileExtensions() { if (useRegisteredSuffixPatternMatch() && this.contentNegotiationManager != null) { return this.contentNegotiationManager.getAllFileExtensions(); diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java index 61eaeac00d..4e83bab4f8 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java +++ b/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 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 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) { - List 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 (result == null || MediaType.APPLICATION_OCTET_STREAM.equals(result)) { + result = MediaTypeFactory.getMediaType("file." + extension).orElse(null); } - if (CollectionUtils.isEmpty(mediaTypes)) { - return false; - } - for (MediaType mediaType : mediaTypes) { - if (!safeMediaType(mediaType)) { - return false; - } - } - return true; + return result; } private boolean safeMediaType(MediaType mediaType) { diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMapping.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMapping.java index 3dadc61a37..5b6d42ed49 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMapping.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMapping.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. @@ -59,6 +59,18 @@ import org.springframework.web.servlet.mvc.method.RequestMappingInfoHandlerMappi * {@link RequestMapping @RequestMapping} annotations in * {@link Controller @Controller} classes. * + *

Note:

In 5.2.4, + * {@link #setUseSuffixPatternMatch(boolean) useSuffixPatternMatch} and + * {@link #setUseRegisteredSuffixPatternMatch(boolean) useRegisteredSuffixPatternMatch} + * are deprecated in order to discourage use of path extensions for request + * mapping and for content negotiation (with similar deprecations in + * {@link ContentNegotiationManager}). For further context, please read issue + * #24719. + * + *

In 5.3, {@link #setUseRegisteredSuffixPatternMatch(boolean) useRegisteredSuffixPatternMatch} + * switches to being on by default so that path matching becomes constrained + * to registered suffixes only. + * * @author Arjen Poutsma * @author Rossen Stoyanchev * @author Sam Brannen @@ -89,7 +101,10 @@ public class RequestMappingHandlerMapping extends RequestMappingInfoHandlerMappi *

The default value is {@code true}. *

Also see {@link #setUseRegisteredSuffixPatternMatch(boolean)} for * more fine-grained control over specific suffixes to allow. + * @deprecated as of 5.2.4. See class level comment about deprecation of + * path extension config options. */ + @Deprecated public void setUseSuffixPatternMatch(boolean useSuffixPatternMatch) { this.useSuffixPatternMatch = useSuffixPatternMatch; } @@ -100,7 +115,11 @@ public class RequestMappingHandlerMapping extends RequestMappingInfoHandlerMappi * is generally recommended to reduce ambiguity and to avoid issues such as * when a "." appears in the path for other reasons. *

By default this is set to "false". + * @deprecated as of 5.2.4. See class level comment about deprecation of + * path extension config options note also that in 5.3 the default for this + * property will switch from {@code false} to {@code true}. */ + @Deprecated public void setUseRegisteredSuffixPatternMatch(boolean useRegisteredSuffixPatternMatch) { this.useRegisteredSuffixPatternMatch = useRegisteredSuffixPatternMatch; this.useSuffixPatternMatch = (useRegisteredSuffixPatternMatch || this.useSuffixPatternMatch); @@ -159,13 +178,14 @@ public class RequestMappingHandlerMapping extends RequestMappingInfoHandlerMappi } @Override + @SuppressWarnings("deprecation") public void afterPropertiesSet() { this.config = new RequestMappingInfo.BuilderConfiguration(); this.config.setUrlPathHelper(getUrlPathHelper()); this.config.setPathMatcher(getPathMatcher()); - this.config.setSuffixPatternMatch(this.useSuffixPatternMatch); - this.config.setTrailingSlashMatch(this.useTrailingSlashMatch); - this.config.setRegisteredSuffixPatternMatch(this.useRegisteredSuffixPatternMatch); + this.config.setSuffixPatternMatch(useSuffixPatternMatch()); + this.config.setTrailingSlashMatch(useTrailingSlashMatch()); + this.config.setRegisteredSuffixPatternMatch(useRegisteredSuffixPatternMatch()); this.config.setContentNegotiationManager(getContentNegotiationManager()); super.afterPropertiesSet(); @@ -173,15 +193,21 @@ public class RequestMappingHandlerMapping extends RequestMappingInfoHandlerMappi /** - * Whether to use suffix pattern matching. + * Whether to use registered suffixes for pattern matching. + * @deprecated as of 5.2.4. See class-level note on the deprecation of path + * extension config options. */ + @Deprecated public boolean useSuffixPatternMatch() { return this.useSuffixPatternMatch; } /** * Whether to use registered suffixes for pattern matching. + * @deprecated as of 5.2.4. See class-level note on the deprecation of path + * extension config options. */ + @Deprecated public boolean useRegisteredSuffixPatternMatch() { return this.useRegisteredSuffixPatternMatch; } @@ -195,8 +221,12 @@ public class RequestMappingHandlerMapping extends RequestMappingInfoHandlerMappi /** * Return the file extensions to use for suffix pattern matching. + * @deprecated as of 5.2.4. See class-level note on the deprecation of path + * extension config options. */ @Nullable + @Deprecated + @SuppressWarnings("deprecation") public List getFileExtensions() { return this.config.getFileExtensions(); } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java index 6dcf31c1d5..186c79d7bb 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java +++ b/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 PathExtensionContentNegotiationStrategy contentNegotiationStrategy; + private final Map mediaTypes = new HashMap<>(4); @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. + *

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 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 getMediaTypes() { + return this.mediaTypes; + } + /** * Specify the CORS configuration for resources served by this handler. *

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} - * 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 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. *

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 - * 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: + *

    + *
  1. {@link javax.servlet.ServletContext#getMimeType(String)} + *
  2. {@link #getMediaTypes()} + *
  3. {@link MediaTypeFactory#getMediaType(String)} + *
* @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; } /** diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java index 62ff42be89..9af18827f9 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java +++ b/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 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"); - handler.handleRequest(this.request, this.response); + MockHttpServletRequest request = new MockHttpServletRequest(servletContext, "GET", ""); + 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; }"); diff --git a/src/docs/asciidoc/web/webmvc.adoc b/src/docs/asciidoc/web/webmvc.adoc index 6de7b41083..af1a836792 100644 --- a/src/docs/asciidoc/web/webmvc.adoc +++ b/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 <>. +[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", - HandlerTypePredicate.forAnnotation(RestController.class)); + .addPathPrefix("/api", 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", - HandlerTypePredicate.forAnnotation(RestController::class.java)) + .addPathPrefix("/api", HandlerTypePredicate.forAnnotation(RestController::class.java)) } @Bean @@ -5835,7 +5844,6 @@ The following example shows how to achieve the same configuration in XML: ----