From 57b466fdfc28a6a3baaf0ab4538e7ecc239ee340 Mon Sep 17 00:00:00 2001 From: matthew-pearson Date: Sun, 14 Feb 2016 09:19:04 +0000 Subject: [PATCH] 404 rather than 405 or 200 Check that the path is valid and resolvable before checking that the http method is supported. For invalid or unresolvable paths, always respond with a 404. --- .../resource/ResourceHttpRequestHandler.java | 16 ++-- .../ResourceHttpRequestHandlerTests.java | 90 ++++++++++++++++++- 2 files changed, 94 insertions(+), 12 deletions(-) diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java index 8b7d4c3471f..1362972a2be 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java @@ -226,6 +226,14 @@ public class ResourceHttpRequestHandler extends WebContentGenerator public void handleRequest(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { + // First, check whether a matching resource exists + Resource resource = getResource(request); + if (resource == null) { + logger.trace("No matching resource found - returning 404"); + response.sendError(HttpServletResponse.SC_NOT_FOUND); + return; + } + if (HttpMethod.OPTIONS.matches(request.getMethod())) { response.setHeader("Allow", getAllowHeader()); return; @@ -234,14 +242,6 @@ public class ResourceHttpRequestHandler extends WebContentGenerator // Supported methods and required session checkRequest(request); - // Check whether a matching resource exists - Resource resource = getResource(request); - if (resource == null) { - logger.trace("No matching resource found - returning 404"); - response.sendError(HttpServletResponse.SC_NOT_FOUND); - return; - } - // Header phase if (new ServletWebRequest(request, response).checkNotModified(resource.lastModified())) { logger.trace("Resource not modified - returning 304"); diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java index 818bb5cfcce..5cae355039a 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java @@ -39,6 +39,8 @@ import org.junit.Test; import org.springframework.core.io.ClassPathResource; import org.springframework.core.io.Resource; import org.springframework.core.io.UrlResource; +import org.springframework.http.HttpMethod; +import org.springframework.http.HttpStatus; import org.springframework.mock.web.test.MockHttpServletRequest; import org.springframework.mock.web.test.MockHttpServletResponse; import org.springframework.mock.web.test.MockServletContext; @@ -223,7 +225,47 @@ public class ResourceHttpRequestHandlerTests { } @Test - public void invalidPath() throws Exception { + public void invalidPathForDelete() throws Exception { + invalidPath(HttpMethod.DELETE); + } + + @Test + public void invalidPathForGet() throws Exception { + invalidPath(HttpMethod.GET); + } + + @Test + public void invalidPathForHead() throws Exception { + invalidPath(HttpMethod.HEAD); + } + + @Test + public void invalidPathForOptions() throws Exception { + invalidPath(HttpMethod.OPTIONS); + } + + @Test + public void invalidPathForPost() throws Exception { + invalidPath(HttpMethod.POST); + } + + @Test + public void invalidPathForPut() throws Exception { + invalidPath(HttpMethod.PUT); + } + + @Test + public void invalidPathForPatch() throws Exception { + invalidPath(HttpMethod.PATCH); + } + + @Test + public void invalidPathForTrace() throws Exception { + invalidPath(HttpMethod.TRACE); + } + + private void invalidPath(HttpMethod httpMethod) throws Exception { + this.request.setMethod(httpMethod.name()); Resource location = new ClassPathResource("test/", getClass()); this.handler.setLocations(Arrays.asList(location)); @@ -255,7 +297,7 @@ public class ResourceHttpRequestHandlerTests { if (!location.createRelative(requestPath).exists() && !requestPath.contains(":")) { fail(requestPath + " doesn't actually exist as a relative path"); } - assertEquals(404, this.response.getStatus()); + assertEquals(HttpStatus.NOT_FOUND.value(), this.response.getStatus()); } @Test @@ -376,10 +418,50 @@ public class ResourceHttpRequestHandlerTests { } @Test - public void resourceNotFound() throws Exception { + public void resourceNotFoundForDelete() throws Exception { + resourceNotFound(HttpMethod.DELETE); + } + + @Test + public void resourceNotFoundForGet() throws Exception { + resourceNotFound(HttpMethod.GET); + } + + @Test + public void resourceNotFoundForHead() throws Exception { + resourceNotFound(HttpMethod.HEAD); + } + + @Test + public void resourceNotFoundForOptions() throws Exception { + resourceNotFound(HttpMethod.OPTIONS); + } + + @Test + public void resourceNotFoundForPost() throws Exception { + resourceNotFound(HttpMethod.POST); + } + + @Test + public void resourceNotFoundForPut() throws Exception { + resourceNotFound(HttpMethod.PUT); + } + + @Test + public void resourceNotFoundForPatch() throws Exception { + resourceNotFound(HttpMethod.PATCH); + } + + @Test + public void resourceNotFoundForTrace() throws Exception { + resourceNotFound(HttpMethod.TRACE); + } + + private void resourceNotFound(HttpMethod httpMethod) throws Exception { + this.request.setMethod(httpMethod.name()); this.request.setAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, "not-there.css"); this.handler.handleRequest(this.request, this.response); - assertEquals(404, this.response.getStatus()); + assertEquals(HttpStatus.NOT_FOUND.value(), this.response.getStatus()); } @Test