Set favorPathExtension to false by default

Applies a change that was intended in #23915 but wasn't. Closes gh-26119
spring-projects · Nov 24, 2020 · 8070108 · 8070108
1 parent 3612990
commit 8070108
Show file tree

Hide file tree

Showing 8 changed files with 64 additions and 33 deletions.
diff --git a/...eb/src/main/java/org/springframework/web/accept/ContentNegotiationManagerFactoryBean.java b/...eb/src/main/java/org/springframework/web/accept/ContentNegotiationManagerFactoryBean.java
@@ -108,7 +108,7 @@ public class ContentNegotiationManagerFactoryBean
 
 	private String parameterName = "format";
 
-	private boolean favorPathExtension = true;
+	private boolean favorPathExtension = false;
 
 	private Map<String, MediaType> mediaTypes = new HashMap<>();
 

diff --git a/...c/test/java/org/springframework/web/accept/ContentNegotiationManagerFactoryBeanTests.java b/...c/test/java/org/springframework/web/accept/ContentNegotiationManagerFactoryBeanTests.java
@@ -72,8 +72,8 @@ void defaultSettings() throws Exception {
 		this.servletRequest.setRequestURI("/flower.gif");
 
 		assertThat(manager.resolveMediaTypes(this.webRequest))
-				.as("Should be able to resolve file extensions by default")
-				.isEqualTo(Collections.singletonList(MediaType.IMAGE_GIF));
+				.as("Should not resolve file extensions by default")
+				.containsExactly(MediaType.ALL);
 
 		this.servletRequest.setRequestURI("/flower.foobarbaz");
 
@@ -226,6 +226,7 @@ void fileExtensions() {
 	@Test
 	void ignoreAcceptHeader() throws Exception {
 		this.factoryBean.setIgnoreAcceptHeader(true);
+		this.factoryBean.setFavorParameter(true);
 		this.factoryBean.afterPropertiesSet();
 		ContentNegotiationManager manager = this.factoryBean.getObject();
 

diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/config/MvcNamespaceTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/config/MvcNamespaceTests.java
@@ -206,7 +206,9 @@ public void testDefaultConfig() throws Exception {
 		MockHttpServletRequest request = new MockHttpServletRequest("GET", "/foo.json");
 		NativeWebRequest webRequest = new ServletWebRequest(request);
 		ContentNegotiationManager manager = mapping.getContentNegotiationManager();
-		assertThat(manager.resolveMediaTypes(webRequest)).isEqualTo(Collections.singletonList(MediaType.APPLICATION_JSON));
+		assertThat(manager.resolveMediaTypes(webRequest))
+				.as("Should not resolve file extensions by default")
+				.containsExactly(MediaType.ALL);
 
 		RequestMappingHandlerAdapter adapter = appContext.getBean(RequestMappingHandlerAdapter.class);
 		assertThat(adapter).isNotNull();
@@ -743,13 +745,17 @@ public void testContentNegotiationManager() throws Exception {
 		RequestMappingHandlerMapping mapping = appContext.getBean(RequestMappingHandlerMapping.class);
 		ContentNegotiationManager manager = mapping.getContentNegotiationManager();
 
-		MockHttpServletRequest request = new MockHttpServletRequest("GET", "/foo.xml");
+		MockHttpServletRequest request = new MockHttpServletRequest("GET", "/foo");
+		request.setParameter("format", "xml");
 		NativeWebRequest webRequest = new ServletWebRequest(request);
-		assertThat(manager.resolveMediaTypes(webRequest)).isEqualTo(Collections.singletonList(MediaType.valueOf("application/rss+xml")));
+		assertThat(manager.resolveMediaTypes(webRequest))
+				.containsExactly(MediaType.valueOf("application/rss+xml"));
 
 		ViewResolverComposite compositeResolver = this.appContext.getBean(ViewResolverComposite.class);
 		assertThat(compositeResolver).isNotNull();
-		assertThat(compositeResolver.getViewResolvers().size()).as("Actual: " + compositeResolver.getViewResolvers()).isEqualTo(1);
+		assertThat(compositeResolver.getViewResolvers().size())
+				.as("Actual: " + compositeResolver.getViewResolvers())
+				.isEqualTo(1);
 
 		ViewResolver resolver = compositeResolver.getViewResolvers().get(0);
 		assertThat(resolver.getClass()).isEqualTo(ContentNegotiatingViewResolver.class);

diff --git a/.../org/springframework/web/servlet/config/annotation/ContentNegotiationConfigurerTests.java b/.../org/springframework/web/servlet/config/annotation/ContentNegotiationConfigurerTests.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.
@@ -59,26 +59,34 @@ public void defaultSettings() throws Exception {
 
 		this.servletRequest.setRequestURI("/flower.gif");
 
-		assertThat(manager.resolveMediaTypes(this.webRequest).get(0)).as("Should be able to resolve file extensions by default").isEqualTo(MediaType.IMAGE_GIF);
+		assertThat(manager.resolveMediaTypes(this.webRequest))
+				.as("Should not resolve file extensions by default")
+				.containsExactly(MediaType.ALL);
 
 		this.servletRequest.setRequestURI("/flower?format=gif");
 		this.servletRequest.addParameter("format", "gif");
 
-		assertThat(manager.resolveMediaTypes(this.webRequest)).as("Should not resolve request parameters by default").isEqualTo(ContentNegotiationStrategy.MEDIA_TYPE_ALL_LIST);
+		assertThat(manager.resolveMediaTypes(this.webRequest))
+				.as("Should not resolve request parameters by default")
+				.isEqualTo(ContentNegotiationStrategy.MEDIA_TYPE_ALL_LIST);
 
 		this.servletRequest.setRequestURI("/flower");
 		this.servletRequest.addHeader("Accept", MediaType.IMAGE_GIF_VALUE);
 
-		assertThat(manager.resolveMediaTypes(this.webRequest).get(0)).as("Should resolve Accept header by default").isEqualTo(MediaType.IMAGE_GIF);
+		assertThat(manager.resolveMediaTypes(this.webRequest))
+				.as("Should resolve Accept header by default")
+				.containsExactly(MediaType.IMAGE_GIF);
 	}
 
 	@Test
 	public void addMediaTypes() throws Exception {
+		this.configurer.favorParameter(true);
 		this.configurer.mediaTypes(Collections.singletonMap("json", MediaType.APPLICATION_JSON));
 		ContentNegotiationManager manager = this.configurer.buildContentNegotiationManager();
 
-		this.servletRequest.setRequestURI("/flower.json");
-		assertThat(manager.resolveMediaTypes(this.webRequest).get(0)).isEqualTo(MediaType.APPLICATION_JSON);
+		this.servletRequest.setRequestURI("/flower");
+		this.servletRequest.addParameter("format", "json");
+		assertThat(manager.resolveMediaTypes(this.webRequest)).containsExactly(MediaType.APPLICATION_JSON);
 	}
 
 	@Test
@@ -97,6 +105,7 @@ public void favorParameter() throws Exception {
 	@Test
 	public void ignoreAcceptHeader() throws Exception {
 		this.configurer.ignoreAcceptHeader(true);
+		this.configurer.favorParameter(true);
 		ContentNegotiationManager manager = this.configurer.buildContentNegotiationManager();
 
 		this.servletRequest.setRequestURI("/flower");

diff --git a/...ringframework/web/servlet/config/annotation/WebMvcConfigurationSupportExtensionTests.java b/...ringframework/web/servlet/config/annotation/WebMvcConfigurationSupportExtensionTests.java
@@ -33,7 +33,6 @@
 import org.springframework.core.io.FileSystemResourceLoader;
 import org.springframework.format.FormatterRegistry;
 import org.springframework.http.HttpStatus;
-import org.springframework.http.MediaType;
 import org.springframework.http.converter.HttpMessageConverter;
 import org.springframework.http.converter.StringHttpMessageConverter;
 import org.springframework.http.converter.json.MappingJackson2HttpMessageConverter;
@@ -91,7 +90,6 @@
 import static com.fasterxml.jackson.databind.MapperFeature.DEFAULT_VIEW_INCLUSION;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.mock;
-import static org.springframework.http.MediaType.APPLICATION_ATOM_XML;
 import static org.springframework.http.MediaType.APPLICATION_JSON;
 import static org.springframework.http.MediaType.APPLICATION_XML;
 
@@ -268,34 +266,28 @@ public void webBindingInitializer() throws Exception {
 	@Test
 	@SuppressWarnings("deprecation")
 	public void contentNegotiation() throws Exception {
-		MockHttpServletRequest request = new MockHttpServletRequest("GET", "/foo.json");
+		MockHttpServletRequest request = new MockHttpServletRequest("GET", "/foo");
 		NativeWebRequest webRequest = new ServletWebRequest(request);
 
 		RequestMappingHandlerMapping mapping = this.config.requestMappingHandlerMapping(
 				this.config.mvcContentNegotiationManager(), this.config.mvcConversionService(),
 				this.config.mvcResourceUrlProvider());
+
+		request.setParameter("f", "json");
 		ContentNegotiationManager manager = mapping.getContentNegotiationManager();
 		assertThat(manager.resolveMediaTypes(webRequest)).isEqualTo(Collections.singletonList(APPLICATION_JSON));
 
-		request.setRequestURI("/foo.xml");
+		request.setParameter("f", "xml");
 		assertThat(manager.resolveMediaTypes(webRequest)).isEqualTo(Collections.singletonList(APPLICATION_XML));
 
-		request.setRequestURI("/foo.rss");
-		assertThat(manager.resolveMediaTypes(webRequest)).isEqualTo(Collections.singletonList(MediaType.valueOf("application/rss+xml")));
-
-		request.setRequestURI("/foo.atom");
-		assertThat(manager.resolveMediaTypes(webRequest)).isEqualTo(Collections.singletonList(APPLICATION_ATOM_XML));
-
-		request.setRequestURI("/foo");
-		request.setParameter("f", "json");
-		assertThat(manager.resolveMediaTypes(webRequest)).isEqualTo(Collections.singletonList(APPLICATION_JSON));
-
-		request.setRequestURI("/resources/foo.gif");
 		SimpleUrlHandlerMapping handlerMapping = (SimpleUrlHandlerMapping) this.config.resourceHandlerMapping(
 				this.config.mvcContentNegotiationManager(), this.config.mvcConversionService(),
 				this.config.mvcResourceUrlProvider());
 		handlerMapping.setApplicationContext(this.context);
+
+		request = new MockHttpServletRequest("GET", "/resources/foo.gif");
 		HandlerExecutionChain chain = handlerMapping.getHandler(request);
+
 		assertThat(chain).isNotNull();
 		ResourceHttpRequestHandler handler = (ResourceHttpRequestHandler) chain.getHandler();
 		assertThat(handler).isNotNull();

diff --git a/...work/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java b/...work/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java
@@ -1742,6 +1742,7 @@ void responseBodyAsHtml(boolean usePathPatterns) throws Exception {
 			}
 
 			ContentNegotiationManagerFactoryBean factoryBean = new ContentNegotiationManagerFactoryBean();
+			factoryBean.setFavorPathExtension(true);
 			factoryBean.afterPropertiesSet();
 
 			RootBeanDefinition adapterDef = new RootBeanDefinition(RequestMappingHandlerAdapter.class);
@@ -1773,6 +1774,7 @@ void responseBodyAsHtml(boolean usePathPatterns) throws Exception {
 	void responseBodyAsHtmlWithSuffixPresent(boolean usePathPatterns) throws Exception {
 		initDispatcherServlet(TextRestController.class, usePathPatterns, wac -> {
 			ContentNegotiationManagerFactoryBean factoryBean = new ContentNegotiationManagerFactoryBean();
+			factoryBean.setFavorPathExtension(true);
 			factoryBean.afterPropertiesSet();
 			RootBeanDefinition adapterDef = new RootBeanDefinition(RequestMappingHandlerAdapter.class);
 			adapterDef.getPropertyValues().add("contentNegotiationManager", factoryBean.getObject());
@@ -1833,14 +1835,22 @@ void responseBodyAsHtmlWithProducesCondition(boolean usePathPatterns) throws Exc
 	void responseBodyAsTextWithCssExtension(boolean usePathPatterns) throws Exception {
 		initDispatcherServlet(TextRestController.class, usePathPatterns, wac -> {
 			ContentNegotiationManagerFactoryBean factoryBean = new ContentNegotiationManagerFactoryBean();
+			factoryBean.setFavorParameter(true);
+			factoryBean.addMediaType("css", MediaType.parseMediaType("text/css"));
 			factoryBean.afterPropertiesSet();
+
+			RootBeanDefinition mappingDef = new RootBeanDefinition(RequestMappingHandlerMapping.class);
+			mappingDef.getPropertyValues().add("contentNegotiationManager", factoryBean.getObject());
+			wac.registerBeanDefinition("handlerMapping", mappingDef);
+
 			RootBeanDefinition adapterDef = new RootBeanDefinition(RequestMappingHandlerAdapter.class);
 			adapterDef.getPropertyValues().add("contentNegotiationManager", factoryBean.getObject());
 			wac.registerBeanDefinition("handlerAdapter", adapterDef);
 		});
 
 		byte[] content = "body".getBytes(StandardCharsets.ISO_8859_1);
-		MockHttpServletRequest request = new MockHttpServletRequest("GET", "/a4.css");
+		MockHttpServletRequest request = new MockHttpServletRequest("GET", "/a4");
+		request.addParameter("format", "css");
 		request.setContent(content);
 		MockHttpServletResponse response = new MockHttpServletResponse();
 
@@ -3826,7 +3836,7 @@ public String a3(@RequestBody String body) throws IOException {
 			return body;
 		}
 
-		@RequestMapping(path = "/a4.css", method = RequestMethod.GET)
+		@RequestMapping(path = "/a4", method = RequestMethod.GET)
 		public String a4(@RequestBody String body) {
 			return body;
 		}

diff --git a/...c/test/java/org/springframework/web/servlet/view/ContentNegotiatingViewResolverTests.java b/...c/test/java/org/springframework/web/servlet/view/ContentNegotiatingViewResolverTests.java
@@ -34,6 +34,7 @@
 import org.springframework.web.accept.HeaderContentNegotiationStrategy;
 import org.springframework.web.accept.MappingMediaTypeFileExtensionResolver;
 import org.springframework.web.accept.ParameterContentNegotiationStrategy;
+import org.springframework.web.accept.PathExtensionContentNegotiationStrategy;
 import org.springframework.web.context.request.RequestContextHolder;
 import org.springframework.web.context.request.ServletRequestAttributes;
 import org.springframework.web.context.support.StaticWebApplicationContext;
@@ -85,9 +86,16 @@ public void getMediaTypeAcceptHeaderWithProduces() throws Exception {
 
 	@Test
 	public void resolveViewNameWithPathExtension() throws Exception {
-		request.setRequestURI("/test.xls");
+		request.setRequestURI("/test");
+		request.setParameter("format", "xls");
+
+		String mediaType = "application/vnd.ms-excel";
+		ContentNegotiationManager manager = new ContentNegotiationManager(
+				new ParameterContentNegotiationStrategy(
+						Collections.singletonMap("xls", MediaType.parseMediaType(mediaType))));
 
 		ViewResolver viewResolverMock = mock(ViewResolver.class);
+		viewResolver.setContentNegotiationManager(manager);
 		viewResolver.setViewResolvers(Collections.singletonList(viewResolverMock));
 		viewResolver.afterPropertiesSet();
 
@@ -98,7 +106,7 @@ public void resolveViewNameWithPathExtension() throws Exception {
 
 		given(viewResolverMock.resolveViewName(viewName, locale)).willReturn(null);
 		given(viewResolverMock.resolveViewName(viewName + ".xls", locale)).willReturn(viewMock);
-		given(viewMock.getContentType()).willReturn("application/vnd.ms-excel");
+		given(viewMock.getContentType()).willReturn(mediaType);
 
 		View result = viewResolver.resolveViewName(viewName, locale);
 		assertThat(result).as("Invalid view").isSameAs(viewMock);
@@ -307,8 +315,12 @@ public void resolveViewNameAcceptHeaderDefaultView() throws Exception {
 	public void resolveViewNameFilename() throws Exception {
 		request.setRequestURI("/test.html");
 
+		ContentNegotiationManager manager =
+				new ContentNegotiationManager(new PathExtensionContentNegotiationStrategy());
+
 		ViewResolver viewResolverMock1 = mock(ViewResolver.class, "viewResolver1");
 		ViewResolver viewResolverMock2 = mock(ViewResolver.class, "viewResolver2");
+		viewResolver.setContentNegotiationManager(manager);
 		viewResolver.setViewResolvers(Arrays.asList(viewResolverMock1, viewResolverMock2));
 
 		viewResolver.afterPropertiesSet();
@@ -336,7 +348,7 @@ public void resolveViewNameFilenameDefaultView() throws Exception {
 		request.setRequestURI("/test.json");
 
 		Map<String, MediaType> mapping = Collections.singletonMap("json", MediaType.APPLICATION_JSON);
-		org.springframework.web.accept.PathExtensionContentNegotiationStrategy pathStrategy = new org.springframework.web.accept.PathExtensionContentNegotiationStrategy(mapping);
+		PathExtensionContentNegotiationStrategy pathStrategy = new PathExtensionContentNegotiationStrategy(mapping);
 		viewResolver.setContentNegotiationManager(new ContentNegotiationManager(pathStrategy));
 
 		ViewResolver viewResolverMock1 = mock(ViewResolver.class);

diff --git a/...sources/org/springframework/web/servlet/config/mvc-config-content-negotiation-manager.xml b/...sources/org/springframework/web/servlet/config/mvc-config-content-negotiation-manager.xml
@@ -14,6 +14,7 @@
 	</mvc:view-resolvers>
 
 	<bean id="mvcContentNegotiationManager" class="org.springframework.web.accept.ContentNegotiationManagerFactoryBean">
+		<property name="favorParameter" value="true"/>
 		<property name="mediaTypes">
 			<value>
 				xml=application/rss+xml