Skip to content

Commit 51bb96d

Browse files
poutsmapoutsma
committed
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
1 parent eac9e66 commit 51bb96d

File tree

2 files changed

+70
-54
lines changed

2 files changed

+70
-54
lines changed

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

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -86,32 +86,45 @@ public Flux<DataBuffer> encode(Publisher<? extends ResourceRegion> inputStream,
8686
Assert.notNull(elementType, "'elementType' must not be null");
8787

8888
if (inputStream instanceof Mono) {
89-
return ((Mono<? extends ResourceRegion>) inputStream)
90-
.flatMapMany(region -> writeResourceRegion(region, bufferFactory, hints));
89+
return Mono.from(inputStream)
90+
.flatMapMany(region -> {
91+
if (!region.getResource().isReadable()) {
92+
return Flux.error(new EncodingException("Resource " +
93+
region.getResource() + " is not readable"));
94+
}
95+
96+
return writeResourceRegion(region, bufferFactory, hints);
97+
});
9198
}
9299
else {
93100
final String boundaryString = Hints.getRequiredHint(hints, BOUNDARY_STRING_HINT);
94101
byte[] startBoundary = getAsciiBytes("\r\n--" + boundaryString + "\r\n");
95102
byte[] contentType =
96103
(mimeType != null ? getAsciiBytes("Content-Type: " + mimeType + "\r\n") : new byte[0]);
97104

98-
Flux<DataBuffer> regions = Flux.from(inputStream).
99-
concatMap(region ->
100-
Flux.concat(
105+
return Flux.from(inputStream).
106+
concatMap(region -> {
107+
if (!region.getResource().isReadable()) {
108+
return Flux.error(new EncodingException("Resource " +
109+
region.getResource() + " is not readable"));
110+
}
111+
else {
112+
return Flux.concat(
101113
getRegionPrefix(bufferFactory, startBoundary, contentType, region),
102-
writeResourceRegion(region, bufferFactory, hints)
103-
));
104-
return Flux.concat(regions, getRegionSuffix(bufferFactory, boundaryString));
114+
writeResourceRegion(region, bufferFactory, hints));
115+
}
116+
})
117+
.concatWith(getRegionSuffix(bufferFactory, boundaryString));
105118
}
106119
}
107120

108121
private Flux<DataBuffer> getRegionPrefix(DataBufferFactory bufferFactory, byte[] startBoundary,
109122
byte[] contentType, ResourceRegion region) {
110123

111-
return Flux.just(
124+
return Flux.defer(() -> Flux.just(
112125
bufferFactory.allocateBuffer(startBoundary.length).write(startBoundary),
113126
bufferFactory.allocateBuffer(contentType.length).write(contentType),
114-
bufferFactory.wrap(ByteBuffer.wrap(getContentRangeHeader(region)))
127+
bufferFactory.wrap(ByteBuffer.wrap(getContentRangeHeader(region))))
115128
);
116129
}
117130

@@ -133,7 +146,8 @@ private Flux<DataBuffer> writeResourceRegion(
133146

134147
private Flux<DataBuffer> getRegionSuffix(DataBufferFactory bufferFactory, String boundaryString) {
135148
byte[] endBoundary = getAsciiBytes("\r\n--" + boundaryString + "--");
136-
return Flux.just(bufferFactory.allocateBuffer(endBoundary.length).write(endBoundary));
149+
return Flux.defer(() -> Flux.just(
150+
bufferFactory.allocateBuffer(endBoundary.length).write(endBoundary)));
137151
}
138152

139153
private byte[] getAsciiBytes(String in) {

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

Lines changed: 45 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2017 the original author or authors.
2+
* Copyright 2002-2018 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -31,17 +31,11 @@
3131
import org.springframework.core.io.Resource;
3232
import org.springframework.core.io.buffer.AbstractDataBufferAllocatingTestCase;
3333
import org.springframework.core.io.buffer.DataBuffer;
34-
import org.springframework.core.io.buffer.DataBufferUtils;
35-
import org.springframework.core.io.buffer.DefaultDataBufferFactory;
36-
import org.springframework.core.io.buffer.support.DataBufferTestUtils;
3734
import org.springframework.core.io.support.ResourceRegion;
3835
import org.springframework.util.MimeType;
3936
import org.springframework.util.MimeTypeUtils;
40-
import org.springframework.util.StringUtils;
4137

42-
import static org.junit.Assert.assertArrayEquals;
43-
import static org.junit.Assert.assertFalse;
44-
import static org.junit.Assert.assertTrue;
38+
import static org.junit.Assert.*;
4539

4640
/**
4741
* Test cases for {@link ResourceRegionEncoder} class.
@@ -55,7 +49,6 @@ public class ResourceRegionEncoderTests extends AbstractDataBufferAllocatingTest
5549
@Before
5650
public void setUp() {
5751
this.encoder = new ResourceRegionEncoder();
58-
this.bufferFactory = new DefaultDataBufferFactory();
5952
}
6053

6154
@Test
@@ -111,6 +104,30 @@ public void shouldEncodeMultipleResourceRegionsByteArrayResource() throws Except
111104
new ByteArrayResource(content.getBytes(StandardCharsets.UTF_8)));
112105
}
113106

107+
@Test
108+
public void nonExisting() {
109+
Resource resource = new ClassPathResource("ResourceRegionEncoderTests.txt", getClass());
110+
Resource nonExisting = new ClassPathResource("does not exist", getClass());
111+
Flux<ResourceRegion> regions = Flux.just(
112+
new ResourceRegion(resource, 0, 6),
113+
new ResourceRegion(nonExisting, 0, 6));
114+
115+
String boundary = MimeTypeUtils.generateMultipartBoundaryString();
116+
117+
Flux<DataBuffer> result = this.encoder.encode(regions, this.bufferFactory,
118+
ResolvableType.forClass(ResourceRegion.class),
119+
MimeType.valueOf("text/plain"),
120+
Collections.singletonMap(ResourceRegionEncoder.BOUNDARY_STRING_HINT, boundary));
121+
122+
StepVerifier.create(result)
123+
.consumeNextWith(stringConsumer("\r\n--" + boundary + "\r\n"))
124+
.consumeNextWith(stringConsumer("Content-Type: text/plain\r\n"))
125+
.consumeNextWith(stringConsumer("Content-Range: bytes 0-5/39\r\n\r\n"))
126+
.consumeNextWith(stringConsumer("Spring"))
127+
.expectError(EncodingException.class)
128+
.verify();
129+
}
130+
114131
private void shouldEncodeMultipleResourceRegions(Resource resource) {
115132
Flux<ResourceRegion> regions = Flux.just(
116133
new ResourceRegion(resource, 0, 6),
@@ -120,45 +137,30 @@ private void shouldEncodeMultipleResourceRegions(Resource resource) {
120137
);
121138
String boundary = MimeTypeUtils.generateMultipartBoundaryString();
122139

123-
Flux<DataBuffer> result = this.encoder.encode(regions, this.bufferFactory,
140+
Flux<DataBuffer> result = this.encoder.encode(regions, super.bufferFactory,
124141
ResolvableType.forClass(ResourceRegion.class),
125142
MimeType.valueOf("text/plain"),
126143
Collections.singletonMap(ResourceRegionEncoder.BOUNDARY_STRING_HINT, boundary)
127144
);
128145

129-
Mono<DataBuffer> reduced = result
130-
.reduce(bufferFactory.allocateBuffer(), (previous, current) -> {
131-
previous.write(current);
132-
DataBufferUtils.release(current);
133-
return previous;
134-
});
135-
136-
StepVerifier.create(reduced)
137-
.consumeNextWith(buf -> {
138-
String content = DataBufferTestUtils.dumpString(buf, StandardCharsets.UTF_8);
139-
String[] ranges = StringUtils.tokenizeToStringArray(content, "\r\n",
140-
false, true);
141-
String[] expected = new String[] {
142-
"--" + boundary,
143-
"Content-Type: text/plain",
144-
"Content-Range: bytes 0-5/39",
145-
"Spring",
146-
"--" + boundary,
147-
"Content-Type: text/plain",
148-
"Content-Range: bytes 7-15/39",
149-
"Framework",
150-
"--" + boundary,
151-
"Content-Type: text/plain",
152-
"Content-Range: bytes 17-20/39",
153-
"test",
154-
"--" + boundary,
155-
"Content-Type: text/plain",
156-
"Content-Range: bytes 22-38/39",
157-
"resource content.",
158-
"--" + boundary + "--"
159-
};
160-
assertArrayEquals(expected, ranges);
161-
})
146+
StepVerifier.create(result)
147+
.consumeNextWith(stringConsumer("\r\n--" + boundary + "\r\n"))
148+
.consumeNextWith(stringConsumer("Content-Type: text/plain\r\n"))
149+
.consumeNextWith(stringConsumer("Content-Range: bytes 0-5/39\r\n\r\n"))
150+
.consumeNextWith(stringConsumer("Spring"))
151+
.consumeNextWith(stringConsumer("\r\n--" + boundary + "\r\n"))
152+
.consumeNextWith(stringConsumer("Content-Type: text/plain\r\n"))
153+
.consumeNextWith(stringConsumer("Content-Range: bytes 7-15/39\r\n\r\n"))
154+
.consumeNextWith(stringConsumer("Framework"))
155+
.consumeNextWith(stringConsumer("\r\n--" + boundary + "\r\n"))
156+
.consumeNextWith(stringConsumer("Content-Type: text/plain\r\n"))
157+
.consumeNextWith(stringConsumer("Content-Range: bytes 17-20/39\r\n\r\n"))
158+
.consumeNextWith(stringConsumer("test"))
159+
.consumeNextWith(stringConsumer("\r\n--" + boundary + "\r\n"))
160+
.consumeNextWith(stringConsumer("Content-Type: text/plain\r\n"))
161+
.consumeNextWith(stringConsumer("Content-Range: bytes 22-38/39\r\n\r\n"))
162+
.consumeNextWith(stringConsumer("resource content."))
163+
.consumeNextWith(stringConsumer("\r\n--" + boundary + "--"))
162164
.expectComplete()
163165
.verify();
164166
}

0 commit comments

Comments
 (0)