From fbdece6759682c24a69e513960155ea02f380b43 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Mon, 29 Sep 2025 09:21:20 +0100 Subject: [PATCH 1/4] Polishing in ResourceHttpMessageWriter See gh-35536 --- .../http/codec/ResourceHttpMessageWriter.java | 51 +++++++++++-------- 1 file changed, 29 insertions(+), 22 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/http/codec/ResourceHttpMessageWriter.java b/spring-web/src/main/java/org/springframework/http/codec/ResourceHttpMessageWriter.java index ddf10aa40f4..7c7e9573713 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/ResourceHttpMessageWriter.java +++ b/spring-web/src/main/java/org/springframework/http/codec/ResourceHttpMessageWriter.java @@ -51,12 +51,14 @@ import org.springframework.lang.Nullable; import org.springframework.util.MimeTypeUtils; /** - * {@code HttpMessageWriter} that can write a {@link Resource}. + * {@code HttpMessageWriter} that can write a {@link Resource} from both`` client + * and server perspectives. * - *

Also an implementation of {@code HttpMessageWriter} with support for writing one - * or more {@link ResourceRegion}'s based on the HTTP ranges specified in the request. + *

From a server perspective, the server-side only write method supports + * writing one or more {@link ResourceRegion}'s based on HTTP ranges specified + * in the request. * - *

For reading to a Resource, use {@link ResourceDecoder} wrapped with + *

To read a Resource, use {@link ResourceDecoder} wrapped with * {@link DecoderHttpMessageReader}. * * @author Arjen Poutsma @@ -122,16 +124,19 @@ public class ResourceHttpMessageWriter implements HttpMessageWriter { if (result != null) { return result; } - else { - Mono input = Mono.just(resource); - DataBufferFactory factory = message.bufferFactory(); - Flux body = this.encoder.encode(input, factory, type, message.getHeaders().getContentType(), hints) - .subscribeOn(Schedulers.boundedElastic()); - if (logger.isDebugEnabled()) { - body = body.doOnNext(buffer -> Hints.touchDataBuffer(buffer, hints, logger)); - } - return message.writeWith(body); + + Mono input = Mono.just(resource); + DataBufferFactory factory = message.bufferFactory(); + MediaType contentType = message.getHeaders().getContentType(); + + Flux body = this.encoder.encode(input, factory, type, contentType, hints) + .subscribeOn(Schedulers.boundedElastic()); + + if (logger.isDebugEnabled()) { + body = body.doOnNext(buffer -> Hints.touchDataBuffer(buffer, hints, logger)); } + + return message.writeWith(body); })); } @@ -139,7 +144,10 @@ public class ResourceHttpMessageWriter implements HttpMessageWriter { * Adds the default headers for the given resource to the given message. * @since 6.1 */ - public Mono addDefaultHeaders(ReactiveHttpOutputMessage message, Resource resource, @Nullable MediaType contentType, Map hints) { + public Mono addDefaultHeaders( + ReactiveHttpOutputMessage message, Resource resource, @Nullable MediaType contentType, + Map hints) { + return Mono.defer(() -> { HttpHeaders headers = message.getHeaders(); MediaType resourceMediaType = getResourceMediaType(contentType, resource, hints); @@ -149,16 +157,15 @@ public class ResourceHttpMessageWriter implements HttpMessageWriter { headers.set(HttpHeaders.ACCEPT_RANGES, "bytes"); } - if (headers.getContentLength() < 0) { - return lengthOf(resource) - .flatMap(contentLength -> { - headers.setContentLength(contentLength); - return Mono.empty(); - }); - } - else { + if (headers.getContentLength() >= 0) { return Mono.empty(); } + + return lengthOf(resource) + .flatMap(contentLength -> { + headers.setContentLength(contentLength); + return Mono.empty(); + }); }); } From a19b51b7e0c7763c6992cf113b6dbdfe00f58451 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Mon, 29 Sep 2025 09:29:34 +0100 Subject: [PATCH 2/4] Handle invalid position in ResourceHttpMessageWriter Closes gh-35536 --- .../http/codec/ResourceHttpMessageWriter.java | 16 +++++++++++++--- .../codec/ResourceHttpMessageWriterTests.java | 9 +++++++++ 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/http/codec/ResourceHttpMessageWriter.java b/spring-web/src/main/java/org/springframework/http/codec/ResourceHttpMessageWriter.java index 7c7e9573713..3a32540e0e4 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/ResourceHttpMessageWriter.java +++ b/spring-web/src/main/java/org/springframework/http/codec/ResourceHttpMessageWriter.java @@ -233,8 +233,7 @@ public class ResourceHttpMessageWriter implements HttpMessageWriter { ranges = request.getHeaders().getRange(); } catch (IllegalArgumentException ex) { - response.setStatusCode(HttpStatus.REQUESTED_RANGE_NOT_SATISFIABLE); - return response.setComplete(); + return handleInvalidRange(response); } return Mono.from(inputStream).flatMap(resource -> { @@ -242,7 +241,13 @@ public class ResourceHttpMessageWriter implements HttpMessageWriter { return writeResource(resource, elementType, mediaType, response, hints); } response.setStatusCode(HttpStatus.PARTIAL_CONTENT); - List regions = HttpRange.toResourceRegions(ranges, resource); + List regions; + try { + regions = HttpRange.toResourceRegions(ranges, resource); + } + catch (IllegalArgumentException ex) { + return handleInvalidRange(response); + } MediaType resourceMediaType = getResourceMediaType(mediaType, resource, hints); if (regions.size() == 1){ ResourceRegion region = regions.get(0); @@ -268,6 +273,11 @@ public class ResourceHttpMessageWriter implements HttpMessageWriter { }); } + private static Mono handleInvalidRange(ServerHttpResponse response) { + response.setStatusCode(HttpStatus.REQUESTED_RANGE_NOT_SATISFIABLE); + return response.setComplete(); + } + private Mono writeSingleRegion(ResourceRegion region, ReactiveHttpOutputMessage message, Map hints) { diff --git a/spring-web/src/test/java/org/springframework/http/codec/ResourceHttpMessageWriterTests.java b/spring-web/src/test/java/org/springframework/http/codec/ResourceHttpMessageWriterTests.java index 19af37c5610..436513a3fb6 100644 --- a/spring-web/src/test/java/org/springframework/http/codec/ResourceHttpMessageWriterTests.java +++ b/spring-web/src/test/java/org/springframework/http/codec/ResourceHttpMessageWriterTests.java @@ -156,6 +156,15 @@ class ResourceHttpMessageWriterTests { assertThat(this.response.getStatusCode()).isEqualTo(HttpStatus.REQUESTED_RANGE_NOT_SATISFIABLE); } + @Test // gh-35536 + void invalidRangePosition() { + + testWrite(get("/").header(HttpHeaders.RANGE, "bytes=2000-5000").build()); + + assertThat(this.response.getHeaders().getFirst(HttpHeaders.ACCEPT_RANGES)).isEqualTo("bytes"); + assertThat(this.response.getStatusCode()).isEqualTo(HttpStatus.REQUESTED_RANGE_NOT_SATISFIABLE); + } + private void testWrite(MockServerHttpRequest request) { Mono mono = this.writer.write(this.input, null, null, TEXT_PLAIN, request, this.response, HINTS); From 636523a2f5a9e9b9a22fdef0c7b33195de5ea28a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9da=20Housni=20Alaoui?= Date: Wed, 17 Sep 2025 11:09:40 +0200 Subject: [PATCH 3/4] AbstractMockHttpServletRequestBuilder#buildRequest is not idempotent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See gh-35493 Signed-off-by: Réda Housni Alaoui --- .../request/AbstractMockHttpServletRequestBuilder.java | 8 +++++--- .../AbstractMockHttpServletRequestBuilderTests.java | 9 +++++++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/spring-test/src/main/java/org/springframework/test/web/servlet/request/AbstractMockHttpServletRequestBuilder.java b/spring-test/src/main/java/org/springframework/test/web/servlet/request/AbstractMockHttpServletRequestBuilder.java index c6b4ba460e2..ee9fb856cf5 100644 --- a/spring-test/src/main/java/org/springframework/test/web/servlet/request/AbstractMockHttpServletRequestBuilder.java +++ b/spring-test/src/main/java/org/springframework/test/web/servlet/request/AbstractMockHttpServletRequestBuilder.java @@ -74,6 +74,7 @@ import org.springframework.web.util.UrlPathHelper; * @author Arjen Poutsma * @author Sam Brannen * @author Kamill Sokol + * @author Réda Housni Alaoui * @since 6.2 * @param a self reference to the builder type */ @@ -854,16 +855,17 @@ public abstract class AbstractMockHttpServletRequestBuilder map) { diff --git a/spring-test/src/test/java/org/springframework/test/web/servlet/request/AbstractMockHttpServletRequestBuilderTests.java b/spring-test/src/test/java/org/springframework/test/web/servlet/request/AbstractMockHttpServletRequestBuilderTests.java index ba677f5266c..8f4707605d7 100644 --- a/spring-test/src/test/java/org/springframework/test/web/servlet/request/AbstractMockHttpServletRequestBuilderTests.java +++ b/spring-test/src/test/java/org/springframework/test/web/servlet/request/AbstractMockHttpServletRequestBuilderTests.java @@ -31,6 +31,7 @@ import static org.assertj.core.api.Assertions.assertThat; * Tests for {@link AbstractMockHttpServletRequestBuilder} * * @author Stephane Nicoll + * @author Réda Housni Alaoui */ class AbstractMockHttpServletRequestBuilderTests { @@ -97,6 +98,14 @@ class AbstractMockHttpServletRequestBuilderTests { } + @Test + void pathInfoIsNotMutatedByBuildMethod() { + TestRequestBuilder builder = new TestRequestBuilder(HttpMethod.GET).uri("/b"); + assertThat(buildRequest(builder).getPathInfo()).isEqualTo("/b"); + builder.uri("/a"); + assertThat(buildRequest(builder).getPathInfo()).isEqualTo("/a"); + } + private MockHttpServletRequest buildRequest(AbstractMockHttpServletRequestBuilder builder) { return builder.buildRequest(this.servletContext); } From df860fd3cdeaf94513ab9f0d3208ead6214df3b0 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Tue, 30 Sep 2025 15:35:01 +0100 Subject: [PATCH 4/4] Polishing contribution Closes gh-35493 --- .../AbstractMockHttpServletRequestBuilder.java | 18 +++++++----------- ...ractMockHttpServletRequestBuilderTests.java | 2 +- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/spring-test/src/main/java/org/springframework/test/web/servlet/request/AbstractMockHttpServletRequestBuilder.java b/spring-test/src/main/java/org/springframework/test/web/servlet/request/AbstractMockHttpServletRequestBuilder.java index ee9fb856cf5..1a3fe0684f8 100644 --- a/spring-test/src/main/java/org/springframework/test/web/servlet/request/AbstractMockHttpServletRequestBuilder.java +++ b/spring-test/src/main/java/org/springframework/test/web/servlet/request/AbstractMockHttpServletRequestBuilder.java @@ -74,7 +74,6 @@ import org.springframework.web.util.UrlPathHelper; * @author Arjen Poutsma * @author Sam Brannen * @author Kamill Sokol - * @author Réda Housni Alaoui * @since 6.2 * @param a self reference to the builder type */ @@ -855,17 +854,14 @@ public abstract class AbstractMockHttpServletRequestBuilder "Invalid servlet path [" + this.servletPath + "] for request URI [" + requestUri + "]"); + String other = requestUri.substring(this.contextPath.length() + this.servletPath.length()); + path = (StringUtils.hasText(other) ? UrlPathHelper.defaultInstance.decodeRequestString(request, other) : null); } - request.setPathInfo(pathInfoToUse); + request.setPathInfo(path); } private void addRequestParams(MockHttpServletRequest request, MultiValueMap map) { diff --git a/spring-test/src/test/java/org/springframework/test/web/servlet/request/AbstractMockHttpServletRequestBuilderTests.java b/spring-test/src/test/java/org/springframework/test/web/servlet/request/AbstractMockHttpServletRequestBuilderTests.java index 8f4707605d7..70680b7fc37 100644 --- a/spring-test/src/test/java/org/springframework/test/web/servlet/request/AbstractMockHttpServletRequestBuilderTests.java +++ b/spring-test/src/test/java/org/springframework/test/web/servlet/request/AbstractMockHttpServletRequestBuilderTests.java @@ -98,7 +98,7 @@ class AbstractMockHttpServletRequestBuilderTests { } - @Test + @Test // gh-35493 void pathInfoIsNotMutatedByBuildMethod() { TestRequestBuilder builder = new TestRequestBuilder(HttpMethod.GET).uri("/b"); assertThat(buildRequest(builder).getPathInfo()).isEqualTo("/b");