Fix ResourceRegionEncoder and tests

Fix ResourceRegionEncoder so that it checks for resource existance
before writing boundaries. Also defer data buffer allocation until
necessary.

Issue: SPR-17419

spring-core/src/main/java/org/springframework/core/codec/ResourceRegionEncoder.java

@@ -86,8 +86,15 @@ public class ResourceRegionEncoder extends AbstractEncoder<ResourceRegion> {
 		Assert.notNull(elementType, "'elementType' must not be null");
 
 		if (inputStream instanceof Mono) {
-			return ((Mono<? extends ResourceRegion>) inputStream)
+			return Mono.from(inputStream)
-					.flatMapMany(region -> writeResourceRegion(region, bufferFactory, hints));
+					.flatMapMany(region -> {
+						if (!region.getResource().isReadable()) {
+							return Flux.error(new EncodingException("Resource " +
+									region.getResource() + " is not readable"));
+						}
+
+						return writeResourceRegion(region, bufferFactory, hints);
+					});
 		}
 		else {
 			final String boundaryString = Hints.getRequiredHint(hints, BOUNDARY_STRING_HINT);
@@ -95,23 +102,29 @@ public class ResourceRegionEncoder extends AbstractEncoder<ResourceRegion> {
 			byte[] contentType =
 					(mimeType != null ? getAsciiBytes("Content-Type: " + mimeType + "\r\n") : new byte[0]);
 
-			Flux<DataBuffer> regions = Flux.from(inputStream).
+			return Flux.from(inputStream).
-					concatMap(region ->
+					concatMap(region -> {
-							Flux.concat(
+						if (!region.getResource().isReadable()) {
+							return Flux.error(new EncodingException("Resource " +
+									region.getResource() + " is not readable"));
+						}
+						else {
+							return Flux.concat(
 									getRegionPrefix(bufferFactory, startBoundary, contentType, region),
-									writeResourceRegion(region, bufferFactory, hints)
+									writeResourceRegion(region, bufferFactory, hints));
-							));
+						}
-			return Flux.concat(regions, getRegionSuffix(bufferFactory, boundaryString));
+					})
+					.concatWith(getRegionSuffix(bufferFactory, boundaryString));
 		}
 	}
 
 	private Flux<DataBuffer> getRegionPrefix(DataBufferFactory bufferFactory, byte[] startBoundary,
 			byte[] contentType, ResourceRegion region) {
 
-		return Flux.just(
+		return Flux.defer(() -> Flux.just(
 				bufferFactory.allocateBuffer(startBoundary.length).write(startBoundary),
 				bufferFactory.allocateBuffer(contentType.length).write(contentType),
-				bufferFactory.wrap(ByteBuffer.wrap(getContentRangeHeader(region)))
+				bufferFactory.wrap(ByteBuffer.wrap(getContentRangeHeader(region))))
 		);
 	}
 
@@ -133,7 +146,8 @@ public class ResourceRegionEncoder extends AbstractEncoder<ResourceRegion> {
 
 	private Flux<DataBuffer> getRegionSuffix(DataBufferFactory bufferFactory, String boundaryString) {
 		byte[] endBoundary = getAsciiBytes("\r\n--" + boundaryString + "--");
-		return Flux.just(bufferFactory.allocateBuffer(endBoundary.length).write(endBoundary));
+		return Flux.defer(() -> Flux.just(
+				bufferFactory.allocateBuffer(endBoundary.length).write(endBoundary)));
 	}
 
 	private byte[] getAsciiBytes(String in) {

spring-core/src/test/java/org/springframework/core/codec/ResourceRegionEncoderTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -31,17 +31,11 @@ import org.springframework.core.io.ClassPathResource;
 import org.springframework.core.io.Resource;
 import org.springframework.core.io.buffer.AbstractDataBufferAllocatingTestCase;
 import org.springframework.core.io.buffer.DataBuffer;
-import org.springframework.core.io.buffer.DataBufferUtils;
-import org.springframework.core.io.buffer.DefaultDataBufferFactory;
-import org.springframework.core.io.buffer.support.DataBufferTestUtils;
 import org.springframework.core.io.support.ResourceRegion;
 import org.springframework.util.MimeType;
 import org.springframework.util.MimeTypeUtils;
-import org.springframework.util.StringUtils;
 
-import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.*;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
 
 /**
  * Test cases for {@link ResourceRegionEncoder} class.
@@ -55,7 +49,6 @@ public class ResourceRegionEncoderTests extends AbstractDataBufferAllocatingTest
 	@Before
 	public void setUp() {
 		this.encoder = new ResourceRegionEncoder();
-		this.bufferFactory = new DefaultDataBufferFactory();
 	}
 
 	@Test
@@ -111,6 +104,30 @@ public class ResourceRegionEncoderTests extends AbstractDataBufferAllocatingTest
 				new ByteArrayResource(content.getBytes(StandardCharsets.UTF_8)));
 	}
 
+	@Test
+	public void nonExisting() {
+		Resource resource = new ClassPathResource("ResourceRegionEncoderTests.txt", getClass());
+		Resource nonExisting = new ClassPathResource("does not exist", getClass());
+		Flux<ResourceRegion> regions = Flux.just(
+				new ResourceRegion(resource, 0, 6),
+				new ResourceRegion(nonExisting, 0, 6));
+
+		String boundary = MimeTypeUtils.generateMultipartBoundaryString();
+
+		Flux<DataBuffer> result = this.encoder.encode(regions, this.bufferFactory,
+				ResolvableType.forClass(ResourceRegion.class),
+				MimeType.valueOf("text/plain"),
+				Collections.singletonMap(ResourceRegionEncoder.BOUNDARY_STRING_HINT, boundary));
+
+		StepVerifier.create(result)
+				.consumeNextWith(stringConsumer("\r\n--" + boundary + "\r\n"))
+				.consumeNextWith(stringConsumer("Content-Type: text/plain\r\n"))
+				.consumeNextWith(stringConsumer("Content-Range: bytes 0-5/39\r\n\r\n"))
+				.consumeNextWith(stringConsumer("Spring"))
+				.expectError(EncodingException.class)
+				.verify();
+	}
+
 	private void shouldEncodeMultipleResourceRegions(Resource resource) {
 		Flux<ResourceRegion> regions = Flux.just(
 				new ResourceRegion(resource, 0, 6),
@@ -120,45 +137,30 @@ public class ResourceRegionEncoderTests extends AbstractDataBufferAllocatingTest
 		);
 		String boundary = MimeTypeUtils.generateMultipartBoundaryString();
 
-		Flux<DataBuffer> result = this.encoder.encode(regions, this.bufferFactory,
+		Flux<DataBuffer> result = this.encoder.encode(regions, super.bufferFactory,
 				ResolvableType.forClass(ResourceRegion.class),
 				MimeType.valueOf("text/plain"),
 				Collections.singletonMap(ResourceRegionEncoder.BOUNDARY_STRING_HINT, boundary)
 		);
 
-		Mono<DataBuffer> reduced = result
+		StepVerifier.create(result)
-				.reduce(bufferFactory.allocateBuffer(), (previous, current) -> {
+				.consumeNextWith(stringConsumer("\r\n--" + boundary + "\r\n"))
-					previous.write(current);
+				.consumeNextWith(stringConsumer("Content-Type: text/plain\r\n"))
-					DataBufferUtils.release(current);
+				.consumeNextWith(stringConsumer("Content-Range: bytes 0-5/39\r\n\r\n"))
-					return previous;
+				.consumeNextWith(stringConsumer("Spring"))
-				});
+				.consumeNextWith(stringConsumer("\r\n--" + boundary + "\r\n"))
-
+				.consumeNextWith(stringConsumer("Content-Type: text/plain\r\n"))
-		StepVerifier.create(reduced)
+				.consumeNextWith(stringConsumer("Content-Range: bytes 7-15/39\r\n\r\n"))
-				.consumeNextWith(buf -> {
+				.consumeNextWith(stringConsumer("Framework"))
-					String content = DataBufferTestUtils.dumpString(buf, StandardCharsets.UTF_8);
+				.consumeNextWith(stringConsumer("\r\n--" + boundary + "\r\n"))
-					String[] ranges = StringUtils.tokenizeToStringArray(content, "\r\n",
+				.consumeNextWith(stringConsumer("Content-Type: text/plain\r\n"))
-							false, true);
+				.consumeNextWith(stringConsumer("Content-Range: bytes 17-20/39\r\n\r\n"))
-					String[] expected = new String[] {
+				.consumeNextWith(stringConsumer("test"))
-							"--" + boundary,
+				.consumeNextWith(stringConsumer("\r\n--" + boundary + "\r\n"))
-							"Content-Type: text/plain",
+				.consumeNextWith(stringConsumer("Content-Type: text/plain\r\n"))
-							"Content-Range: bytes 0-5/39",
+				.consumeNextWith(stringConsumer("Content-Range: bytes 22-38/39\r\n\r\n"))
-							"Spring",
+				.consumeNextWith(stringConsumer("resource content."))
-							"--" + boundary,
+				.consumeNextWith(stringConsumer("\r\n--" + boundary + "--"))
-							"Content-Type: text/plain",
-							"Content-Range: bytes 7-15/39",
-							"Framework",
-							"--" + boundary,
-							"Content-Type: text/plain",
-							"Content-Range: bytes 17-20/39",
-							"test",
-							"--" + boundary,
-							"Content-Type: text/plain",
-							"Content-Range: bytes 22-38/39",
-							"resource content.",
-							"--" + boundary + "--"
-					};
-					assertArrayEquals(expected, ranges);
-				})
 				.expectComplete()
 				.verify();
 	}