Skip to content

Commit fe40462

Browse files
rstoyanchevrstoyanchev
committed
Fix media type regression in resource handling
Issue: SPR-14577
1 parent 5a004c3 commit fe40462

File tree

4 files changed

+50
-53
lines changed

4 files changed

+50
-53
lines changed

‎spring-web/src/main/java/org/springframework/web/accept/MappingMediaTypeFileExtensionResolver.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,10 @@ public MappingMediaTypeFileExtensionResolver(Map<String, MediaType> mediaTypes)
6767
}
6868

6969

70+
public Map<String, MediaType> getMediaTypes() {
71+
return this.mediaTypes;
72+
}
73+
7074
protected List<MediaType> getAllMediaTypes() {
7175
return new ArrayList<MediaType>(this.mediaTypes.values());
7276
}

‎spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java‎

Lines changed: 22 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@
1919
import java.io.IOException;
2020
import java.net.URLDecoder;
2121
import java.util.ArrayList;
22+
import java.util.HashMap;
2223
import java.util.List;
24+
import java.util.Map;
2325
import javax.servlet.ServletContext;
2426
import javax.servlet.ServletException;
2527
import javax.servlet.ServletResponse;
@@ -51,6 +53,7 @@
5153
import org.springframework.web.accept.ContentNegotiationManager;
5254
import org.springframework.web.accept.ContentNegotiationManagerFactoryBean;
5355
import org.springframework.web.accept.PathExtensionContentNegotiationStrategy;
56+
import org.springframework.web.accept.ServletPathExtensionContentNegotiationStrategy;
5457
import org.springframework.web.context.request.ServletWebRequest;
5558
import org.springframework.web.cors.CorsConfiguration;
5659
import org.springframework.web.cors.CorsConfigurationSource;
@@ -111,7 +114,9 @@ public class ResourceHttpRequestHandler extends WebContentGenerator
111114

112115
private ContentNegotiationManager contentNegotiationManager;
113116

114-
private final ContentNegotiationManagerFactoryBean cnmFactoryBean = new ContentNegotiationManagerFactoryBean();
117+
private ServletPathExtensionContentNegotiationStrategy pathExtensionStrategy;
118+
119+
private ServletContext servletContext;
115120

116121
private CorsConfiguration corsConfiguration;
117122

@@ -254,7 +259,7 @@ public CorsConfiguration getCorsConfiguration(HttpServletRequest request) {
254259

255260
@Override
256261
protected void initServletContext(ServletContext servletContext) {
257-
this.cnmFactoryBean.setServletContext(servletContext);
262+
this.servletContext = servletContext;
258263
}
259264

260265

@@ -268,16 +273,13 @@ public void afterPropertiesSet() throws Exception {
268273
this.resourceResolvers.add(new PathResourceResolver());
269274
}
270275
initAllowedLocations();
271-
if (this.contentNegotiationManager == null) {
272-
this.cnmFactoryBean.afterPropertiesSet();
273-
this.contentNegotiationManager = this.cnmFactoryBean.getObject();
274-
}
275276
if (this.resourceHttpMessageConverter == null) {
276277
this.resourceHttpMessageConverter = new ResourceHttpMessageConverter();
277278
}
278279
if (this.resourceRegionHttpMessageConverter == null) {
279280
this.resourceRegionHttpMessageConverter = new ResourceRegionHttpMessageConverter();
280281
}
282+
this.pathExtensionStrategy = initPathExtensionStrategy();
281283
}
282284

283285
/**
@@ -300,6 +302,19 @@ protected void initAllowedLocations() {
300302
}
301303
}
302304

305+
protected ServletPathExtensionContentNegotiationStrategy initPathExtensionStrategy() {
306+
Map<String, MediaType> mediaTypes = null;
307+
if (getContentNegotiationManager() != null) {
308+
PathExtensionContentNegotiationStrategy strategy =
309+
getContentNegotiationManager().getStrategy(PathExtensionContentNegotiationStrategy.class);
310+
if (strategy != null) {
311+
mediaTypes = new HashMap<String, MediaType>(strategy.getMediaTypes());
312+
}
313+
}
314+
return new ServletPathExtensionContentNegotiationStrategy(this.servletContext, mediaTypes);
315+
}
316+
317+
303318
/**
304319
* Processes a resource request.
305320
* <p>Checks for the existence of the requested resource in the configured list of locations.
@@ -512,32 +527,7 @@ protected boolean isInvalidPath(String path) {
512527
*/
513528
@SuppressWarnings("deprecation")
514529
protected MediaType getMediaType(HttpServletRequest request, Resource resource) {
515-
// For backwards compatibility
516-
MediaType mediaType = getMediaType(resource);
517-
if (mediaType != null) {
518-
return mediaType;
519-
}
520-
521-
Class<PathExtensionContentNegotiationStrategy> clazz = PathExtensionContentNegotiationStrategy.class;
522-
PathExtensionContentNegotiationStrategy strategy = this.contentNegotiationManager.getStrategy(clazz);
523-
if (strategy != null) {
524-
mediaType = strategy.getMediaTypeForResource(resource);
525-
}
526-
527-
if (mediaType == null) {
528-
ServletWebRequest webRequest = new ServletWebRequest(request);
529-
try {
530-
List<MediaType> mediaTypes = getContentNegotiationManager().resolveMediaTypes(webRequest);
531-
if (!mediaTypes.isEmpty()) {
532-
mediaType = mediaTypes.get(0);
533-
}
534-
}
535-
catch (HttpMediaTypeNotAcceptableException ex) {
536-
// Ignore
537-
}
538-
}
539-
540-
return mediaType;
530+
return this.pathExtensionStrategy.getMediaTypeForResource(resource);
541531
}
542532

543533
/**

‎spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java‎

Lines changed: 23 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -248,37 +248,38 @@ public void getResourceWithRegisteredMediaType() throws Exception {
248248
ContentNegotiationManager manager = factory.getObject();
249249

250250
List<Resource> paths = Collections.singletonList(new ClassPathResource("test/", getClass()));
251-
this.handler = new ResourceHttpRequestHandler();
252-
this.handler.setLocations(paths);
253-
this.handler.setContentNegotiationManager(manager);
254-
this.handler.afterPropertiesSet();
251+
ResourceHttpRequestHandler handler = new ResourceHttpRequestHandler();
252+
handler.setServletContext(new MockServletContext());
253+
handler.setLocations(paths);
254+
handler.setContentNegotiationManager(manager);
255+
handler.afterPropertiesSet();
255256

256257
this.request.setAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, "foo.css");
257-
this.handler.handleRequest(this.request, this.response);
258+
handler.handleRequest(this.request, this.response);
258259

259260
assertEquals("foo/bar", this.response.getContentType());
260261
assertEquals("h1 { color:red; }", this.response.getContentAsString());
261262
}
262263

263-
@Test // SPR-13658
264-
public void getResourceWithRegisteredMediaTypeDefaultStrategy() throws Exception {
264+
@Test // SPR-14577
265+
public void getMediaTypeWithFavorPathExtensionOff() throws Exception {
265266
ContentNegotiationManagerFactoryBean factory = new ContentNegotiationManagerFactoryBean();
266267
factory.setFavorPathExtension(false);
267-
factory.setDefaultContentType(new MediaType("foo", "bar"));
268268
factory.afterPropertiesSet();
269269
ContentNegotiationManager manager = factory.getObject();
270270

271271
List<Resource> paths = Collections.singletonList(new ClassPathResource("test/", getClass()));
272-
this.handler = new ResourceHttpRequestHandler();
273-
this.handler.setLocations(paths);
274-
this.handler.setContentNegotiationManager(manager);
275-
this.handler.afterPropertiesSet();
272+
ResourceHttpRequestHandler handler = new ResourceHttpRequestHandler();
273+
handler.setServletContext(new MockServletContext());
274+
handler.setLocations(paths);
275+
handler.setContentNegotiationManager(manager);
276+
handler.afterPropertiesSet();
276277

277-
this.request.setAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, "foo.css");
278-
this.handler.handleRequest(this.request, this.response);
278+
this.request.addHeader("Accept", "application/json,text/plain,*/*");
279+
this.request.setAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, "foo.html");
280+
handler.handleRequest(this.request, this.response);
279281

280-
assertEquals("foo/bar", this.response.getContentType());
281-
assertEquals("h1 { color:red; }", this.response.getContentAsString());
282+
assertEquals("text/html", this.response.getContentType());
282283
}
283284

284285
@Test // SPR-14368
@@ -297,13 +298,13 @@ public String getVirtualServerName() {
297298
};
298299

299300
List<Resource> paths = Collections.singletonList(new ClassPathResource("test/", getClass()));
300-
this.handler = new ResourceHttpRequestHandler();
301-
this.handler.setServletContext(servletContext);
302-
this.handler.setLocations(paths);
303-
this.handler.afterPropertiesSet();
301+
ResourceHttpRequestHandler handler = new ResourceHttpRequestHandler();
302+
handler.setServletContext(servletContext);
303+
handler.setLocations(paths);
304+
handler.afterPropertiesSet();
304305

305306
this.request.setAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, "foo.css");
306-
this.handler.handleRequest(this.request, this.response);
307+
handler.handleRequest(this.request, this.response);
307308

308309
assertEquals("foo/bar", this.response.getContentType());
309310
assertEquals("h1 { color:red; }", this.response.getContentAsString());
@@ -412,6 +413,7 @@ public void initAllowedLocationsWithExplicitConfiguration() throws Exception {
412413

413414
ResourceHttpRequestHandler handler = new ResourceHttpRequestHandler();
414415
handler.setResourceResolvers(Collections.singletonList(pathResolver));
416+
handler.setServletContext(new MockServletContext());
415417
handler.setLocations(Arrays.asList(location1, location2));
416418
handler.afterPropertiesSet();
417419

‎spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceUrlProviderTests.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ public class ResourceUrlProviderTests {
5858
public void setUp() throws Exception {
5959
this.locations.add(new ClassPathResource("test/", getClass()));
6060
this.locations.add(new ClassPathResource("testalternatepath/", getClass()));
61+
this.handler.setServletContext(new MockServletContext());
6162
this.handler.setLocations(locations);
6263
this.handler.afterPropertiesSet();
6364
this.handlerMap.put("/resources/**", this.handler);

0 commit comments

Comments
 (0)