Skip to content

Commit 578af59

Browse files
bclozelbclozel
committed
Polish byte-range resource handling
1 parent 129d7be commit 578af59

File tree

4 files changed

+35
-30
lines changed

4 files changed

+35
-30
lines changed

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@
5454
import org.springframework.web.reactive.accept.CompositeContentTypeResolver;
5555
import org.springframework.web.reactive.accept.PathExtensionContentTypeResolver;
5656
import org.springframework.web.server.MethodNotAllowedException;
57-
import org.springframework.web.server.ResponseStatusException;
5857
import org.springframework.web.server.ServerWebExchange;
5958
import org.springframework.web.server.WebHandler;
6059

@@ -331,6 +330,7 @@ public Mono<Void> handle(ServerWebExchange exchange) {
331330
// Content phase
332331
if (HttpMethod.HEAD.equals(exchange.getRequest().getMethod())) {
333332
setHeaders(exchange, resource, mediaType);
333+
exchange.getResponse().getHeaders().set(HttpHeaders.ACCEPT_RANGES, "bytes");
334334
logger.trace("HEAD request - skipping content");
335335
return Mono.empty();
336336
}
@@ -340,7 +340,7 @@ public Mono<Void> handle(ServerWebExchange exchange) {
340340
null, ResolvableType.forClass(Resource.class), mediaType,
341341
exchange.getRequest(), exchange.getResponse(), Collections.emptyMap());
342342
}
343-
catch (IOException|ResponseStatusException ex) {
343+
catch (IOException ex) {
344344
return Mono.error(ex);
345345
}
346346
});
@@ -503,7 +503,6 @@ protected void setHeaders(ServerWebExchange exchange, Resource resource, MediaTy
503503
HttpHeaders resourceHeaders = ((HttpResource) resource).getResponseHeaders();
504504
exchange.getResponse().getHeaders().putAll(resourceHeaders);
505505
}
506-
headers.set(HttpHeaders.ACCEPT_RANGES, "bytes");
507506
}
508507

509508

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -525,10 +525,10 @@ public void partialContentInvalidRangeHeader() throws Exception {
525525
this.request.addHeader("Range", "bytes= foo bar");
526526
this.exchange.getAttributes().put(PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, "foo.txt");
527527

528-
TestSubscriber.subscribe(this.handler.handle(this.exchange))
529-
.assertErrorWith(throwable -> {
530-
assertThat(throwable, instanceOf(IllegalArgumentException.class));
531-
});
528+
TestSubscriber.subscribe(this.handler.handle(this.exchange)).assertComplete();
529+
530+
assertEquals(HttpStatus.REQUESTED_RANGE_NOT_SATISFIABLE, this.response.getStatusCode());
531+
assertEquals("bytes", this.response.getHeaders().getFirst("Accept-Ranges"));
532532
}
533533

534534
@Test

‎spring-web/src/main/java/org/springframework/http/codec/ResourceHttpMessageWriter.java

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -104,24 +104,30 @@ public Mono<Void> write(Publisher<? extends Resource> inputStream, ResolvableTyp
104104
public Mono<Void> write(Publisher<? extends Resource> inputStream, ResolvableType streamType,
105105
ResolvableType elementType, MediaType mediaType, ServerHttpRequest request,
106106
ServerHttpResponse response, Map<String, Object> hints) {
107-
108-
Map<String, Object> mergedHints = new HashMap<>(hints);
109-
mergedHints.putAll(resolveWriteHints(streamType, elementType, mediaType, request));
110-
if (mergedHints.containsKey(HTTP_RANGE_REQUEST_HINT)) {
111-
response.setStatusCode(HttpStatus.PARTIAL_CONTENT);
112-
List<HttpRange> httpRanges = (List<HttpRange>) mergedHints.get(HTTP_RANGE_REQUEST_HINT);
113-
if (httpRanges.size() > 1) {
114-
final String boundary = MimeTypeUtils.generateMultipartBoundaryString();
115-
mergedHints.put(ResourceRegionHttpMessageWriter.BOUNDARY_STRING_HINT, boundary);
107+
try {
108+
response.getHeaders().set(HttpHeaders.ACCEPT_RANGES, "bytes");
109+
Map<String, Object> mergedHints = new HashMap<>(hints);
110+
mergedHints.putAll(resolveWriteHints(streamType, elementType, mediaType, request));
111+
if (mergedHints.containsKey(HTTP_RANGE_REQUEST_HINT)) {
112+
response.setStatusCode(HttpStatus.PARTIAL_CONTENT);
113+
List<HttpRange> httpRanges = (List<HttpRange>) mergedHints.get(HTTP_RANGE_REQUEST_HINT);
114+
if (httpRanges.size() > 1) {
115+
final String boundary = MimeTypeUtils.generateMultipartBoundaryString();
116+
mergedHints.put(ResourceRegionHttpMessageWriter.BOUNDARY_STRING_HINT, boundary);
117+
}
118+
Flux<ResourceRegion> regions = Flux.from(inputStream)
119+
.flatMap(resource -> Flux.fromIterable(HttpRange.toResourceRegions(httpRanges, resource)));
120+
121+
return this.resourceRegionHttpMessageWriter
122+
.write(regions, ResolvableType.forClass(ResourceRegion.class), mediaType, response, mergedHints);
123+
}
124+
else {
125+
return write(inputStream, elementType, mediaType, response, mergedHints);
116126
}
117-
Flux<ResourceRegion> regions = Flux.from(inputStream)
118-
.flatMap(resource -> Flux.fromIterable(HttpRange.toResourceRegions(httpRanges, resource)));
119-
120-
return this.resourceRegionHttpMessageWriter
121-
.write(regions, ResolvableType.forClass(ResourceRegion.class), mediaType, response, mergedHints);
122127
}
123-
else {
124-
return write(inputStream, elementType, mediaType, response, mergedHints);
128+
catch (IllegalArgumentException exc) {
129+
response.setStatusCode(HttpStatus.REQUESTED_RANGE_NOT_SATISFIABLE);
130+
return response.setComplete();
125131
}
126132
}
127133

‎spring-web/src/test/java/org/springframework/http/codec/ResourceHttpMessageWriterTests.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,7 @@
2020
import java.util.Collections;
2121

2222
import org.junit.Before;
23-
import org.junit.Rule;
2423
import org.junit.Test;
25-
import org.junit.rules.ExpectedException;
2624
import org.reactivestreams.Publisher;
2725
import reactor.core.publisher.Flux;
2826
import reactor.core.publisher.Mono;
@@ -36,6 +34,7 @@
3634
import org.springframework.core.io.buffer.support.DataBufferTestUtils;
3735
import org.springframework.http.HttpHeaders;
3836
import org.springframework.http.HttpRange;
37+
import org.springframework.http.HttpStatus;
3938
import org.springframework.http.MediaType;
4039
import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest;
4140
import org.springframework.mock.http.server.reactive.test.MockServerHttpResponse;
@@ -60,9 +59,6 @@ public class ResourceHttpMessageWriterTests {
6059

6160
private Resource resource;
6261

63-
@Rule
64-
public ExpectedException expectedException = ExpectedException.none();
65-
6662

6763
@Before
6864
public void setUp() throws Exception {
@@ -84,6 +80,7 @@ public void shouldWriteResource() throws Exception {
8480

8581
assertThat(this.response.getHeaders().getContentType(), is(MediaType.TEXT_PLAIN));
8682
assertThat(this.response.getHeaders().getContentLength(), is(39L));
83+
assertThat(this.response.getHeaders().getFirst(HttpHeaders.ACCEPT_RANGES), is("bytes"));
8784

8885
Mono<String> result = reduceToString(this.response.getBody(), this.response.bufferFactory());
8986
TestSubscriber.subscribe(result).assertComplete().assertValues("Spring Framework test resource content.");
@@ -98,19 +95,22 @@ public void shouldWriteResourceRange() throws Exception {
9895

9996
assertThat(this.response.getHeaders().getContentType(), is(MediaType.TEXT_PLAIN));
10097
assertThat(this.response.getHeaders().getFirst(HttpHeaders.CONTENT_RANGE), is("bytes 0-5/39"));
98+
assertThat(this.response.getHeaders().getFirst(HttpHeaders.ACCEPT_RANGES), is("bytes"));
10199
assertThat(this.response.getHeaders().getContentLength(), is(6L));
102100

103101
Mono<String> result = reduceToString(this.response.getBody(), this.response.bufferFactory());
104102
TestSubscriber.subscribe(result).assertComplete().assertValues("Spring");
105103
}
106104

107105
@Test
108-
public void shouldThrowErrorInvalidRange() throws Exception {
106+
public void shouldSetRangeNotSatisfiableStatus() throws Exception {
109107
this.request.getHeaders().set(HttpHeaders.RANGE, "invalid");
110108

111-
this.expectedException.expect(IllegalArgumentException.class);
112109
TestSubscriber.subscribe(this.writer.write(Mono.just(resource), null, ResolvableType.forClass(Resource.class),
113110
MediaType.TEXT_PLAIN, this.request, this.response, Collections.emptyMap()));
111+
112+
assertThat(this.response.getHeaders().getFirst(HttpHeaders.ACCEPT_RANGES), is("bytes"));
113+
assertThat(this.response.getStatusCode(), is(HttpStatus.REQUESTED_RANGE_NOT_SATISFIABLE));
114114
}
115115

116116
private Mono<String> reduceToString(Publisher<DataBuffer> buffers, DataBufferFactory bufferFactory) {

0 commit comments

Comments
 (0)