Skip to content

Commit 3febec3

Browse files
committed
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
1 parent 1356bd4 commit 3febec3

File tree

2 files changed

+28
-11
lines changed

2 files changed

+28
-11
lines changed

spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
import org.springframework.util.StringUtils;
5151
import org.springframework.web.reactive.HandlerMapping;
5252
import org.springframework.web.server.MethodNotAllowedException;
53+
import org.springframework.web.server.ResponseStatusException;
5354
import org.springframework.web.server.ServerWebExchange;
5455
import org.springframework.web.server.WebHandler;
5556

@@ -86,8 +87,10 @@ public class ResourceWebHandler implements WebHandler, InitializingBean {
8687
/** Set of supported HTTP methods */
8788
private static final Set<HttpMethod> SUPPORTED_METHODS = EnumSet.of(HttpMethod.GET, HttpMethod.HEAD);
8889

89-
private static final Log logger = LogFactory.getLog(ResourceWebHandler.class);
90+
private static final ResponseStatusException NOT_FOUND_EXCEPTION =
91+
new ResponseStatusException(HttpStatus.NOT_FOUND);
9092

93+
private static final Log logger = LogFactory.getLog(ResourceWebHandler.class);
9194

9295
private final List<Resource> locations = new ArrayList<>(4);
9396

@@ -245,8 +248,7 @@ public Mono<Void> handle(ServerWebExchange exchange) {
245248
return getResource(exchange)
246249
.switchIfEmpty(Mono.defer(() -> {
247250
logger.trace("No matching resource found - returning 404");
248-
exchange.getResponse().setStatusCode(HttpStatus.NOT_FOUND);
249-
return Mono.empty();
251+
return Mono.error(NOT_FOUND_EXCEPTION);
250252
}))
251253
.flatMap(resource -> {
252254
try {

spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,14 @@
5151
import org.springframework.util.StringUtils;
5252
import org.springframework.web.reactive.HandlerMapping;
5353
import org.springframework.web.server.MethodNotAllowedException;
54+
import org.springframework.web.server.ResponseStatusException;
5455
import org.springframework.web.server.ServerWebExchange;
5556

57+
import static org.hamcrest.Matchers.instanceOf;
5658
import static org.junit.Assert.assertEquals;
5759
import static org.junit.Assert.assertNull;
5860
import static org.junit.Assert.assertSame;
61+
import static org.junit.Assert.assertThat;
5962
import static org.junit.Assert.assertTrue;
6063
import static org.junit.Assert.fail;
6164

@@ -273,11 +276,14 @@ private void testInvalidPath(HttpMethod httpMethod, String requestPath, Resource
273276
MockServerHttpRequest request = MockServerHttpRequest.method(httpMethod, "").build();
274277
ServerWebExchange exchange = MockServerWebExchange.from(request);
275278
setPathWithinHandlerMapping(exchange, requestPath);
276-
this.handler.handle(exchange).block(TIMEOUT);
279+
StepVerifier.create(this.handler.handle(exchange))
280+
.expectErrorSatisfies(err -> {
281+
assertThat(err, instanceOf(ResponseStatusException.class));
282+
assertEquals(HttpStatus.NOT_FOUND, ((ResponseStatusException) err).getStatus());
283+
}).verify(TIMEOUT);
277284
if (!location.createRelative(requestPath).exists() && !requestPath.contains(":")) {
278285
fail(requestPath + " doesn't actually exist as a relative path");
279286
}
280-
assertEquals(HttpStatus.NOT_FOUND, exchange.getResponse().getStatusCode());
281287
}
282288

283289
@Test
@@ -363,8 +369,11 @@ public void modified() throws Exception {
363369
public void directory() throws Exception {
364370
MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("").build());
365371
setPathWithinHandlerMapping(exchange, "js/");
366-
this.handler.handle(exchange).block(TIMEOUT);
367-
assertEquals(HttpStatus.NOT_FOUND, exchange.getResponse().getStatusCode());
372+
StepVerifier.create(this.handler.handle(exchange))
373+
.expectErrorSatisfies(err -> {
374+
assertThat(err, instanceOf(ResponseStatusException.class));
375+
assertEquals(HttpStatus.NOT_FOUND, ((ResponseStatusException) err).getStatus());
376+
}).verify(TIMEOUT);
368377
}
369378

370379
@Test
@@ -381,8 +390,11 @@ public void directoryInJarFile() throws Exception {
381390
public void missingResourcePath() throws Exception {
382391
MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("").build());
383392
setPathWithinHandlerMapping(exchange, "");
384-
this.handler.handle(exchange).block(TIMEOUT);
385-
assertEquals(HttpStatus.NOT_FOUND, exchange.getResponse().getStatusCode());
393+
StepVerifier.create(this.handler.handle(exchange))
394+
.expectErrorSatisfies(err -> {
395+
assertThat(err, instanceOf(ResponseStatusException.class));
396+
assertEquals(HttpStatus.NOT_FOUND, ((ResponseStatusException) err).getStatus());
397+
}).verify(TIMEOUT);
386398
}
387399

388400
@Test(expected = IllegalArgumentException.class)
@@ -409,8 +421,11 @@ private void resourceNotFound(HttpMethod httpMethod) throws Exception {
409421
MockServerHttpRequest request = MockServerHttpRequest.method(httpMethod, "").build();
410422
MockServerWebExchange exchange = MockServerWebExchange.from(request);
411423
setPathWithinHandlerMapping(exchange, "not-there.css");
412-
this.handler.handle(exchange).block(TIMEOUT);
413-
assertEquals(HttpStatus.NOT_FOUND, exchange.getResponse().getStatusCode());
424+
StepVerifier.create(this.handler.handle(exchange))
425+
.expectErrorSatisfies(err -> {
426+
assertThat(err, instanceOf(ResponseStatusException.class));
427+
assertEquals(HttpStatus.NOT_FOUND, ((ResponseStatusException) err).getStatus());
428+
}).verify(TIMEOUT);
414429
}
415430

416431
@Test

0 commit comments

Comments
 (0)