Set favorPathExtension to false by default
Applies a change that was intended in #23915 but wasn't. Closes gh-26119
This commit is contained in:
		
							parent
							
								
									3612990344
								
							
						
					
					
						commit
						80701082cd
					
				| 
						 | 
				
			
			@ -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<>();
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -72,8 +72,8 @@ class ContentNegotiationManagerFactoryBeanTests {
 | 
			
		|||
		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 @@ class ContentNegotiationManagerFactoryBeanTests {
 | 
			
		|||
	@Test
 | 
			
		||||
	void ignoreAcceptHeader() throws Exception {
 | 
			
		||||
		this.factoryBean.setIgnoreAcceptHeader(true);
 | 
			
		||||
		this.factoryBean.setFavorParameter(true);
 | 
			
		||||
		this.factoryBean.afterPropertiesSet();
 | 
			
		||||
		ContentNegotiationManager manager = this.factoryBean.getObject();
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -206,7 +206,9 @@ public class MvcNamespaceTests {
 | 
			
		|||
		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 class MvcNamespaceTests {
 | 
			
		|||
		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);
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -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 class ContentNegotiationConfigurerTests {
 | 
			
		|||
 | 
			
		||||
		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 class ContentNegotiationConfigurerTests {
 | 
			
		|||
	@Test
 | 
			
		||||
	public void ignoreAcceptHeader() throws Exception {
 | 
			
		||||
		this.configurer.ignoreAcceptHeader(true);
 | 
			
		||||
		this.configurer.favorParameter(true);
 | 
			
		||||
		ContentNegotiationManager manager = this.configurer.buildContentNegotiationManager();
 | 
			
		||||
 | 
			
		||||
		this.servletRequest.setRequestURI("/flower");
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -33,7 +33,6 @@ import org.springframework.core.convert.converter.Converter;
 | 
			
		|||
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.DeserializationFeature.FAIL_ON_UNKN
 | 
			
		|||
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 class WebMvcConfigurationSupportExtensionTests {
 | 
			
		|||
	@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();
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -1742,6 +1742,7 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl
 | 
			
		|||
			}
 | 
			
		||||
 | 
			
		||||
			ContentNegotiationManagerFactoryBean factoryBean = new ContentNegotiationManagerFactoryBean();
 | 
			
		||||
			factoryBean.setFavorPathExtension(true);
 | 
			
		||||
			factoryBean.afterPropertiesSet();
 | 
			
		||||
 | 
			
		||||
			RootBeanDefinition adapterDef = new RootBeanDefinition(RequestMappingHandlerAdapter.class);
 | 
			
		||||
| 
						 | 
				
			
			@ -1773,6 +1774,7 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl
 | 
			
		|||
	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 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl
 | 
			
		|||
	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 class ServletAnnotationControllerHandlerMethodTests extends AbstractServl
 | 
			
		|||
			return body;
 | 
			
		||||
		}
 | 
			
		||||
 | 
			
		||||
		@RequestMapping(path = "/a4.css", method = RequestMethod.GET)
 | 
			
		||||
		@RequestMapping(path = "/a4", method = RequestMethod.GET)
 | 
			
		||||
		public String a4(@RequestBody String body) {
 | 
			
		||||
			return body;
 | 
			
		||||
		}
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -34,6 +34,7 @@ import org.springframework.web.accept.FixedContentNegotiationStrategy;
 | 
			
		|||
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 class ContentNegotiatingViewResolverTests {
 | 
			
		|||
 | 
			
		||||
	@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 class ContentNegotiatingViewResolverTests {
 | 
			
		|||
 | 
			
		||||
		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 class ContentNegotiatingViewResolverTests {
 | 
			
		|||
	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 class ContentNegotiatingViewResolverTests {
 | 
			
		|||
		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);
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -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
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
		Loading…
	
		Reference in New Issue