Skip to content

Commit 3b95e0b

Browse files
rstoyanchevrstoyanchev
committed
Fix media type regression in resource handling
Issue: SPR-14577
1 parent 417a9d4 commit 3b95e0b

File tree

4 files changed

+50
-49
lines changed

4 files changed

+50
-49
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<>(this.mediaTypes.values());
7276
}

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

Lines changed: 22 additions & 28 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<>(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.
@@ -511,28 +526,7 @@ protected boolean isInvalidPath(String path) {
511526
* @return the corresponding media type, or {@code null} if none found
512527
*/
513528
protected MediaType getMediaType(HttpServletRequest request, Resource resource) {
514-
MediaType mediaType = null;
515-
516-
Class<PathExtensionContentNegotiationStrategy> clazz = PathExtensionContentNegotiationStrategy.class;
517-
PathExtensionContentNegotiationStrategy strategy = this.contentNegotiationManager.getStrategy(clazz);
518-
if (strategy != null) {
519-
mediaType = strategy.getMediaTypeForResource(resource);
520-
}
521-
522-
if (mediaType == null) {
523-
ServletWebRequest webRequest = new ServletWebRequest(request);
524-
try {
525-
List<MediaType> mediaTypes = getContentNegotiationManager().resolveMediaTypes(webRequest);
526-
if (!mediaTypes.isEmpty()) {
527-
mediaType = mediaTypes.get(0);
528-
}
529-
}
530-
catch (HttpMediaTypeNotAcceptableException ex) {
531-
// Ignore
532-
}
533-
}
534-
535-
return mediaType;
529+
return this.pathExtensionStrategy.getMediaTypeForResource(resource);
536530
}
537531

538532
/**

‎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)