From f1622569065374352b44a8e8dee9a16bbacef5a7 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Fri, 4 Mar 2016 13:24:14 -0500 Subject: [PATCH] Use ContentNegotiationManager for static resources The ResourceHttpRequestHandler now relies on the conifgured ContentNegotiationManager to determine the content type for resource requests rather than implementing that internally. First we check against the matched resource based on the resource file extension. Then we expand the check against the request with any configured content negotiation strategy. Issue: SPR-13658 --- .../web/accept/ContentNegotiationManager.java | 18 ++- ...thExtensionContentNegotiationStrategy.java | 29 +++- ...thExtensionContentNegotiationStrategy.java | 25 ++- ...stractMessageConverterMethodProcessor.java | 10 +- .../resource/ResourceHttpRequestHandler.java | 145 +++++++++++------- .../ResourceHttpRequestHandlerTests.java | 23 +++ 6 files changed, 181 insertions(+), 69 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/accept/ContentNegotiationManager.java b/spring-web/src/main/java/org/springframework/web/accept/ContentNegotiationManager.java index 4450ad10649..09c7f6abc36 100644 --- a/spring-web/src/main/java/org/springframework/web/accept/ContentNegotiationManager.java +++ b/spring-web/src/main/java/org/springframework/web/accept/ContentNegotiationManager.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2016 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. @@ -96,6 +96,22 @@ public class ContentNegotiationManager implements ContentNegotiationStrategy, return this.strategies; } + /** + * Find a {@code ContentNegotiationStrategy} of the given type. + * @param strategyType the strategy type + * @return the first matching strategy or {@code null}. + * @since 4.3 + */ + @SuppressWarnings("unchecked") + public T getStrategy(Class strategyType) { + for (ContentNegotiationStrategy strategy : getStrategies()) { + if (strategyType.isInstance(strategy)) { + return (T) strategy; + } + } + return null; + } + /** * Register more {@code MediaTypeFileExtensionResolver} instances in addition * to those detected at construction. 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 9b19c27154c..f6173264880 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-2015 the original author or authors. + * Copyright 2002-2016 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. @@ -30,6 +30,7 @@ import org.apache.commons.logging.LogFactory; import org.springframework.core.io.ClassPathResource; import org.springframework.core.io.Resource; import org.springframework.http.MediaType; +import org.springframework.util.Assert; import org.springframework.util.ClassUtils; import org.springframework.util.StringUtils; import org.springframework.web.HttpMediaTypeNotAcceptableException; @@ -135,6 +136,32 @@ public class PathExtensionContentNegotiationStrategy throw new HttpMediaTypeNotAcceptableException(getAllMediaTypes()); } + /** + * A public method exposing the knowledge of the path extension strategy to + * resolve file extensions to a MediaType in this case for a given + * {@link Resource}. The method first looks up any explicitly registered + * file extensions first and then falls back on JAF if available. + * @param resource the resource to look up + * @return the MediaType for the extension or {@code null}. + * @since 4.3 + */ + public MediaType getMediaTypeForResource(Resource resource) { + Assert.notNull(resource); + MediaType mediaType = null; + String filename = resource.getFilename(); + String extension = StringUtils.getFilenameExtension(filename); + if (extension != null) { + mediaType = lookupMediaType(extension); + } + if (mediaType == null && JAF_PRESENT) { + mediaType = JafMediaTypeFactory.getMediaType(filename); + } + if (MediaType.APPLICATION_OCTET_STREAM.equals(mediaType)) { + mediaType = null; + } + return mediaType; + } + /** * Inner class to avoid hard-coded dependency on JAF. 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 203387105ce..931e03fb35f 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-2015 the original author or authors. + * Copyright 2002-2016 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,6 +18,7 @@ package org.springframework.web.accept; import java.util.Map; import javax.servlet.ServletContext; +import org.springframework.core.io.Resource; import org.springframework.http.MediaType; import org.springframework.util.Assert; import org.springframework.util.StringUtils; @@ -82,4 +83,26 @@ public class ServletPathExtensionContentNegotiationStrategy return mediaType; } + /** + * Extends the base class + * {@link PathExtensionContentNegotiationStrategy#getMediaTypeForResource} + * with the ability to also look up through the ServletContext. + * @param resource the resource to look up + * @return the MediaType for the extension or {@code null}. + * @since 4.3 + */ + public MediaType getMediaTypeForResource(Resource resource) { + MediaType mediaType = super.getMediaTypeForResource(resource); + if (mediaType == null) { + String mimeType = this.servletContext.getMimeType(resource.getFilename()); + if (StringUtils.hasText(mimeType)) { + mediaType = MediaType.parseMediaType(mimeType); + } + } + if (MediaType.APPLICATION_OCTET_STREAM.equals(mediaType)) { + mediaType = null; + } + return mediaType; + } + } 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 d080ee1a9cc..b2a37b077ab 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 @@ -43,7 +43,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.ContentNegotiationStrategy; import org.springframework.web.accept.PathExtensionContentNegotiationStrategy; import org.springframework.web.context.request.NativeWebRequest; import org.springframework.web.context.request.ServletWebRequest; @@ -122,12 +121,9 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe } private static PathExtensionContentNegotiationStrategy initPathStrategy(ContentNegotiationManager manager) { - for (ContentNegotiationStrategy strategy : manager.getStrategies()) { - if (strategy instanceof PathExtensionContentNegotiationStrategy) { - return (PathExtensionContentNegotiationStrategy) strategy; - } - } - return new PathExtensionContentNegotiationStrategy(); + Class clazz = PathExtensionContentNegotiationStrategy.class; + PathExtensionContentNegotiationStrategy strategy = manager.getStrategy(clazz); + return (strategy != null ? strategy : new PathExtensionContentNegotiationStrategy()); } 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 1f58af40f9a..b13d7fbc320 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 @@ -23,8 +23,6 @@ import java.io.OutputStream; import java.net.URLDecoder; import java.util.ArrayList; import java.util.List; -import javax.activation.FileTypeMap; -import javax.activation.MimetypesFileTypeMap; import javax.servlet.ServletException; import javax.servlet.ServletOutputStream; import javax.servlet.http.HttpServletRequest; @@ -34,7 +32,6 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.beans.factory.InitializingBean; -import org.springframework.core.io.ClassPathResource; import org.springframework.core.io.Resource; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; @@ -49,7 +46,11 @@ import org.springframework.util.ObjectUtils; import org.springframework.util.ResourceUtils; import org.springframework.util.StreamUtils; import org.springframework.util.StringUtils; +import org.springframework.web.HttpMediaTypeNotAcceptableException; 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.context.request.ServletWebRequest; import org.springframework.web.cors.CorsConfiguration; import org.springframework.web.cors.CorsConfigurationSource; @@ -103,6 +104,8 @@ public class ResourceHttpRequestHandler extends WebContentGenerator private final List resourceTransformers = new ArrayList(4); + private ContentNegotiationManager contentNegotiationManager; + private CorsConfiguration corsConfiguration; @@ -162,6 +165,28 @@ public class ResourceHttpRequestHandler extends WebContentGenerator return this.resourceTransformers; } + /** + * Configure a {@code ContentNegotiationManager} to determine the media types + * for resources being served. If the manager contains a path + * extension strategy it will be used to look up the file extension + * of resources being served via + * {@link PathExtensionContentNegotiationStrategy#getMediaTypeForResource + * getMediaTypeForResource}. If that fails the check is then expanded + * to use any configured content negotiation strategy against the request. + *

By default a {@link ContentNegotiationManagerFactoryBean} with default + * settings is used to create the manager. See the Javadoc of + * {@code ContentNegotiationManagerFactoryBean} for details + * @param contentNegotiationManager the manager to use + * @since 4.3 + */ + public void setContentNegotiationManager(ContentNegotiationManager contentNegotiationManager) { + this.contentNegotiationManager = contentNegotiationManager; + } + + public ContentNegotiationManager getContentNegotiationManager() { + return this.contentNegotiationManager; + } + /** * Specify the CORS configuration for resources served by this handler. *

By default this is not set in which allows cross-origin requests. @@ -186,6 +211,10 @@ public class ResourceHttpRequestHandler extends WebContentGenerator this.resourceResolvers.add(new PathResourceResolver()); } initAllowedLocations(); + + if (this.contentNegotiationManager == null) { + this.contentNegotiationManager = initContentNegotiationManager(); + } } /** @@ -208,6 +237,17 @@ public class ResourceHttpRequestHandler extends WebContentGenerator } } + /** + * Create the {@code ContentNegotiationManager} to use to resolve the + * {@link MediaType} for requests. This implementation delegates to + * {@link ContentNegotiationManagerFactoryBean} with default settings. + */ + protected ContentNegotiationManager initContentNegotiationManager() { + ContentNegotiationManagerFactoryBean factory = new ContentNegotiationManagerFactoryBean(); + factory.afterPropertiesSet(); + return factory.getObject(); + } + /** * Processes a resource request. @@ -250,8 +290,8 @@ public class ResourceHttpRequestHandler extends WebContentGenerator // Apply cache settings, if any prepareResponse(response); - // Check the resource's media type - MediaType mediaType = getMediaType(resource); + // Check the media type for the resource + MediaType mediaType = getMediaType(request, resource); if (mediaType != null) { if (logger.isTraceEnabled()) { logger.trace("Determined media type '" + mediaType + "' for " + resource); @@ -391,25 +431,56 @@ public class ResourceHttpRequestHandler extends WebContentGenerator } /** - * Determine an appropriate media type for the given resource. + * Determine the media type for the given request and the resource matched + * to it. This implementation first tries to determine the MediaType based + * strictly on the file extension of the Resource via + * {@link PathExtensionContentNegotiationStrategy#getMediaTypeForResource} + * and then expands to check against the request via + * {@link ContentNegotiationManager#resolveMediaTypes}. + * @param request the current request * @param resource the resource to check * @return the corresponding media type, or {@code null} if none found */ - protected MediaType getMediaType(Resource resource) { - MediaType mediaType = null; - String mimeType = getServletContext().getMimeType(resource.getFilename()); - if (StringUtils.hasText(mimeType)) { - mediaType = MediaType.parseMediaType(mimeType); + @SuppressWarnings("deprecation") + protected MediaType getMediaType(HttpServletRequest request, Resource resource) { + + // For backwards compatibility + MediaType mediaType = getMediaType(resource); + if (mediaType != null) { + return mediaType; } - if (jafPresent && (mediaType == null || MediaType.APPLICATION_OCTET_STREAM.equals(mediaType))) { - MediaType jafMediaType = ActivationMediaTypeFactory.getMediaType(resource.getFilename()); - if (jafMediaType != null && !MediaType.APPLICATION_OCTET_STREAM.equals(jafMediaType)) { - mediaType = jafMediaType; + + 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 { + getContentNegotiationManager().resolveMediaTypes(webRequest); + } + catch (HttpMediaTypeNotAcceptableException ex) { + // Ignore } } + return mediaType; } + /** + * Determine an appropriate media type for the given resource. + * @param resource the resource to check + * @return the corresponding media type, or {@code null} if none found + * @deprecated as of 4.3 this method is deprecated; please override + * {@link #getMediaType(HttpServletRequest, Resource)} instead. + */ + @Deprecated + protected MediaType getMediaType(Resource resource) { + return null; + } + /** * Set headers on the given servlet response. * Called for GET requests as well as HEAD requests. @@ -575,48 +646,4 @@ public class ResourceHttpRequestHandler extends WebContentGenerator return "ResourceHttpRequestHandler [locations=" + getLocations() + ", resolvers=" + getResourceResolvers() + "]"; } - - /** - * Inner class to avoid a hard-coded JAF dependency. - */ - private static class ActivationMediaTypeFactory { - - private static final FileTypeMap fileTypeMap; - - static { - fileTypeMap = loadFileTypeMapFromContextSupportModule(); - } - - private static FileTypeMap loadFileTypeMapFromContextSupportModule() { - // See if we can find the extended mime.types from the context-support module... - Resource mappingLocation = new ClassPathResource("org/springframework/mail/javamail/mime.types"); - if (mappingLocation.exists()) { - InputStream inputStream = null; - try { - inputStream = mappingLocation.getInputStream(); - return new MimetypesFileTypeMap(inputStream); - } - catch (IOException ex) { - // ignore - } - finally { - if (inputStream != null) { - try { - inputStream.close(); - } - catch (IOException ex) { - // ignore - } - } - } - } - return FileTypeMap.getDefaultFileTypeMap(); - } - - public static MediaType getMediaType(String filename) { - String mediaType = fileTypeMap.getContentType(filename); - return (StringUtils.hasText(mediaType) ? MediaType.parseMediaType(mediaType) : null); - } - } - } 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 276966fe4ed..f904a3829c9 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 @@ -42,11 +42,14 @@ import org.springframework.core.io.Resource; import org.springframework.core.io.UrlResource; import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; +import org.springframework.http.MediaType; import org.springframework.mock.web.test.MockHttpServletRequest; import org.springframework.mock.web.test.MockHttpServletResponse; import org.springframework.mock.web.test.MockServletContext; import org.springframework.util.StringUtils; import org.springframework.web.HttpRequestMethodNotSupportedException; +import org.springframework.web.accept.ContentNegotiationManager; +import org.springframework.web.accept.ContentNegotiationManagerFactoryBean; import org.springframework.web.servlet.HandlerMapping; /** @@ -225,6 +228,26 @@ public class ResourceHttpRequestHandlerTests { assertEquals("function foo() { console.log(\"hello world\"); }", this.response.getContentAsString()); } + @Test // SPR-13658 + public void getResourceWithRegisteredMediaType() throws Exception { + ContentNegotiationManagerFactoryBean factory = new ContentNegotiationManagerFactoryBean(); + factory.addMediaType("css", 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(); + + this.request.setAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, "foo.css"); + this.handler.handleRequest(this.request, this.response); + + assertEquals("foo/bar", this.response.getContentType()); + assertEquals("h1 { color:red; }", this.response.getContentAsString()); + } + @Test public void invalidPath() throws Exception { for (HttpMethod method : HttpMethod.values()) {