Skip to content

Commit c99cc53

Browse files
committed
Fix directories I/O in ResourceHttpRequestHandler
Prior to this commit, the `ResourceHttpRequestHandler` would not properly handle HTTP requests to **directories contained in JARs**. This would result in HTTP 500 errors, caused by `FileNotFoundException` or `NullPointerException`. This can be tracked to webapp ClassLoader implementations in servlet containers: * in Jetty9x, fetching a directory within a JAR as a `Resource` and getting its InputStream work fine, but attempting to `close()` it results in a NullPointerException as the underlying stream is null. * In Tomcat6x, one cannot fetch an InputStream for the same `Resource` as it throws a FileNotFoundException. This change adds more try/catch clauses and catches more Exception so as to result in HTTP 200 OK responses instead of server errors. While this is inconsistent because the same code path would result in HTTP 404 with existing directories on the file system, there's no other simple way to make those checks for resources contained in JARs. Issue: SPR-12999
1 parent cd9b58b commit c99cc53

File tree

2 files changed

+57
-9
lines changed

2 files changed

+57
-9
lines changed

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

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package org.springframework.web.servlet.resource;
1818

19+
import java.io.FileNotFoundException;
1920
import java.io.IOException;
2021
import java.io.InputStream;
2122
import java.net.URLDecoder;
@@ -404,17 +405,23 @@ protected void setHeaders(HttpServletResponse response, Resource resource, Media
404405
* @throws IOException in case of errors while writing the content
405406
*/
406407
protected void writeContent(HttpServletResponse response, Resource resource) throws IOException {
407-
InputStream in = resource.getInputStream();
408408
try {
409-
StreamUtils.copy(in, response.getOutputStream());
410-
}
411-
finally {
409+
InputStream in = resource.getInputStream();
412410
try {
413-
in.close();
411+
StreamUtils.copy(in, response.getOutputStream());
414412
}
415-
catch (IOException ex) {
413+
finally {
414+
try {
415+
in.close();
416+
}
417+
catch (Throwable ex) {
418+
// ignore, see SPR-12999
419+
}
416420
}
417421
}
422+
catch (FileNotFoundException ex) {
423+
// ignore, see SPR-12999
424+
}
418425
}
419426

420427
@Override

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

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2014 the original author or authors.
2+
* Copyright 2002-2015 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,7 +16,13 @@
1616

1717
package org.springframework.web.servlet.resource;
1818

19+
import static org.junit.Assert.*;
20+
import static org.mockito.BDDMockito.given;
21+
import static org.mockito.Mockito.*;
22+
23+
import java.io.FileNotFoundException;
1924
import java.io.IOException;
25+
import java.io.InputStream;
2026
import java.util.ArrayList;
2127
import java.util.Arrays;
2228
import java.util.List;
@@ -34,7 +40,6 @@
3440
import org.springframework.web.HttpRequestMethodNotSupportedException;
3541
import org.springframework.web.servlet.HandlerMapping;
3642

37-
import static org.junit.Assert.*;
3843

3944
/**
4045
* Unit tests for ResourceHttpRequestHandler.
@@ -57,6 +62,7 @@ public void setUp() throws Exception {
5762
List<Resource> paths = new ArrayList<>(2);
5863
paths.add(new ClassPathResource("test/", getClass()));
5964
paths.add(new ClassPathResource("testalternatepath/", getClass()));
65+
paths.add(new ClassPathResource("META-INF/resources/webjars/"));
6066

6167
this.handler = new ResourceHttpRequestHandler();
6268
this.handler.setLocations(paths);
@@ -204,9 +210,10 @@ public void initAllowedLocations() throws Exception {
204210
PathResourceResolver resolver = (PathResourceResolver) this.handler.getResourceResolvers().get(0);
205211
Resource[] locations = resolver.getAllowedLocations();
206212

207-
assertEquals(2, locations.length);
213+
assertEquals(3, locations.length);
208214
assertEquals("test/", ((ClassPathResource) locations[0]).getPath());
209215
assertEquals("testalternatepath/", ((ClassPathResource) locations[1]).getPath());
216+
assertEquals("META-INF/resources/webjars/", ((ClassPathResource) locations[2]).getPath());
210217
}
211218

212219
@Test
@@ -251,6 +258,14 @@ public void directory() throws Exception {
251258
assertEquals(404, this.response.getStatus());
252259
}
253260

261+
@Test
262+
public void directoryInJarFile() throws Exception {
263+
this.request.setAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, "underscorejs/");
264+
this.handler.handleRequest(this.request, this.response);
265+
assertEquals(200, this.response.getStatus());
266+
assertEquals(0, this.response.getContentLength());
267+
}
268+
254269
@Test
255270
public void missingResourcePath() throws Exception {
256271
this.request.setAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, "");
@@ -277,6 +292,32 @@ public void resourceNotFound() throws Exception {
277292
assertEquals(404, this.response.getStatus());
278293
}
279294

295+
// SPR-12999
296+
@Test
297+
public void writeContentNotGettingInputStream() throws Exception {
298+
Resource resource = mock(Resource.class);
299+
given(resource.getInputStream()).willThrow(FileNotFoundException.class);
300+
301+
this.handler.writeContent(this.response, resource);
302+
303+
assertEquals(200, this.response.getStatus());
304+
assertEquals(0, this.response.getContentLength());
305+
}
306+
307+
// SPR-12999
308+
@Test
309+
public void writeContentNotClosingInputStream() throws Exception {
310+
Resource resource = mock(Resource.class);
311+
InputStream inputStream = mock(InputStream.class);
312+
given(resource.getInputStream()).willReturn(inputStream);
313+
given(inputStream.read(any())).willReturn(-1);
314+
doThrow(new NullPointerException()).when(inputStream).close();
315+
316+
this.handler.writeContent(this.response, resource);
317+
318+
assertEquals(200, this.response.getStatus());
319+
assertEquals(0, this.response.getContentLength());
320+
}
280321

281322
private long headerAsLong(String responseHeaderName) {
282323
return Long.valueOf(this.response.getHeader(responseHeaderName));

0 commit comments

Comments
 (0)