From 3febec3df6c95651b3a7183bb1f413a8872d53d1 Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Tue, 10 Oct 2017 23:28:03 +0200 Subject: [PATCH] ResourceWebHandler signals error for missing resources Prior to this commit, the `ResourceWebHandler` would itself handle the response with an HTTP 404 in many cases, including a missing static resource. This does not give a chance to `WebExceptionHandler` instances to handle that error and, for example, display an error page. See spring-projects/spring-boot#8625 Issue: SPR-16023 --- .../reactive/resource/ResourceWebHandler.java | 8 +++-- .../resource/ResourceWebHandlerTests.java | 31 ++++++++++++++----- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java index b0f8241f4c4..b9dbb5c4272 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java @@ -50,6 +50,7 @@ import org.springframework.util.ResourceUtils; import org.springframework.util.StringUtils; import org.springframework.web.reactive.HandlerMapping; import org.springframework.web.server.MethodNotAllowedException; +import org.springframework.web.server.ResponseStatusException; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.WebHandler; @@ -86,8 +87,10 @@ public class ResourceWebHandler implements WebHandler, InitializingBean { /** Set of supported HTTP methods */ private static final Set SUPPORTED_METHODS = EnumSet.of(HttpMethod.GET, HttpMethod.HEAD); - private static final Log logger = LogFactory.getLog(ResourceWebHandler.class); + private static final ResponseStatusException NOT_FOUND_EXCEPTION = + new ResponseStatusException(HttpStatus.NOT_FOUND); + private static final Log logger = LogFactory.getLog(ResourceWebHandler.class); private final List locations = new ArrayList<>(4); @@ -245,8 +248,7 @@ public class ResourceWebHandler implements WebHandler, InitializingBean { return getResource(exchange) .switchIfEmpty(Mono.defer(() -> { logger.trace("No matching resource found - returning 404"); - exchange.getResponse().setStatusCode(HttpStatus.NOT_FOUND); - return Mono.empty(); + return Mono.error(NOT_FOUND_EXCEPTION); })) .flatMap(resource -> { try { diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java index 59564124eec..554a9cab6fe 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java @@ -51,11 +51,14 @@ import org.springframework.mock.web.test.server.MockServerWebExchange; import org.springframework.util.StringUtils; import org.springframework.web.reactive.HandlerMapping; import org.springframework.web.server.MethodNotAllowedException; +import org.springframework.web.server.ResponseStatusException; import org.springframework.web.server.ServerWebExchange; +import static org.hamcrest.Matchers.instanceOf; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -273,11 +276,14 @@ public class ResourceWebHandlerTests { MockServerHttpRequest request = MockServerHttpRequest.method(httpMethod, "").build(); ServerWebExchange exchange = MockServerWebExchange.from(request); setPathWithinHandlerMapping(exchange, requestPath); - this.handler.handle(exchange).block(TIMEOUT); + StepVerifier.create(this.handler.handle(exchange)) + .expectErrorSatisfies(err -> { + assertThat(err, instanceOf(ResponseStatusException.class)); + assertEquals(HttpStatus.NOT_FOUND, ((ResponseStatusException) err).getStatus()); + }).verify(TIMEOUT); if (!location.createRelative(requestPath).exists() && !requestPath.contains(":")) { fail(requestPath + " doesn't actually exist as a relative path"); } - assertEquals(HttpStatus.NOT_FOUND, exchange.getResponse().getStatusCode()); } @Test @@ -363,8 +369,11 @@ public class ResourceWebHandlerTests { public void directory() throws Exception { MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("").build()); setPathWithinHandlerMapping(exchange, "js/"); - this.handler.handle(exchange).block(TIMEOUT); - assertEquals(HttpStatus.NOT_FOUND, exchange.getResponse().getStatusCode()); + StepVerifier.create(this.handler.handle(exchange)) + .expectErrorSatisfies(err -> { + assertThat(err, instanceOf(ResponseStatusException.class)); + assertEquals(HttpStatus.NOT_FOUND, ((ResponseStatusException) err).getStatus()); + }).verify(TIMEOUT); } @Test @@ -381,8 +390,11 @@ public class ResourceWebHandlerTests { public void missingResourcePath() throws Exception { MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("").build()); setPathWithinHandlerMapping(exchange, ""); - this.handler.handle(exchange).block(TIMEOUT); - assertEquals(HttpStatus.NOT_FOUND, exchange.getResponse().getStatusCode()); + StepVerifier.create(this.handler.handle(exchange)) + .expectErrorSatisfies(err -> { + assertThat(err, instanceOf(ResponseStatusException.class)); + assertEquals(HttpStatus.NOT_FOUND, ((ResponseStatusException) err).getStatus()); + }).verify(TIMEOUT); } @Test(expected = IllegalArgumentException.class) @@ -409,8 +421,11 @@ public class ResourceWebHandlerTests { MockServerHttpRequest request = MockServerHttpRequest.method(httpMethod, "").build(); MockServerWebExchange exchange = MockServerWebExchange.from(request); setPathWithinHandlerMapping(exchange, "not-there.css"); - this.handler.handle(exchange).block(TIMEOUT); - assertEquals(HttpStatus.NOT_FOUND, exchange.getResponse().getStatusCode()); + StepVerifier.create(this.handler.handle(exchange)) + .expectErrorSatisfies(err -> { + assertThat(err, instanceOf(ResponseStatusException.class)); + assertEquals(HttpStatus.NOT_FOUND, ((ResponseStatusException) err).getStatus()); + }).verify(TIMEOUT); } @Test