Polish byte-range resource handling

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

@@ -54,7 +54,6 @@ import org.springframework.web.reactive.HandlerMapping;
 import org.springframework.web.reactive.accept.CompositeContentTypeResolver;
 import org.springframework.web.reactive.accept.PathExtensionContentTypeResolver;
 import org.springframework.web.server.MethodNotAllowedException;
-import org.springframework.web.server.ResponseStatusException;
 import org.springframework.web.server.ServerWebExchange;
 import org.springframework.web.server.WebHandler;
 
@@ -331,6 +330,7 @@ public class ResourceWebHandler
 						// Content phase
 						if (HttpMethod.HEAD.equals(exchange.getRequest().getMethod())) {
 							setHeaders(exchange, resource, mediaType);
+							exchange.getResponse().getHeaders().set(HttpHeaders.ACCEPT_RANGES, "bytes");
 							logger.trace("HEAD request - skipping content");
 							return Mono.empty();
 						}
@@ -340,7 +340,7 @@ public class ResourceWebHandler
 								null, ResolvableType.forClass(Resource.class), mediaType,
 								exchange.getRequest(), exchange.getResponse(), Collections.emptyMap());
 					}
-					catch (IOException|ResponseStatusException ex) {
+					catch (IOException ex) {
 						return Mono.error(ex);
 					}
 				});
@@ -503,7 +503,6 @@ public class ResourceWebHandler
 			HttpHeaders resourceHeaders = ((HttpResource) resource).getResponseHeaders();
 			exchange.getResponse().getHeaders().putAll(resourceHeaders);
 		}
-		headers.set(HttpHeaders.ACCEPT_RANGES, "bytes");
 	}
 
 

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

@@ -525,10 +525,10 @@ public class ResourceWebHandlerTests {
 		this.request.addHeader("Range", "bytes= foo bar");
 		this.exchange.getAttributes().put(PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, "foo.txt");
 
-		TestSubscriber.subscribe(this.handler.handle(this.exchange))
-				.assertErrorWith(throwable -> {
-					assertThat(throwable, instanceOf(IllegalArgumentException.class));
-				});
+		TestSubscriber.subscribe(this.handler.handle(this.exchange)).assertComplete();
+
+		assertEquals(HttpStatus.REQUESTED_RANGE_NOT_SATISFIABLE, this.response.getStatusCode());
+		assertEquals("bytes", this.response.getHeaders().getFirst("Accept-Ranges"));
 	}
 
 	@Test

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

@@ -104,24 +104,30 @@ public class ResourceHttpMessageWriter extends AbstractServerHttpMessageWriter<R
 	public Mono<Void> write(Publisher<? extends Resource> inputStream, ResolvableType streamType,
 			ResolvableType elementType, MediaType mediaType, ServerHttpRequest request,
 			ServerHttpResponse response, Map<String, Object> hints) {
-
-		Map<String, Object> mergedHints = new HashMap<>(hints);
-		mergedHints.putAll(resolveWriteHints(streamType, elementType, mediaType, request));
-		if (mergedHints.containsKey(HTTP_RANGE_REQUEST_HINT)) {
-			response.setStatusCode(HttpStatus.PARTIAL_CONTENT);
-			List<HttpRange> httpRanges = (List<HttpRange>) mergedHints.get(HTTP_RANGE_REQUEST_HINT);
-			if (httpRanges.size() > 1) {
-				final String boundary = MimeTypeUtils.generateMultipartBoundaryString();
-				mergedHints.put(ResourceRegionHttpMessageWriter.BOUNDARY_STRING_HINT, boundary);
+		try {
+			response.getHeaders().set(HttpHeaders.ACCEPT_RANGES, "bytes");
+			Map<String, Object> mergedHints = new HashMap<>(hints);
+			mergedHints.putAll(resolveWriteHints(streamType, elementType, mediaType, request));
+			if (mergedHints.containsKey(HTTP_RANGE_REQUEST_HINT)) {
+				response.setStatusCode(HttpStatus.PARTIAL_CONTENT);
+				List<HttpRange> httpRanges = (List<HttpRange>) mergedHints.get(HTTP_RANGE_REQUEST_HINT);
+				if (httpRanges.size() > 1) {
+					final String boundary = MimeTypeUtils.generateMultipartBoundaryString();
+					mergedHints.put(ResourceRegionHttpMessageWriter.BOUNDARY_STRING_HINT, boundary);
+				}
+				Flux<ResourceRegion> regions = Flux.from(inputStream)
+						.flatMap(resource -> Flux.fromIterable(HttpRange.toResourceRegions(httpRanges, resource)));
+
+				return this.resourceRegionHttpMessageWriter
+						.write(regions, ResolvableType.forClass(ResourceRegion.class), mediaType, response, mergedHints);
+			}
+			else {
+				return write(inputStream, elementType, mediaType, response, mergedHints);
 			}
-			Flux<ResourceRegion> regions = Flux.from(inputStream)
-					.flatMap(resource -> Flux.fromIterable(HttpRange.toResourceRegions(httpRanges, resource)));
-
-			return this.resourceRegionHttpMessageWriter
-					.write(regions, ResolvableType.forClass(ResourceRegion.class), mediaType, response, mergedHints);
 		}
-		else {
-			return write(inputStream, elementType, mediaType, response, mergedHints);
+		catch (IllegalArgumentException exc) {
+			response.setStatusCode(HttpStatus.REQUESTED_RANGE_NOT_SATISFIABLE);
+			return response.setComplete();
 		}
 	}
 

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

@@ -20,9 +20,7 @@ import java.nio.charset.StandardCharsets;
 import java.util.Collections;
 
 import org.junit.Before;
-import org.junit.Rule;
 import org.junit.Test;
-import org.junit.rules.ExpectedException;
 import org.reactivestreams.Publisher;
 import reactor.core.publisher.Flux;
 import reactor.core.publisher.Mono;
@@ -36,6 +34,7 @@ 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;
@@ -60,9 +59,6 @@ public class ResourceHttpMessageWriterTests {
 
 	private Resource resource;
 
-	@Rule
-	public ExpectedException expectedException = ExpectedException.none();
-
 
 	@Before
 	public void setUp() throws Exception {
@@ -84,6 +80,7 @@ public class ResourceHttpMessageWriterTests {
 
 		assertThat(this.response.getHeaders().getContentType(), is(MediaType.TEXT_PLAIN));
 		assertThat(this.response.getHeaders().getContentLength(), is(39L));
+		assertThat(this.response.getHeaders().getFirst(HttpHeaders.ACCEPT_RANGES), is("bytes"));
 
 		Mono<String> result = reduceToString(this.response.getBody(), this.response.bufferFactory());
 		TestSubscriber.subscribe(result).assertComplete().assertValues("Spring Framework test resource content.");
@@ -98,6 +95,7 @@ public class ResourceHttpMessageWriterTests {
 
 		assertThat(this.response.getHeaders().getContentType(), is(MediaType.TEXT_PLAIN));
 		assertThat(this.response.getHeaders().getFirst(HttpHeaders.CONTENT_RANGE), is("bytes 0-5/39"));
+		assertThat(this.response.getHeaders().getFirst(HttpHeaders.ACCEPT_RANGES), is("bytes"));
 		assertThat(this.response.getHeaders().getContentLength(), is(6L));
 
 		Mono<String> result = reduceToString(this.response.getBody(), this.response.bufferFactory());
@@ -105,12 +103,14 @@ public class ResourceHttpMessageWriterTests {
 	}
 
 	@Test
-	public void shouldThrowErrorInvalidRange() throws Exception {
+	public void shouldSetRangeNotSatisfiableStatus() throws Exception {
 		this.request.getHeaders().set(HttpHeaders.RANGE, "invalid");
 
-		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()));
+
+		assertThat(this.response.getHeaders().getFirst(HttpHeaders.ACCEPT_RANGES), is("bytes"));
+		assertThat(this.response.getStatusCode(), is(HttpStatus.REQUESTED_RANGE_NOT_SATISFIABLE));
 	}
 
 	private Mono<String> reduceToString(Publisher<DataBuffer> buffers, DataBufferFactory bufferFactory) {