diff --git a/spring-core/src/main/java/org/springframework/core/codec/ResourceRegionEncoder.java b/spring-core/src/main/java/org/springframework/core/codec/ResourceRegionEncoder.java index 80fd50bb24..273511011c 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/ResourceRegionEncoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/ResourceRegionEncoder.java @@ -28,6 +28,8 @@ import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import org.springframework.core.ResolvableType; +import org.springframework.core.io.InputStreamResource; +import org.springframework.core.io.Resource; import org.springframework.core.io.buffer.DataBuffer; import org.springframework.core.io.buffer.DataBufferFactory; import org.springframework.core.io.buffer.DataBufferUtils; @@ -134,7 +136,7 @@ public class ResourceRegionEncoder extends AbstractEncoder { private byte[] getContentRangeHeader(ResourceRegion region) { long start = region.getPosition(); long end = start + region.getCount() - 1; - OptionalLong contentLength = ResourceUtils.contentLength(region.getResource()); + OptionalLong contentLength = contentLength(region.getResource()); if (contentLength.isPresent()) { return getAsciiBytes("Content-Range: bytes " + start + "-" + end + "/" + contentLength.getAsLong() + "\r\n\r\n"); } @@ -143,4 +145,22 @@ public class ResourceRegionEncoder extends AbstractEncoder { } } + /** + * Determine, if possible, the contentLength of the given resource without reading it. + * @param resource the resource instance + * @return the contentLength of the resource + */ + private OptionalLong contentLength(Resource resource) { + // Don't try to determine contentLength on InputStreamResource - cannot be read afterwards... + // Note: custom InputStreamResource subclasses could provide a pre-calculated content length! + if (InputStreamResource.class != resource.getClass()) { + try { + return OptionalLong.of(resource.contentLength()); + } + catch (IOException ignored) { + } + } + return OptionalLong.empty(); + } + } diff --git a/spring-core/src/main/java/org/springframework/util/ResourceUtils.java b/spring-core/src/main/java/org/springframework/util/ResourceUtils.java index 1ab9a67cac..0f37194c05 100644 --- a/spring-core/src/main/java/org/springframework/util/ResourceUtils.java +++ b/spring-core/src/main/java/org/springframework/util/ResourceUtils.java @@ -18,16 +18,11 @@ package org.springframework.util; import java.io.File; import java.io.FileNotFoundException; -import java.io.IOException; import java.net.MalformedURLException; import java.net.URI; import java.net.URISyntaxException; import java.net.URL; import java.net.URLConnection; -import java.util.OptionalLong; - -import org.springframework.core.io.InputStreamResource; -import org.springframework.core.io.Resource; /** * Utility methods for resolving resource locations to files in the @@ -389,23 +384,4 @@ public abstract class ResourceUtils { con.setUseCaches(con.getClass().getSimpleName().startsWith("JNLP")); } - /** - * Determine, if possible, the contentLength of the given resource - * without reading it. - * @param resource the resource instance - * @return the contentLength of the resource - */ - public static OptionalLong contentLength(Resource resource) { - // Don't try to determine contentLength on InputStreamResource - cannot be read afterwards... - // Note: custom InputStreamResource subclasses could provide a pre-calculated content length! - if (InputStreamResource.class != resource.getClass()) { - try { - return OptionalLong.of(resource.contentLength()); - } - catch (IOException ignored) { - } - } - return OptionalLong.empty(); - } - } diff --git a/spring-web-reactive/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java b/spring-web-reactive/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java index be59b5b709..2bd9d0260b 100644 --- a/spring-web-reactive/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java +++ b/spring-web-reactive/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java @@ -527,9 +527,7 @@ public class ResourceWebHandlerTests { TestSubscriber.subscribe(this.handler.handle(this.exchange)) .assertErrorWith(throwable -> { - assertThat(throwable, instanceOf(ResponseStatusException.class)); - ResponseStatusException exc = (ResponseStatusException) throwable; - assertThat(exc.getStatus(), is(HttpStatus.REQUESTED_RANGE_NOT_SATISFIABLE)); + assertThat(throwable, instanceOf(IllegalArgumentException.class)); }); } 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 cd44721d4b..6baa294ebd 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 @@ -23,6 +23,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.OptionalLong; import org.reactivestreams.Publisher; import reactor.core.publisher.Flux; @@ -31,6 +32,7 @@ import reactor.core.publisher.Mono; import org.springframework.core.ResolvableType; import org.springframework.core.codec.ResourceDecoder; import org.springframework.core.codec.ResourceEncoder; +import org.springframework.core.io.InputStreamResource; import org.springframework.core.io.Resource; import org.springframework.core.io.support.ResourceRegion; import org.springframework.http.HttpHeaders; @@ -43,8 +45,6 @@ import org.springframework.http.ZeroCopyHttpOutputMessage; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.http.server.reactive.ServerHttpResponse; import org.springframework.util.MimeTypeUtils; -import org.springframework.util.ResourceUtils; -import org.springframework.web.server.ResponseStatusException; /** * Implementation of {@link HttpMessageWriter} that can write @@ -74,18 +74,14 @@ public class ResourceHttpMessageWriter extends AbstractServerHttpMessageWriter resolveWriteHints(ResolvableType streamType, ResolvableType elementType, MediaType mediaType, ServerHttpRequest request) { - try { - List httpRanges = request.getHeaders().getRange(); - if (!httpRanges.isEmpty()) { - return Collections.singletonMap(ResourceHttpMessageWriter.HTTP_RANGE_REQUEST_HINT, httpRanges); - } - } - catch (IllegalArgumentException ex) { - throw new ResponseStatusException(HttpStatus.REQUESTED_RANGE_NOT_SATISFIABLE, - "Could not parse Range request header", ex); + + List httpRanges = request.getHeaders().getRange(); + if (!httpRanges.isEmpty()) { + return Collections.singletonMap(ResourceHttpMessageWriter.HTTP_RANGE_REQUEST_HINT, httpRanges); } return Collections.emptyMap(); } @@ -139,12 +135,31 @@ public class ResourceHttpMessageWriter extends AbstractServerHttpMessageWriter writeContent(Resource resource, ResolvableType type, ReactiveHttpOutputMessage outputMessage, Map hints) { + if (outputMessage instanceof ZeroCopyHttpOutputMessage) { Optional file = getFile(resource); if (file.isPresent()) { diff --git a/spring-web/src/main/java/org/springframework/http/codec/ResourceRegionHttpMessageWriter.java b/spring-web/src/main/java/org/springframework/http/codec/ResourceRegionHttpMessageWriter.java index f22710fc2f..2849a48a3e 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/ResourceRegionHttpMessageWriter.java +++ b/spring-web/src/main/java/org/springframework/http/codec/ResourceRegionHttpMessageWriter.java @@ -28,6 +28,7 @@ import reactor.core.publisher.Mono; import org.springframework.core.ResolvableType; import org.springframework.core.codec.ResourceRegionEncoder; +import org.springframework.core.io.InputStreamResource; import org.springframework.core.io.Resource; import org.springframework.core.io.support.ResourceRegion; import org.springframework.http.MediaType; @@ -80,7 +81,7 @@ class ResourceRegionHttpMessageWriter extends EncoderHttpMessageWriter { long start = region.getPosition(); long end = start + region.getCount() - 1; @@ -91,6 +92,24 @@ class ResourceRegionHttpMessageWriter extends EncoderHttpMessageWriter writeResourceRegion(ResourceRegion region, ResolvableType type, ReactiveHttpOutputMessage outputMessage) { if (outputMessage instanceof ZeroCopyHttpOutputMessage) { 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 f7e2c19143..ee03eb790d 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 @@ -16,13 +16,9 @@ package org.springframework.http.codec; -import static org.hamcrest.Matchers.*; -import static org.junit.Assert.*; - import java.nio.charset.StandardCharsets; import java.util.Collections; -import org.hamcrest.Matchers; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -40,16 +36,18 @@ import org.springframework.core.io.buffer.DataBufferUtils; import org.springframework.core.io.buffer.support.DataBufferTestUtils; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpRange; -import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest; import org.springframework.mock.http.server.reactive.test.MockServerHttpResponse; import org.springframework.tests.TestSubscriber; import org.springframework.util.MimeTypeUtils; -import org.springframework.web.server.ResponseStatusException; + +import static org.hamcrest.Matchers.*; +import static org.junit.Assert.*; /** * Unit tests for {@link ResourceHttpMessageWriter}. + * * @author Brian Clozel */ public class ResourceHttpMessageWriterTests { @@ -72,6 +70,7 @@ public class ResourceHttpMessageWriterTests { this.resource = new ByteArrayResource(content.getBytes(StandardCharsets.UTF_8)); } + @Test public void writableMediaTypes() throws Exception { assertThat(this.writer.getWritableMediaTypes(), @@ -80,7 +79,6 @@ public class ResourceHttpMessageWriterTests { @Test public void shouldWriteResource() throws Exception { - TestSubscriber.subscribe(this.writer.write(Mono.just(resource), null, ResolvableType.forClass(Resource.class), MediaType.TEXT_PLAIN, this.request, this.response, Collections.emptyMap())).assertComplete(); @@ -93,7 +91,6 @@ public class ResourceHttpMessageWriterTests { @Test public void shouldWriteResourceRange() throws Exception { - this.request.getHeaders().setRange(Collections.singletonList(HttpRange.createByteRange(0, 5))); TestSubscriber.subscribe(this.writer.write(Mono.just(resource), null, ResolvableType.forClass(Resource.class), @@ -111,10 +108,9 @@ public class ResourceHttpMessageWriterTests { public void shouldThrowErrorInvalidRange() throws Exception { this.request.getHeaders().set(HttpHeaders.RANGE, "invalid"); - this.expectedException.expect(ResponseStatusException.class); + this.expectedException.expect(IllegalArgumentException.class); TestSubscriber.subscribe(this.writer.write(Mono.just(resource), null, ResolvableType.forClass(Resource.class), MediaType.TEXT_PLAIN, this.request, this.response, Collections.emptyMap())); - this.expectedException.expect(Matchers.hasProperty("status", is(HttpStatus.REQUESTED_RANGE_NOT_SATISFIABLE))); } private Mono reduceToString(Publisher buffers, DataBufferFactory bufferFactory) { @@ -127,4 +123,5 @@ public class ResourceHttpMessageWriterTests { }) .map(buffer -> DataBufferTestUtils.dumpString(buffer, StandardCharsets.UTF_8)); } + }