From cb0c0c503fae19d32e96ab9eab0c565fa37bcb62 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Tue, 2 Aug 2011 19:15:06 +0000 Subject: [PATCH] ContentNegotiatingViewResolver properly handles invalid accept headers (SPR-7712) --- .../view/ContentNegotiatingViewResolver.java | 135 +++++++++--------- .../ContentNegotiatingViewResolverTests.java | 15 +- 2 files changed, 82 insertions(+), 68 deletions(-) diff --git a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/view/ContentNegotiatingViewResolver.java b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/view/ContentNegotiatingViewResolver.java index 0a35822142..15a08d9b44 100644 --- a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/view/ContentNegotiatingViewResolver.java +++ b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/view/ContentNegotiatingViewResolver.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2010 the original author or authors. + * Copyright 2002-2011 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. @@ -66,17 +66,17 @@ import org.springframework.web.util.WebUtils; *

This view resolver uses the requested {@linkplain MediaType media type} to select a suitable {@link View} for a * request. This media type is determined by using the following criteria: *

    - *
  1. If the requested path has a file extension and if the {@link #setFavorPathExtension(boolean)} property is + *
  2. If the requested path has a file extension and if the {@link #setFavorPathExtension} property is * {@code true}, the {@link #setMediaTypes(Map) mediaTypes} property is inspected for a matching media type.
    3. - *
  3. If the request contains a parameter defining the extension and if the {@link #setFavorParameter(boolean)} + *
  4. If the request contains a parameter defining the extension and if the {@link #setFavorParameter} * property is true, the {@link #setMediaTypes(Map) mediaTypes} property is inspected for a matching * media type. The default name of the parameter is format and it can be configured using the * {@link #setParameterName(String) parameterName} property.
    5. *
  5. If there is no match in the {@link #setMediaTypes(Map) mediaTypes} property and if the Java Activation - * Framework (JAF) is both {@linkplain #setUseJaf(boolean) enabled} and present on the class path, + * Framework (JAF) is both {@linkplain #setUseJaf enabled} and present on the class path, * {@link FileTypeMap#getContentType(String)} is used instead.
    6. *
  6. If the previous steps did not result in a media type, and - * {@link #setIgnoreAcceptHeader(boolean) ignoreAcceptHeader} is {@code false}, the request {@code Accept} header is + * {@link #setIgnoreAcceptHeader ignoreAcceptHeader} is {@code false}, the request {@code Accept} header is * used.
    7. *
* @@ -145,7 +145,7 @@ public class ContentNegotiatingViewResolver extends WebApplicationObjectSupport } /** - * Indicates whether the extension of the request path should be used to determine the requested media type, + * Indicate whether the extension of the request path should be used to determine the requested media type, * in favor of looking at the {@code Accept} header. The default value is {@code true}. *

For instance, when this flag is true (the default), a request for {@code /hotels.pdf} * will result in an {@code AbstractPdfView} being resolved, while the {@code Accept} header can be the @@ -156,7 +156,7 @@ public class ContentNegotiatingViewResolver extends WebApplicationObjectSupport } /** - * Indicates whether a request parameter should be used to determine the requested media type, + * Indicate whether a request parameter should be used to determine the requested media type, * in favor of looking at the {@code Accept} header. The default value is {@code false}. *

For instance, when this flag is true, a request for {@code /hotels?format=pdf} will result * in an {@code AbstractPdfView} being resolved, while the {@code Accept} header can be the browser-defined @@ -167,39 +167,38 @@ public class ContentNegotiatingViewResolver extends WebApplicationObjectSupport } /** - * Sets the parameter name that can be used to determine the requested media type if the {@link - * #setFavorParameter(boolean)} property is {@code true}. The default parameter name is {@code format}. + * Set the parameter name that can be used to determine the requested media type if the {@link + * #setFavorParameter} property is {@code true}. The default parameter name is {@code format}. */ public void setParameterName(String parameterName) { this.parameterName = parameterName; } /** - * Indicates whether the HTTP {@code Accept} header should be ignored. Default is {@code false}. - * If set to {@code true}, this view resolver will only refer to the file extension and/or paramter, - * as indicated by the {@link #setFavorPathExtension(boolean) favorPathExtension} and - * {@link #setFavorParameter(boolean) favorParameter} properties. + * Indicate whether the HTTP {@code Accept} header should be ignored. Default is {@code false}. + *

If set to {@code true}, this view resolver will only refer to the file extension and/or + * parameter, as indicated by the {@link #setFavorPathExtension favorPathExtension} and + * {@link #setFavorParameter favorParameter} properties. */ public void setIgnoreAcceptHeader(boolean ignoreAcceptHeader) { this.ignoreAcceptHeader = ignoreAcceptHeader; } /** - * Indicates whether a {@link HttpServletResponse#SC_NOT_ACCEPTABLE 406 Not Acceptable} status code should be - * returned if no suitable view can be found. - * + * Indicate whether a {@link HttpServletResponse#SC_NOT_ACCEPTABLE 406 Not Acceptable} + * status code should be returned if no suitable view can be found. *

Default is {@code false}, meaning that this view resolver returns {@code null} for - * {@link #resolveViewName(String, Locale)} when an acceptable view cannot be found. This will allow for view - * resolvers chaining. When this property is set to {@code true}, - * {@link #resolveViewName(String, Locale)} will respond with a view that sets the response status to - * {@code 406 Not Acceptable} instead. + * {@link #resolveViewName(String, Locale)} when an acceptable view cannot be found. + * This will allow for view resolvers chaining. When this property is set to {@code true}, + * {@link #resolveViewName(String, Locale)} will respond with a view that sets the + * response status to {@code 406 Not Acceptable} instead. */ public void setUseNotAcceptableStatusCode(boolean useNotAcceptableStatusCode) { this.useNotAcceptableStatusCode = useNotAcceptableStatusCode; } /** - * Sets the mapping from file extensions to media types. + * Set the mapping from file extensions to media types. *

When this mapping is not set or when an extension is not present, this view resolver * will fall back to using a {@link FileTypeMap} when the Java Action Framework is available. */ @@ -213,7 +212,7 @@ public class ContentNegotiatingViewResolver extends WebApplicationObjectSupport } /** - * Sets the default views to use when a more specific view can not be obtained + * Set the default views to use when a more specific view can not be obtained * from the {@link ViewResolver} chain. */ public void setDefaultViews(List defaultViews) { @@ -221,7 +220,7 @@ public class ContentNegotiatingViewResolver extends WebApplicationObjectSupport } /** - * Sets the default content type. + * Set the default content type. *

This content type will be used when file extension, parameter, nor {@code Accept} * header define a content-type, either through being disabled or empty. */ @@ -230,7 +229,7 @@ public class ContentNegotiatingViewResolver extends WebApplicationObjectSupport } /** - * Indicates whether to use the Java Activation Framework to map from file extensions to media types. + * Indicate whether to use the Java Activation Framework to map from file extensions to media types. *

Default is {@code true}, i.e. the Java Activation Framework is used (if available). */ public void setUseJaf(boolean useJaf) { @@ -265,13 +264,38 @@ public class ContentNegotiatingViewResolver extends WebApplicationObjectSupport OrderComparator.sort(this.viewResolvers); } + public View resolveViewName(String viewName, Locale locale) throws Exception { + RequestAttributes attrs = RequestContextHolder.getRequestAttributes(); + Assert.isInstanceOf(ServletRequestAttributes.class, attrs); + List requestedMediaTypes = getMediaTypes(((ServletRequestAttributes) attrs).getRequest()); + if (requestedMediaTypes != null) { + List candidateViews = getCandidateViews(viewName, locale, requestedMediaTypes); + View bestView = getBestView(candidateViews, requestedMediaTypes); + if (bestView != null) { + return bestView; + } + } + if (this.useNotAcceptableStatusCode) { + if (logger.isDebugEnabled()) { + logger.debug("No acceptable view found; returning 406 (Not Acceptable) status code"); + } + return NOT_ACCEPTABLE_VIEW; + } + else { + if (logger.isDebugEnabled()) { + logger.debug("No acceptable view found; returning null"); + } + return null; + } + } + /** * Determines the list of {@link MediaType} for the given {@link HttpServletRequest}. *

The default implementation invokes {@link #getMediaTypeFromFilename(String)} if {@linkplain - * #setFavorPathExtension(boolean) favorPathExtension} property is true. If the property is - * false, or when a media type cannot be determined from the request path, this method will - * inspect the {@code Accept} header of the request. - *

This method can be overriden to provide a different algorithm. + * #setFavorPathExtension favorPathExtension} property is true. If the property is + * false, or when a media type cannot be determined from the request path, + * this method will inspect the {@code Accept} header of the request. + *

This method can be overridden to provide a different algorithm. * @param request the current servlet request * @return the list of media types requested, if any */ @@ -303,12 +327,20 @@ public class ContentNegotiatingViewResolver extends WebApplicationObjectSupport if (!this.ignoreAcceptHeader) { String acceptHeader = request.getHeader(ACCEPT_HEADER); if (StringUtils.hasText(acceptHeader)) { - List mediaTypes = MediaType.parseMediaTypes(acceptHeader); - MediaType.sortByQualityValue(mediaTypes); - if (logger.isDebugEnabled()) { - logger.debug("Requested media types are " + mediaTypes + " (based on Accept header)"); + try { + List mediaTypes = MediaType.parseMediaTypes(acceptHeader); + MediaType.sortByQualityValue(mediaTypes); + if (logger.isDebugEnabled()) { + logger.debug("Requested media types are " + mediaTypes + " (based on Accept header)"); + } + return mediaTypes; + } + catch (IllegalArgumentException ex) { + if (logger.isDebugEnabled()) { + logger.debug("Could not parse accept header [" + acceptHeader + "]: " + ex.getMessage()); + } + return null; } - return mediaTypes; } } if (this.defaultContentType != null) { @@ -360,31 +392,6 @@ public class ContentNegotiatingViewResolver extends WebApplicationObjectSupport return this.mediaTypes.get(parameterValue.toLowerCase(Locale.ENGLISH)); } - public View resolveViewName(String viewName, Locale locale) throws Exception { - RequestAttributes attrs = RequestContextHolder.getRequestAttributes(); - Assert.isInstanceOf(ServletRequestAttributes.class, attrs); - List requestedMediaTypes = getMediaTypes(((ServletRequestAttributes) attrs).getRequest()); - List candidateViews = getCandidateViews(viewName, locale, requestedMediaTypes); - View bestView = getBestView(candidateViews, requestedMediaTypes); - if (bestView != null) { - return bestView; - } - else { - if (this.useNotAcceptableStatusCode) { - if (logger.isDebugEnabled()) { - logger.debug("No acceptable view found; returning 406 (Not Acceptable) status code"); - } - return NOT_ACCEPTABLE_VIEW; - } - else { - if (logger.isDebugEnabled()) { - logger.debug("No acceptable view found; returning null"); - } - return null; - } - } - } - private List getCandidateViews(String viewName, Locale locale, List requestedMediaTypes) throws Exception { @@ -414,7 +421,7 @@ public class ContentNegotiatingViewResolver extends WebApplicationObjectSupport private List getExtensionsForMediaType(MediaType requestedMediaType) { List result = new ArrayList(); - for (Entry entry : mediaTypes.entrySet()) { + for (Entry entry : this.mediaTypes.entrySet()) { if (requestedMediaType.includes(entry.getValue())) { result.add(entry.getKey()); } @@ -438,9 +445,8 @@ public class ContentNegotiatingViewResolver extends WebApplicationObjectSupport } if (bestView != null) { if (logger.isDebugEnabled()) { - logger.debug( - "Returning [" + bestView + "] based on requested media type '" + bestRequestedMediaType + - "'"); + logger.debug("Returning [" + bestView + "] based on requested media type '" + + bestRequestedMediaType + "'"); } break; } @@ -495,7 +501,7 @@ public class ContentNegotiatingViewResolver extends WebApplicationObjectSupport public static MediaType getMediaType(String fileName) { String mediaType = fileTypeMap.getContentType(fileName); - return StringUtils.hasText(mediaType) ? MediaType.parseMediaType(mediaType) : null; + return (StringUtils.hasText(mediaType) ? MediaType.parseMediaType(mediaType) : null); } } @@ -506,8 +512,7 @@ public class ContentNegotiatingViewResolver extends WebApplicationObjectSupport return null; } - public void render(Map model, HttpServletRequest request, HttpServletResponse response) - throws Exception { + public void render(Map model, HttpServletRequest request, HttpServletResponse response) { response.setStatus(HttpServletResponse.SC_NOT_ACCEPTABLE); } }; diff --git a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/view/ContentNegotiatingViewResolverTests.java b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/view/ContentNegotiatingViewResolverTests.java index 000c0c3f28..35681c1f69 100644 --- a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/view/ContentNegotiatingViewResolverTests.java +++ b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/view/ContentNegotiatingViewResolverTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2010 the original author or authors. + * Copyright 2002-2011 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,9 +24,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; -import static org.easymock.EasyMock.*; import org.junit.After; -import static org.junit.Assert.*; import org.junit.Before; import org.junit.Test; @@ -38,6 +36,9 @@ import org.springframework.web.context.request.ServletRequestAttributes; import org.springframework.web.servlet.View; import org.springframework.web.servlet.ViewResolver; +import static org.easymock.EasyMock.*; +import static org.junit.Assert.*; + /** * @author Arjen Poutsma */ @@ -171,6 +172,14 @@ public class ContentNegotiatingViewResolverTests { verify(viewResolverMock, viewMock); } + @Test + public void resolveViewNameWithInvalidAcceptHeader() throws Exception { + request.addHeader("Accept", "application"); + + View result = viewResolver.resolveViewName("test", Locale.ENGLISH); + assertNull(result); + } + @Test public void resolveViewNameWithRequestParameter() throws Exception { request.addParameter("format", "xls");