Update ContentNegotiationManager for unknown path exts

This change refines the logic of "mapping" content negotiation strategies with regards to how to handle cases where no mapping is found. The request parameter strategy now treats request parameter values that do not match any mapped media type as 406 errors. The path extension strategy provides a new flag called "ignoreUnknownExtensions" (true by default) that when set to false also results in a 406. The same flag is also exposed through the ContentNegotiationManagerFactoryBean and the MVC Java config. Issue: SPR-10170
guosean · May 1, 2014 · 0d2aa51 · 0d2aa51
1 parent c50887c
commit 0d2aa51
Showing 11 changed files with 175 additions and 42 deletions.
diff --git a/...c/main/java/org/springframework/web/accept/AbstractMappingContentNegotiationStrategy.java b/...c/main/java/org/springframework/web/accept/AbstractMappingContentNegotiationStrategy.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2014 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 org.springframework.http.MediaType;
 import org.springframework.util.StringUtils;
+import org.springframework.web.HttpMediaTypeNotAcceptableException;
 import org.springframework.web.context.request.NativeWebRequest;
 
 /**
@@ -34,6 +35,7 @@
 public abstract class AbstractMappingContentNegotiationStrategy extends MappingMediaTypeFileExtensionResolver
 		implements ContentNegotiationStrategy, MediaTypeFileExtensionResolver {
 
+
 	/**
 	 * Create an instance with the given extension-to-MediaType lookup.
 	 * @throws IllegalArgumentException if a media type string cannot be parsed
@@ -42,8 +44,9 @@ public AbstractMappingContentNegotiationStrategy(Map<String, MediaType> mediaTyp
 		super(mediaTypes);
 	}
 
+
 	@Override
-	public List<MediaType> resolveMediaTypes(NativeWebRequest webRequest) {
+	public List<MediaType> resolveMediaTypes(NativeWebRequest webRequest) throws HttpMediaTypeNotAcceptableException {
 		String key = getMediaTypeKey(webRequest);
 		if (StringUtils.hasText(key)) {
 			MediaType mediaType = lookupMediaType(key);
@@ -76,7 +79,7 @@ protected void handleMatch(String mappingKey, MediaType mediaType) {
 	 * Invoked when no matching media type is found in the lookup map.
 	 * Sub-classes can take further steps to determine the media type.
 	 */
-	protected MediaType handleNoMatch(NativeWebRequest request, String mappingKey) {
+	protected MediaType handleNoMatch(NativeWebRequest request, String key) throws HttpMediaTypeNotAcceptableException {
 		return null;
 	}
 

diff --git a/...eb/src/main/java/org/springframework/web/accept/ContentNegotiationManagerFactoryBean.java b/...eb/src/main/java/org/springframework/web/accept/ContentNegotiationManagerFactoryBean.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2014 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.
@@ -55,6 +55,8 @@ public class ContentNegotiationManagerFactoryBean
 
 	private Map<String, MediaType> mediaTypes = new HashMap<String, MediaType>();
 
+	private boolean ignoreUnknownPathExtensions = true;
+
 	private Boolean useJaf;
 
 	private String parameterName = "format";
@@ -116,6 +118,17 @@ public void addMediaTypes(Map<String, MediaType> mediaTypes) {
 		}
 	}
 
+	/**
+	 * Whether to ignore requests that have a file extension that does not match
+	 * any mapped media types. Setting this to {@code false} will result in a
+	 * {@code HttpMediaTypeNotAcceptableException} when there is no match.
+	 *
+	 * <p>By default this is set to {@code true}.
+	 */
+	public void setIgnoreUnknownPathExtensions(boolean ignoreUnknownPathExtensions) {
+		this.ignoreUnknownPathExtensions = ignoreUnknownPathExtensions;
+	}
+
 	/**
 	 * Indicate whether to use the Java Activation Framework as a fallback option
 	 * to map from file extensions to media types. This is used only when
@@ -191,6 +204,7 @@ public void afterPropertiesSet() {
 			} else {
 				strategy = new PathExtensionContentNegotiationStrategy(this.mediaTypes);
 			}
+			strategy.setIgnoreUnknownExtensions(this.ignoreUnknownPathExtensions);
 			if (this.useJaf != null) {
 				strategy.setUseJaf(this.useJaf);
 			}

diff --git a/...b/src/main/java/org/springframework/web/accept/MappingMediaTypeFileExtensionResolver.java b/...b/src/main/java/org/springframework/web/accept/MappingMediaTypeFileExtensionResolver.java
@@ -16,6 +16,7 @@
 
 package org.springframework.web.accept;
 
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.LinkedList;
 import java.util.List;
@@ -59,6 +60,7 @@ public MappingMediaTypeFileExtensionResolver(Map<String, MediaType> mediaTypes)
 		}
 	}
 
+
 	/**
 	 * Find the file extensions mapped to the given MediaType.
 	 * @return 0 or more extensions, never {@code null}
@@ -74,6 +76,10 @@ public List<String> getAllFileExtensions() {
 		return Collections.unmodifiableList(this.allFileExtensions);
 	}
 
+	protected List<MediaType> getAllMediaTypes() {
+		return new ArrayList<MediaType>(this.mediaTypes.values());
+	}
+
 	/**
 	 * Return the MediaType mapped to the given extension.
 	 * @return a MediaType for the key or {@code null}

diff --git a/...web/src/main/java/org/springframework/web/accept/ParameterContentNegotiationStrategy.java b/...web/src/main/java/org/springframework/web/accept/ParameterContentNegotiationStrategy.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2014 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 org.apache.commons.logging.LogFactory;
 import org.springframework.http.MediaType;
 import org.springframework.util.Assert;
+import org.springframework.web.HttpMediaTypeNotAcceptableException;
 import org.springframework.web.context.request.NativeWebRequest;
 
 /**
@@ -68,4 +69,8 @@ protected void handleMatch(String mediaTypeKey, MediaType mediaType) {
 		}
 	}
 
+	@Override
+	protected MediaType handleNoMatch(NativeWebRequest request, String key) throws HttpMediaTypeNotAcceptableException {
+		throw new HttpMediaTypeNotAcceptableException(getAllMediaTypes());
+	}
 }
diff --git a/...src/main/java/org/springframework/web/accept/PathExtensionContentNegotiationStrategy.java b/...src/main/java/org/springframework/web/accept/PathExtensionContentNegotiationStrategy.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2014 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,6 +32,7 @@
 import org.springframework.http.MediaType;
 import org.springframework.util.ClassUtils;
 import org.springframework.util.StringUtils;
+import org.springframework.web.HttpMediaTypeNotAcceptableException;
 import org.springframework.web.context.request.NativeWebRequest;
 import org.springframework.web.util.UrlPathHelper;
 import org.springframework.web.util.WebUtils;
@@ -65,6 +66,8 @@ public class PathExtensionContentNegotiationStrategy extends AbstractMappingCont
 
 	private boolean useJaf = JAF_PRESENT;
 
+	private boolean ignoreUnknownExtensions = true;
+
 
 	/**
 	 * Create an instance with the given extension-to-MediaType lookup.
@@ -82,14 +85,30 @@ public PathExtensionContentNegotiationStrategy() {
 		super(null);
 	}
 
+
 	/**
-	 * Indicate whether to use the Java Activation Framework to map from file extensions to media types.
-	 * <p>Default is {@code true}, i.e. the Java Activation Framework is used (if available).
+	 * Indicate whether to use the Java Activation Framework to map from file
+	 * extensions to media types.
+	 *
+	 * <p>Default is {@code true}, i.e. the Java Activation Framework is used
+	 * (if available).
 	 */
 	public void setUseJaf(boolean useJaf) {
 		this.useJaf = useJaf;
 	}
 
+	/**
+	 * Whether to ignore requests that have a file extension that does not match
+	 * any mapped media types. Setting this to {@code false} will result in a
+	 * {@code HttpMediaTypeNotAcceptableException}.
+	 *
+	 * <p>By default this is set to {@code true}.
+	 */
+	public void setIgnoreUnknownExtensions(boolean ignoreUnknownExtensions) {
+		this.ignoreUnknownExtensions = ignoreUnknownExtensions;
+	}
+
+
 	@Override
 	protected String getMediaTypeKey(NativeWebRequest webRequest) {
 		HttpServletRequest servletRequest = webRequest.getNativeRequest(HttpServletRequest.class);
@@ -108,13 +127,18 @@ protected void handleMatch(String extension, MediaType mediaType) {
 	}
 
 	@Override
-	protected MediaType handleNoMatch(NativeWebRequest webRequest, String extension) {
+	protected MediaType handleNoMatch(NativeWebRequest webRequest, String extension)
+			throws HttpMediaTypeNotAcceptableException {
+
 		if (this.useJaf) {
 			MediaType jafMediaType = JafMediaTypeFactory.getMediaType("file." + extension);
 			if (jafMediaType != null && !MediaType.APPLICATION_OCTET_STREAM.equals(jafMediaType)) {
 				return jafMediaType;
 			}
 		}
+		if (!this.ignoreUnknownExtensions) {
+			throw new HttpMediaTypeNotAcceptableException(getAllMediaTypes());
+		}
 		return null;
 	}
 

diff --git a/...n/java/org/springframework/web/accept/ServletPathExtensionContentNegotiationStrategy.java b/...n/java/org/springframework/web/accept/ServletPathExtensionContentNegotiationStrategy.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2014 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 org.springframework.http.MediaType;
 import org.springframework.util.Assert;
 import org.springframework.util.StringUtils;
+import org.springframework.web.HttpMediaTypeNotAcceptableException;
 import org.springframework.web.context.request.NativeWebRequest;
 
 /**
@@ -64,7 +65,9 @@ public ServletPathExtensionContentNegotiationStrategy(ServletContext servletCont
 	 * and if that doesn't help, delegate to the parent implementation.
 	 */
 	@Override
-	protected MediaType handleNoMatch(NativeWebRequest webRequest, String extension) {
+	protected MediaType handleNoMatch(NativeWebRequest webRequest, String extension)
+			throws HttpMediaTypeNotAcceptableException {
+
 		MediaType mediaType = null;
 		if (this.servletContext != null) {
 			String mimeType = this.servletContext.getMimeType("file." + extension);

diff --git a/...c/test/java/org/springframework/web/accept/ContentNegotiationManagerFactoryBeanTests.java b/...c/test/java/org/springframework/web/accept/ContentNegotiationManagerFactoryBeanTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2014 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.
@@ -26,6 +26,7 @@
 import org.junit.Test;
 import org.springframework.http.MediaType;
 import org.springframework.mock.web.test.MockHttpServletRequest;
+import org.springframework.web.HttpMediaTypeNotAcceptableException;
 import org.springframework.web.context.request.NativeWebRequest;
 import org.springframework.web.context.request.ServletWebRequest;
 
@@ -41,6 +42,7 @@ public class ContentNegotiationManagerFactoryBeanTests {
 
 	private MockHttpServletRequest servletRequest;
 
+
 	@Before
 	public void setup() {
 		this.servletRequest = new MockHttpServletRequest();
@@ -50,6 +52,7 @@ public void setup() {
 		this.factoryBean.setServletContext(this.servletRequest.getServletContext());
 	}
 
+
 	@Test
 	public void defaultSettings() throws Exception {
 		this.factoryBean.afterPropertiesSet();
@@ -60,11 +63,16 @@ public void defaultSettings() throws Exception {
 		assertEquals("Should be able to resolve file extensions by default",
 				Arrays.asList(MediaType.IMAGE_GIF), manager.resolveMediaTypes(this.webRequest));
 
-		this.servletRequest.setRequestURI("/flower?format=gif");
-		this.servletRequest.addParameter("format", "gif");
+		this.servletRequest.setRequestURI("/flower.xyz");
+
+		assertEquals("Should ignore unknown extensions by default",
+				Collections.<MediaType>emptyList(), manager.resolveMediaTypes(this.webRequest));
+
+		this.servletRequest.setRequestURI("/flower");
+		this.servletRequest.setParameter("format", "gif");
 
 		assertEquals("Should not resolve request parameters by default",
-				Collections.emptyList(), manager.resolveMediaTypes(this.webRequest));
+				Collections.<MediaType>emptyList(), manager.resolveMediaTypes(this.webRequest));
 
 		this.servletRequest.setRequestURI("/flower");
 		this.servletRequest.addHeader("Accept", MediaType.IMAGE_GIF_VALUE);
@@ -75,7 +83,7 @@ public void defaultSettings() throws Exception {
 
 	@Test
 	public void addMediaTypes() throws Exception {
-		Map<String, MediaType> mediaTypes = new HashMap<String, MediaType>();
+		Map<String, MediaType> mediaTypes = new HashMap<>();
 		mediaTypes.put("json", MediaType.APPLICATION_JSON);
 		this.factoryBean.addMediaTypes(mediaTypes);
 
@@ -86,11 +94,26 @@ public void addMediaTypes() throws Exception {
 		assertEquals(Arrays.asList(MediaType.APPLICATION_JSON), manager.resolveMediaTypes(this.webRequest));
 	}
 
+	// SPR-10170
+
+	@Test(expected = HttpMediaTypeNotAcceptableException.class)
+	public void favorPathExtensionWithUnknownMediaType() throws Exception {
+		this.factoryBean.setFavorPathExtension(true);
+		this.factoryBean.setIgnoreUnknownPathExtensions(false);
+		this.factoryBean.afterPropertiesSet();
+		ContentNegotiationManager manager = this.factoryBean.getObject();
+
+		this.servletRequest.setRequestURI("/flower.xyz");
+		this.servletRequest.addParameter("format", "json");
+
+		manager.resolveMediaTypes(this.webRequest);
+	}
+
 	@Test
 	public void favorParameter() throws Exception {
 		this.factoryBean.setFavorParameter(true);
 
-		Map<String, MediaType> mediaTypes = new HashMap<String, MediaType>();
+		Map<String, MediaType> mediaTypes = new HashMap<>();
 		mediaTypes.put("json", MediaType.APPLICATION_JSON);
 		this.factoryBean.addMediaTypes(mediaTypes);
 
@@ -103,6 +126,20 @@ public void favorParameter() throws Exception {
 		assertEquals(Arrays.asList(MediaType.APPLICATION_JSON), manager.resolveMediaTypes(this.webRequest));
 	}
 
+	// SPR-10170
+
+	@Test(expected = HttpMediaTypeNotAcceptableException.class)
+	public void favorParameterWithUnknownMediaType() throws HttpMediaTypeNotAcceptableException {
+		this.factoryBean.setFavorParameter(true);
+		this.factoryBean.afterPropertiesSet();
+		ContentNegotiationManager manager = this.factoryBean.getObject();
+
+		this.servletRequest.setRequestURI("/flower");
+		this.servletRequest.setParameter("format", "xyz");
+
+		manager.resolveMediaTypes(this.webRequest);
+	}
+
 	@Test
 	public void ignoreAcceptHeader() throws Exception {
 		this.factoryBean.setIgnoreAcceptHeader(true);
@@ -112,7 +149,7 @@ public void ignoreAcceptHeader() throws Exception {
 		this.servletRequest.setRequestURI("/flower");
 		this.servletRequest.addHeader("Accept", MediaType.IMAGE_GIF_VALUE);
 
-		assertEquals(Collections.emptyList(), manager.resolveMediaTypes(this.webRequest));
+		assertEquals(Collections.<MediaType>emptyList(), manager.resolveMediaTypes(this.webRequest));
 	}
 
 	@Test

diff --git a/.../src/test/java/org/springframework/web/accept/MappingContentNegotiationStrategyTests.java b/.../src/test/java/org/springframework/web/accept/MappingContentNegotiationStrategyTests.java
@@ -35,7 +35,7 @@
 public class MappingContentNegotiationStrategyTests {
 
 	@Test
-	public void resolveMediaTypes() {
+	public void resolveMediaTypes() throws Exception {
 		Map<String, MediaType> mapping = Collections.singletonMap("json", MediaType.APPLICATION_JSON);
 		TestMappingContentNegotiationStrategy strategy = new TestMappingContentNegotiationStrategy("json", mapping);
 
@@ -46,7 +46,7 @@ public void resolveMediaTypes() {
 	}
 
 	@Test
-	public void resolveMediaTypesNoMatch() {
+	public void resolveMediaTypesNoMatch() throws Exception {
 		Map<String, MediaType> mapping = null;
 		TestMappingContentNegotiationStrategy strategy = new TestMappingContentNegotiationStrategy("blah", mapping);
 
@@ -56,7 +56,7 @@ public void resolveMediaTypesNoMatch() {
 	}
 
 	@Test
-	public void resolveMediaTypesNoKey() {
+	public void resolveMediaTypesNoKey() throws Exception {
 		Map<String, MediaType> mapping = Collections.singletonMap("json", MediaType.APPLICATION_JSON);
 		TestMappingContentNegotiationStrategy strategy = new TestMappingContentNegotiationStrategy(null, mapping);
 
@@ -66,7 +66,7 @@ public void resolveMediaTypesNoKey() {
 	}
 
 	@Test
-	public void resolveMediaTypesHandleNoMatch() {
+	public void resolveMediaTypesHandleNoMatch() throws Exception {
 		Map<String, MediaType> mapping = null;
 		TestMappingContentNegotiationStrategy strategy = new TestMappingContentNegotiationStrategy("xml", mapping);
 

diff --git a/.../test/java/org/springframework/web/accept/MappingMediaTypeFileExtensionResolverTests.java b/.../test/java/org/springframework/web/accept/MappingMediaTypeFileExtensionResolverTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2014 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.