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 46209caddd1..554d7e85da7 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 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 (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) { 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 6dcf31c1d52..186c79d7bb2 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 62ff42be894..9af18827f9e 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 6de7b41083c..af1a8367928 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: ----