From fe404628e917a2efc0cf771a24dad50ca73618dc Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Mon, 29 Aug 2016 16:27:24 -0400 Subject: [PATCH] Fix media type regression in resource handling Issue: SPR-14577 --- ...MappingMediaTypeFileExtensionResolver.java | 4 ++ .../resource/ResourceHttpRequestHandler.java | 54 ++++++++----------- .../ResourceHttpRequestHandlerTests.java | 44 +++++++-------- .../resource/ResourceUrlProviderTests.java | 1 + 4 files changed, 50 insertions(+), 53 deletions(-) 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 49c38d6dd34..9c518c4eb43 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 @@ -67,6 +67,10 @@ public class MappingMediaTypeFileExtensionResolver implements MediaTypeFileExten } + public Map getMediaTypes() { + return this.mediaTypes; + } + protected List getAllMediaTypes() { return new ArrayList(this.mediaTypes.values()); } 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 823fec222cb..4e4cc695300 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 @@ -19,7 +19,9 @@ package org.springframework.web.servlet.resource; import java.io.IOException; import java.net.URLDecoder; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; import javax.servlet.ServletContext; import javax.servlet.ServletException; import javax.servlet.ServletResponse; @@ -51,6 +53,7 @@ import org.springframework.web.HttpRequestHandler; import org.springframework.web.accept.ContentNegotiationManager; import org.springframework.web.accept.ContentNegotiationManagerFactoryBean; 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; @@ -111,7 +114,9 @@ public class ResourceHttpRequestHandler extends WebContentGenerator private ContentNegotiationManager contentNegotiationManager; - private final ContentNegotiationManagerFactoryBean cnmFactoryBean = new ContentNegotiationManagerFactoryBean(); + private ServletPathExtensionContentNegotiationStrategy pathExtensionStrategy; + + private ServletContext servletContext; private CorsConfiguration corsConfiguration; @@ -254,7 +259,7 @@ public class ResourceHttpRequestHandler extends WebContentGenerator @Override protected void initServletContext(ServletContext servletContext) { - this.cnmFactoryBean.setServletContext(servletContext); + this.servletContext = servletContext; } @@ -268,16 +273,13 @@ public class ResourceHttpRequestHandler extends WebContentGenerator this.resourceResolvers.add(new PathResourceResolver()); } initAllowedLocations(); - if (this.contentNegotiationManager == null) { - this.cnmFactoryBean.afterPropertiesSet(); - this.contentNegotiationManager = this.cnmFactoryBean.getObject(); - } if (this.resourceHttpMessageConverter == null) { this.resourceHttpMessageConverter = new ResourceHttpMessageConverter(); } if (this.resourceRegionHttpMessageConverter == null) { this.resourceRegionHttpMessageConverter = new ResourceRegionHttpMessageConverter(); } + this.pathExtensionStrategy = initPathExtensionStrategy(); } /** @@ -300,6 +302,19 @@ public class ResourceHttpRequestHandler extends WebContentGenerator } } + protected ServletPathExtensionContentNegotiationStrategy initPathExtensionStrategy() { + Map mediaTypes = null; + if (getContentNegotiationManager() != null) { + PathExtensionContentNegotiationStrategy strategy = + getContentNegotiationManager().getStrategy(PathExtensionContentNegotiationStrategy.class); + if (strategy != null) { + mediaTypes = new HashMap(strategy.getMediaTypes()); + } + } + return new ServletPathExtensionContentNegotiationStrategy(this.servletContext, mediaTypes); + } + + /** * Processes a resource request. *

Checks for the existence of the requested resource in the configured list of locations. @@ -512,32 +527,7 @@ public class ResourceHttpRequestHandler extends WebContentGenerator */ @SuppressWarnings("deprecation") protected MediaType getMediaType(HttpServletRequest request, Resource resource) { - // For backwards compatibility - MediaType mediaType = getMediaType(resource); - if (mediaType != null) { - return mediaType; - } - - Class clazz = PathExtensionContentNegotiationStrategy.class; - PathExtensionContentNegotiationStrategy strategy = this.contentNegotiationManager.getStrategy(clazz); - if (strategy != null) { - mediaType = strategy.getMediaTypeForResource(resource); - } - - if (mediaType == null) { - ServletWebRequest webRequest = new ServletWebRequest(request); - try { - List mediaTypes = getContentNegotiationManager().resolveMediaTypes(webRequest); - if (!mediaTypes.isEmpty()) { - mediaType = mediaTypes.get(0); - } - } - catch (HttpMediaTypeNotAcceptableException ex) { - // Ignore - } - } - - return mediaType; + return this.pathExtensionStrategy.getMediaTypeForResource(resource); } /** 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 c78f75739ef..e835aff8c15 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 @@ -248,37 +248,38 @@ public class ResourceHttpRequestHandlerTests { ContentNegotiationManager manager = factory.getObject(); List paths = Collections.singletonList(new ClassPathResource("test/", getClass())); - this.handler = new ResourceHttpRequestHandler(); - this.handler.setLocations(paths); - this.handler.setContentNegotiationManager(manager); - this.handler.afterPropertiesSet(); + ResourceHttpRequestHandler handler = new ResourceHttpRequestHandler(); + handler.setServletContext(new MockServletContext()); + handler.setLocations(paths); + handler.setContentNegotiationManager(manager); + handler.afterPropertiesSet(); this.request.setAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, "foo.css"); - this.handler.handleRequest(this.request, this.response); + handler.handleRequest(this.request, this.response); assertEquals("foo/bar", this.response.getContentType()); assertEquals("h1 { color:red; }", this.response.getContentAsString()); } - @Test // SPR-13658 - public void getResourceWithRegisteredMediaTypeDefaultStrategy() throws Exception { + @Test // SPR-14577 + public void getMediaTypeWithFavorPathExtensionOff() throws Exception { ContentNegotiationManagerFactoryBean factory = new ContentNegotiationManagerFactoryBean(); factory.setFavorPathExtension(false); - factory.setDefaultContentType(new MediaType("foo", "bar")); factory.afterPropertiesSet(); ContentNegotiationManager manager = factory.getObject(); List paths = Collections.singletonList(new ClassPathResource("test/", getClass())); - this.handler = new ResourceHttpRequestHandler(); - this.handler.setLocations(paths); - this.handler.setContentNegotiationManager(manager); - this.handler.afterPropertiesSet(); + ResourceHttpRequestHandler handler = new ResourceHttpRequestHandler(); + handler.setServletContext(new MockServletContext()); + handler.setLocations(paths); + handler.setContentNegotiationManager(manager); + handler.afterPropertiesSet(); - this.request.setAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, "foo.css"); - this.handler.handleRequest(this.request, this.response); + this.request.addHeader("Accept", "application/json,text/plain,*/*"); + this.request.setAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, "foo.html"); + handler.handleRequest(this.request, this.response); - assertEquals("foo/bar", this.response.getContentType()); - assertEquals("h1 { color:red; }", this.response.getContentAsString()); + assertEquals("text/html", this.response.getContentType()); } @Test // SPR-14368 @@ -297,13 +298,13 @@ public class ResourceHttpRequestHandlerTests { }; List paths = Collections.singletonList(new ClassPathResource("test/", getClass())); - this.handler = new ResourceHttpRequestHandler(); - this.handler.setServletContext(servletContext); - this.handler.setLocations(paths); - this.handler.afterPropertiesSet(); + ResourceHttpRequestHandler handler = new ResourceHttpRequestHandler(); + handler.setServletContext(servletContext); + handler.setLocations(paths); + handler.afterPropertiesSet(); this.request.setAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, "foo.css"); - this.handler.handleRequest(this.request, this.response); + handler.handleRequest(this.request, this.response); assertEquals("foo/bar", this.response.getContentType()); assertEquals("h1 { color:red; }", this.response.getContentAsString()); @@ -412,6 +413,7 @@ public class ResourceHttpRequestHandlerTests { ResourceHttpRequestHandler handler = new ResourceHttpRequestHandler(); handler.setResourceResolvers(Collections.singletonList(pathResolver)); + handler.setServletContext(new MockServletContext()); handler.setLocations(Arrays.asList(location1, location2)); handler.afterPropertiesSet(); diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceUrlProviderTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceUrlProviderTests.java index fbb222c01b7..85d4e669cfe 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceUrlProviderTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceUrlProviderTests.java @@ -58,6 +58,7 @@ public class ResourceUrlProviderTests { public void setUp() throws Exception { this.locations.add(new ClassPathResource("test/", getClass())); this.locations.add(new ClassPathResource("testalternatepath/", getClass())); + this.handler.setServletContext(new MockServletContext()); this.handler.setLocations(locations); this.handler.afterPropertiesSet(); this.handlerMap.put("/resources/**", this.handler);