From d37ef61b306c83d4e75d3b673bed9d19544a0e7f Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Fri, 20 Jan 2023 17:07:35 +0100 Subject: [PATCH] Resolve static resources without wildcard pattern Prior to this commit, the reactive `ResourceWebHandler` would only look at the path within the current mapping when resolving static resources to be served. This means that when registering a handler at `"/resources/**"` with a `"classpath:/static/"` location, the handler would process a `"GET /resources/file.txt"` as the `"/static/file.txt"` classpath location. When a developer registers a fixed pattern like `"/resources/file.txt"` with the same location, the path within the handler mapping is empty as there is no dynamic part in the given pattern. While the typical use case for this feature is to register multiple resources at once with a pattern, we should support a single registration like this. This commit ensures that if the matching `PathPattern` for the current request does not have a pattern syntax (i.e. no regexp, no wildcard), we can use it to match the resource directly. Otherwise, we can use the path within the handler mapping to resolve the resource as before. Closes gh-29739 --- .../reactive/resource/ResourceWebHandler.java | 19 ++++--- .../config/ResourceHandlerRegistryTests.java | 3 ++ .../resource/ResourceWebHandlerTests.java | 50 ++++++++++++++++++- 3 files changed, 65 insertions(+), 7 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 a8ca7a322f..e665a68bbd 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -57,6 +57,7 @@ import org.springframework.web.server.MethodNotAllowedException; import org.springframework.web.server.ResponseStatusException; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.WebHandler; +import org.springframework.web.util.pattern.PathPattern; /** * {@code HttpRequestHandler} that serves static resources in an optimized way @@ -456,10 +457,8 @@ public class ResourceWebHandler implements WebHandler, InitializingBean { } protected Mono getResource(ServerWebExchange exchange) { - String name = HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE; - PathContainer pathWithinHandler = exchange.getRequiredAttribute(name); - - String path = processPath(pathWithinHandler.value()); + String rawPath = getResourcePath(exchange); + String path = processPath(rawPath); if (!StringUtils.hasText(path) || isInvalidPath(path)) { return Mono.empty(); } @@ -474,6 +473,15 @@ public class ResourceWebHandler implements WebHandler, InitializingBean { .flatMap(resource -> this.transformerChain.transform(exchange, resource)); } + private String getResourcePath(ServerWebExchange exchange) { + PathPattern pattern = exchange.getRequiredAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE); + if (!pattern.hasPatternSyntax()) { + return pattern.getPatternString(); + } + PathContainer pathWithinHandler = exchange.getRequiredAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE); + return pathWithinHandler.value(); + } + /** * Process the given resource path. *

The default implementation replaces: @@ -484,7 +492,6 @@ public class ResourceWebHandler implements WebHandler, InitializingBean { * with a single "/" or "". For example {@code " / // foo/bar"} * becomes {@code "/foo/bar"}. * - * @since 3.2.12 */ protected String processPath(String path) { path = StringUtils.replace(path, "\\", "/"); diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/config/ResourceHandlerRegistryTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/config/ResourceHandlerRegistryTests.java index 2df9809562..79e56317c8 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/config/ResourceHandlerRegistryTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/config/ResourceHandlerRegistryTests.java @@ -46,6 +46,7 @@ import org.springframework.web.reactive.resource.VersionResourceResolver; import org.springframework.web.reactive.resource.WebJarsResourceResolver; import org.springframework.web.testfixture.http.server.reactive.MockServerHttpRequest; import org.springframework.web.testfixture.server.MockServerWebExchange; +import org.springframework.web.util.pattern.PathPatternParser; import static java.nio.charset.StandardCharsets.UTF_8; import static org.assertj.core.api.Assertions.assertThat; @@ -82,6 +83,8 @@ class ResourceHandlerRegistryTests { MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("")); exchange.getAttributes().put(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, PathContainer.parsePath("/testStylesheet.css")); + exchange.getAttributes().put(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE, + new PathPatternParser().parse("/**")); ResourceWebHandler handler = getHandler("/resources/**"); handler.handle(exchange).block(Duration.ofSeconds(5)); 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 2030681274..82daf08e48 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 @@ -57,6 +57,7 @@ import org.springframework.web.testfixture.http.server.reactive.MockServerHttpRe import org.springframework.web.testfixture.http.server.reactive.MockServerHttpResponse; import org.springframework.web.testfixture.server.MockServerWebExchange; import org.springframework.web.util.UriUtils; +import org.springframework.web.util.pattern.PathPatternParser; import static java.nio.charset.StandardCharsets.UTF_8; import static org.assertj.core.api.Assertions.assertThat; @@ -72,6 +73,7 @@ import static org.mockito.Mockito.mock; * * @author Rossen Stoyanchev * @author Sam Brannen + * @author Brian Clozel */ class ResourceWebHandlerTests { @@ -102,6 +104,7 @@ class ResourceWebHandlerTests { void getResource() throws Exception { MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("")); setPathWithinHandlerMapping(exchange, "foo.css"); + setBestMachingPattern(exchange, "/**"); this.handler.handle(exchange).block(TIMEOUT); HttpHeaders headers = exchange.getResponse().getHeaders(); @@ -119,6 +122,7 @@ class ResourceWebHandlerTests { void getResourceHttpHeader() throws Exception { MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.head("")); setPathWithinHandlerMapping(exchange, "foo.css"); + setBestMachingPattern(exchange, "/**"); this.handler.handle(exchange).block(TIMEOUT); assertThat((Object) exchange.getResponse().getStatusCode()).isNull(); @@ -139,6 +143,7 @@ class ResourceWebHandlerTests { void getResourceHttpOptions() { MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.options("")); setPathWithinHandlerMapping(exchange, "foo.css"); + setBestMachingPattern(exchange, "/**"); this.handler.handle(exchange).block(TIMEOUT); assertThat(exchange.getResponse().getStatusCode()).isNull(); @@ -149,6 +154,7 @@ class ResourceWebHandlerTests { void getResourceNoCache() throws Exception { MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("")); setPathWithinHandlerMapping(exchange, "foo.css"); + setBestMachingPattern(exchange, "/**"); this.handler.setCacheControl(CacheControl.noStore()); this.handler.handle(exchange).block(TIMEOUT); @@ -169,6 +175,7 @@ class ResourceWebHandlerTests { MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("")); setPathWithinHandlerMapping(exchange, "versionString/foo.css"); + setBestMachingPattern(exchange, "/**"); this.handler.handle(exchange).block(TIMEOUT); assertThat(exchange.getResponse().getHeaders().getETag()).isEqualTo("W/\"versionString\""); @@ -180,6 +187,7 @@ class ResourceWebHandlerTests { void getResourceWithHtmlMediaType() throws Exception { MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("")); setPathWithinHandlerMapping(exchange, "foo.html"); + setBestMachingPattern(exchange, "/**"); this.handler.handle(exchange).block(TIMEOUT); HttpHeaders headers = exchange.getResponse().getHeaders(); @@ -195,6 +203,7 @@ class ResourceWebHandlerTests { void getResourceFromAlternatePath() throws Exception { MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("")); setPathWithinHandlerMapping(exchange, "baz.css"); + setBestMachingPattern(exchange, "/**"); this.handler.handle(exchange).block(TIMEOUT); HttpHeaders headers = exchange.getResponse().getHeaders(); @@ -212,6 +221,7 @@ class ResourceWebHandlerTests { void getResourceFromSubDirectory() { MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("")); setPathWithinHandlerMapping(exchange, "js/foo.js"); + setBestMachingPattern(exchange, "/**"); this.handler.handle(exchange).block(TIMEOUT); assertThat(exchange.getResponse().getHeaders().getContentType()).isEqualTo(MediaType.parseMediaType("application/javascript")); @@ -222,6 +232,7 @@ class ResourceWebHandlerTests { void getResourceFromSubDirectoryOfAlternatePath() { MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("")); setPathWithinHandlerMapping(exchange, "js/baz.js"); + setBestMachingPattern(exchange, "/**"); this.handler.handle(exchange).block(TIMEOUT); HttpHeaders headers = exchange.getResponse().getHeaders(); @@ -229,6 +240,18 @@ class ResourceWebHandlerTests { assertResponseBody(exchange, "function foo() { console.log(\"hello world\"); }"); } + @Test + void getResourceWithFullPathAsPattern() { + MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/foo.css")); + setPathWithinHandlerMapping(exchange, ""); + setBestMachingPattern(exchange, "/foo.css"); + this.handler.handle(exchange).block(TIMEOUT); + HttpHeaders headers = exchange.getResponse().getHeaders(); + assertThat(headers.getContentType()).isEqualTo(MediaType.parseMediaType("text/css")); + assertThat(headers.getContentLength()).isEqualTo(17); + assertResponseBody(exchange, "h1 { color:red; }"); + } + @Test void getResourceWithRegisteredMediaType() throws Exception { MediaType mediaType = new MediaType("foo", "bar"); @@ -240,6 +263,7 @@ class ResourceWebHandlerTests { MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("")); setPathWithinHandlerMapping(exchange, "foo.bar"); + setBestMachingPattern(exchange, "/**"); handler.handle(exchange).block(TIMEOUT); HttpHeaders headers = exchange.getResponse().getHeaders(); @@ -258,6 +282,7 @@ class ResourceWebHandlerTests { MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("")); setPathWithinHandlerMapping(exchange, UriUtils.encodePath("test/foo with spaces.css", UTF_8)); + setBestMachingPattern(exchange, "/**"); handler.handle(exchange).block(TIMEOUT); HttpHeaders headers = exchange.getResponse().getHeaders(); @@ -294,6 +319,7 @@ class ResourceWebHandlerTests { MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("") .header("Accept", "application/json,text/plain,*/*")); setPathWithinHandlerMapping(exchange, "foo.html"); + setBestMachingPattern(exchange, "/**"); handler.handle(exchange).block(TIMEOUT); assertThat(exchange.getResponse().getHeaders().getContentType()).isEqualTo(MediaType.TEXT_HTML); @@ -337,7 +363,7 @@ class ResourceWebHandlerTests { private void testInvalidPath(String requestPath, ResourceWebHandler handler) { ServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("")); setPathWithinHandlerMapping(exchange, requestPath); - + setBestMachingPattern(exchange, "/**"); StepVerifier.create(handler.handle(exchange)) .expectErrorSatisfies(err -> { assertThat(err).isInstanceOf(ResponseStatusException.class); @@ -379,6 +405,7 @@ class ResourceWebHandlerTests { ServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.method(httpMethod, "")); setPathWithinHandlerMapping(exchange, requestPath); + setBestMachingPattern(exchange, "/**"); StepVerifier.create(this.handler.handle(exchange)) .expectErrorSatisfies(err -> { assertThat(err).isInstanceOf(ResponseStatusException.class); @@ -449,6 +476,7 @@ class ResourceWebHandlerTests { MockServerHttpRequest.get("").ifModifiedSince(resourceLastModified("test/foo.css"))); setPathWithinHandlerMapping(exchange, "foo.css"); + setBestMachingPattern(exchange, "/**"); this.handler.handle(exchange).block(TIMEOUT); assertThat(exchange.getResponse().getStatusCode()).isEqualTo(HttpStatus.NOT_MODIFIED); } @@ -459,6 +487,7 @@ class ResourceWebHandlerTests { MockServerHttpRequest request = MockServerHttpRequest.get("").ifModifiedSince(timestamp).build(); MockServerWebExchange exchange = MockServerWebExchange.from(request); setPathWithinHandlerMapping(exchange, "foo.css"); + setBestMachingPattern(exchange, "/**"); this.handler.handle(exchange).block(TIMEOUT); assertThat((Object) exchange.getResponse().getStatusCode()).isNull(); @@ -469,6 +498,7 @@ class ResourceWebHandlerTests { void directory() { MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("")); setPathWithinHandlerMapping(exchange, "js/"); + setBestMachingPattern(exchange, "/**"); StepVerifier.create(this.handler.handle(exchange)) .expectErrorSatisfies(err -> { assertThat(err).isInstanceOf(ResponseStatusException.class); @@ -480,6 +510,7 @@ class ResourceWebHandlerTests { void directoryInJarFile() { MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("")); setPathWithinHandlerMapping(exchange, "underscorejs/"); + setBestMachingPattern(exchange, "/**"); StepVerifier.create(this.handler.handle(exchange)) .expectErrorSatisfies(err -> { assertThat(err).isInstanceOf(ResponseStatusException.class); @@ -491,6 +522,7 @@ class ResourceWebHandlerTests { void missingResourcePath() { MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("")); setPathWithinHandlerMapping(exchange, ""); + setBestMachingPattern(exchange, "/**"); StepVerifier.create(this.handler.handle(exchange)) .expectErrorSatisfies(err -> { assertThat(err).isInstanceOf(ResponseStatusException.class); @@ -509,6 +541,7 @@ class ResourceWebHandlerTests { void unsupportedHttpMethod() { MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.post("")); setPathWithinHandlerMapping(exchange, "foo.css"); + setBestMachingPattern(exchange, "/**"); assertThatExceptionOfType(MethodNotAllowedException.class).isThrownBy(() -> this.handler.handle(exchange).block(TIMEOUT)); } @@ -519,6 +552,7 @@ class ResourceWebHandlerTests { MockServerHttpRequest request = MockServerHttpRequest.method(method, "").build(); MockServerWebExchange exchange = MockServerWebExchange.from(request); setPathWithinHandlerMapping(exchange, "not-there.css"); + setBestMachingPattern(exchange, "/**"); Mono mono = this.handler.handle(exchange); StepVerifier.create(mono) @@ -538,6 +572,7 @@ class ResourceWebHandlerTests { MockServerHttpRequest request = MockServerHttpRequest.get("").header("Range", "bytes=0-1").build(); MockServerWebExchange exchange = MockServerWebExchange.from(request); setPathWithinHandlerMapping(exchange, "foo.txt"); + setBestMachingPattern(exchange, "/**"); this.handler.handle(exchange).block(TIMEOUT); assertThat(exchange.getResponse().getStatusCode()).isEqualTo(HttpStatus.PARTIAL_CONTENT); @@ -554,6 +589,7 @@ class ResourceWebHandlerTests { MockServerHttpRequest request = MockServerHttpRequest.get("").header("range", "bytes=9-").build(); MockServerWebExchange exchange = MockServerWebExchange.from(request); setPathWithinHandlerMapping(exchange, "foo.txt"); + setBestMachingPattern(exchange, "/**"); this.handler.handle(exchange).block(TIMEOUT); assertThat(exchange.getResponse().getStatusCode()).isEqualTo(HttpStatus.PARTIAL_CONTENT); @@ -570,6 +606,7 @@ class ResourceWebHandlerTests { MockServerHttpRequest request = MockServerHttpRequest.get("").header("range", "bytes=9-10000").build(); MockServerWebExchange exchange = MockServerWebExchange.from(request); setPathWithinHandlerMapping(exchange, "foo.txt"); + setBestMachingPattern(exchange, "/**"); this.handler.handle(exchange).block(TIMEOUT); assertThat(exchange.getResponse().getStatusCode()).isEqualTo(HttpStatus.PARTIAL_CONTENT); @@ -586,6 +623,7 @@ class ResourceWebHandlerTests { MockServerHttpRequest request = MockServerHttpRequest.get("").header("range", "bytes=-1").build(); MockServerWebExchange exchange = MockServerWebExchange.from(request); setPathWithinHandlerMapping(exchange, "foo.txt"); + setBestMachingPattern(exchange, "/**"); this.handler.handle(exchange).block(TIMEOUT); assertThat(exchange.getResponse().getStatusCode()).isEqualTo(HttpStatus.PARTIAL_CONTENT); @@ -602,6 +640,7 @@ class ResourceWebHandlerTests { MockServerHttpRequest request = MockServerHttpRequest.get("").header("range", "bytes=-11").build(); MockServerWebExchange exchange = MockServerWebExchange.from(request); setPathWithinHandlerMapping(exchange, "foo.txt"); + setBestMachingPattern(exchange, "/**"); this.handler.handle(exchange).block(TIMEOUT); assertThat(exchange.getResponse().getStatusCode()).isEqualTo(HttpStatus.PARTIAL_CONTENT); @@ -618,6 +657,7 @@ class ResourceWebHandlerTests { MockServerHttpRequest request = MockServerHttpRequest.get("").header("range", "bytes=foo bar").build(); MockServerWebExchange exchange = MockServerWebExchange.from(request); setPathWithinHandlerMapping(exchange, "foo.txt"); + setBestMachingPattern(exchange, "/**"); StepVerifier.create(this.handler.handle(exchange)) .expectNextCount(0) @@ -633,6 +673,7 @@ class ResourceWebHandlerTests { MockServerHttpRequest request = MockServerHttpRequest.get("").header("Range", "bytes=0-1, 4-5, 8-9").build(); MockServerWebExchange exchange = MockServerWebExchange.from(request); setPathWithinHandlerMapping(exchange, "foo.txt"); + setBestMachingPattern(exchange, "/**"); this.handler.handle(exchange).block(TIMEOUT); assertThat(exchange.getResponse().getStatusCode()).isEqualTo(HttpStatus.PARTIAL_CONTENT); @@ -677,6 +718,7 @@ class ResourceWebHandlerTests { MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("")); exchange.getResponse().getHeaders().setCacheControl(CacheControl.noStore().getHeaderValue()); setPathWithinHandlerMapping(exchange, "foo.css"); + setBestMachingPattern(exchange, "/**"); this.handler.handle(exchange).block(TIMEOUT); assertThat(exchange.getResponse().getHeaders().getCacheControl()).isEqualTo("max-age=3600"); @@ -686,6 +728,7 @@ class ResourceWebHandlerTests { void ignoreLastModified() { MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("")); setPathWithinHandlerMapping(exchange, "foo.css"); + setBestMachingPattern(exchange, "/**"); this.handler.setUseLastModified(false); this.handler.handle(exchange).block(TIMEOUT); @@ -702,6 +745,11 @@ class ResourceWebHandlerTests { PathContainer.parsePath(path)); } + private void setBestMachingPattern(ServerWebExchange exchange, String pattern) { + exchange.getAttributes().put(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE, + new PathPatternParser().parse(pattern)); + } + private long resourceLastModified(String resourceName) throws IOException { return new ClassPathResource(resourceName, getClass()).getFile().lastModified(); }