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
This commit is contained in:
Brian Clozel 2015-05-21 14:30:12 +02:00
parent 3d86f15a84
commit 4d5fca596d
2 changed files with 55 additions and 8 deletions

View File

@ -16,6 +16,7 @@
package org.springframework.web.servlet.resource; package org.springframework.web.servlet.resource;
import java.io.FileNotFoundException;
import java.io.IOException; import java.io.IOException;
import java.io.InputStream; import java.io.InputStream;
import java.io.OutputStream; import java.io.OutputStream;
@ -432,6 +433,7 @@ public class ResourceHttpRequestHandler extends WebContentGenerator implements H
* @throws IOException in case of errors while writing the content * @throws IOException in case of errors while writing the content
*/ */
protected void writeContent(HttpServletResponse response, Resource resource) throws IOException { protected void writeContent(HttpServletResponse response, Resource resource) throws IOException {
try {
InputStream in = resource.getInputStream(); InputStream in = resource.getInputStream();
try { try {
StreamUtils.copy(in, response.getOutputStream()); StreamUtils.copy(in, response.getOutputStream());
@ -440,11 +442,15 @@ public class ResourceHttpRequestHandler extends WebContentGenerator implements H
try { try {
in.close(); in.close();
} }
catch (IOException ex) { catch (Throwable ex) {
// ignore // ignore, see SPR-12999
} }
} }
} }
catch (FileNotFoundException ex) {
// ignore, see SPR-12999
}
}
/** /**
* Write parts of the resource as indicated by the request {@code Range} header. * Write parts of the resource as indicated by the request {@code Range} header.

View File

@ -17,8 +17,12 @@
package org.springframework.web.servlet.resource; package org.springframework.web.servlet.resource;
import static org.junit.Assert.*; import static org.junit.Assert.*;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.*;
import java.io.FileNotFoundException;
import java.io.IOException; import java.io.IOException;
import java.io.InputStream;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Arrays; import java.util.Arrays;
import java.util.List; import java.util.List;
@ -60,6 +64,7 @@ public class ResourceHttpRequestHandlerTests {
List<Resource> paths = new ArrayList<>(2); List<Resource> paths = new ArrayList<>(2);
paths.add(new ClassPathResource("test/", getClass())); paths.add(new ClassPathResource("test/", getClass()));
paths.add(new ClassPathResource("testalternatepath/", getClass())); paths.add(new ClassPathResource("testalternatepath/", getClass()));
paths.add(new ClassPathResource("META-INF/resources/webjars/"));
this.handler = new ResourceHttpRequestHandler(); this.handler = new ResourceHttpRequestHandler();
this.handler.setLocations(paths); this.handler.setLocations(paths);
@ -247,9 +252,10 @@ public class ResourceHttpRequestHandlerTests {
PathResourceResolver resolver = (PathResourceResolver) this.handler.getResourceResolvers().get(0); PathResourceResolver resolver = (PathResourceResolver) this.handler.getResourceResolvers().get(0);
Resource[] locations = resolver.getAllowedLocations(); Resource[] locations = resolver.getAllowedLocations();
assertEquals(2, locations.length); assertEquals(3, locations.length);
assertEquals("test/", ((ClassPathResource) locations[0]).getPath()); assertEquals("test/", ((ClassPathResource) locations[0]).getPath());
assertEquals("testalternatepath/", ((ClassPathResource) locations[1]).getPath()); assertEquals("testalternatepath/", ((ClassPathResource) locations[1]).getPath());
assertEquals("META-INF/resources/webjars/", ((ClassPathResource) locations[2]).getPath());
} }
@Test @Test
@ -294,6 +300,14 @@ public class ResourceHttpRequestHandlerTests {
assertEquals(404, this.response.getStatus()); assertEquals(404, this.response.getStatus());
} }
@Test
public void directoryInJarFile() throws Exception {
this.request.setAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, "underscorejs/");
this.handler.handleRequest(this.request, this.response);
assertEquals(200, this.response.getStatus());
assertEquals(0, this.response.getContentLength());
}
@Test @Test
public void missingResourcePath() throws Exception { public void missingResourcePath() throws Exception {
this.request.setAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, ""); this.request.setAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, "");
@ -425,6 +439,33 @@ public class ResourceHttpRequestHandlerTests {
assertEquals("t.", ranges[11]); assertEquals("t.", ranges[11]);
} }
// SPR-12999
@Test
public void writeContentNotGettingInputStream() throws Exception {
Resource resource = mock(Resource.class);
given(resource.getInputStream()).willThrow(FileNotFoundException.class);
this.handler.writeContent(this.response, resource);
assertEquals(200, this.response.getStatus());
assertEquals(0, this.response.getContentLength());
}
// SPR-12999
@Test
public void writeContentNotClosingInputStream() throws Exception {
Resource resource = mock(Resource.class);
InputStream inputStream = mock(InputStream.class);
given(resource.getInputStream()).willReturn(inputStream);
given(inputStream.read(any())).willReturn(-1);
doThrow(new NullPointerException()).when(inputStream).close();
this.handler.writeContent(this.response, resource);
assertEquals(200, this.response.getStatus());
assertEquals(0, this.response.getContentLength());
}
private long headerAsLong(String responseHeaderName) { private long headerAsLong(String responseHeaderName) {
return Long.valueOf(this.response.getHeader(responseHeaderName)); return Long.valueOf(this.response.getHeader(responseHeaderName));