Skip to content

Commit 0ef96b8

Browse files
committed
Filter out empty PartEvents in PartEventHttpMessageWriter
This commit makes sure that PartEvents with empty data buffer are filtered out before written. Empty buffers caused issues with the JdkClientHttpConnector. Closes gh-29400
1 parent 85d029f commit 0ef96b8

File tree

2 files changed

+110
-4
lines changed

2 files changed

+110
-4
lines changed

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

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import org.springframework.core.io.buffer.DataBuffer;
2929
import org.springframework.core.io.buffer.DataBufferFactory;
3030
import org.springframework.core.io.buffer.DataBufferUtils;
31+
import org.springframework.http.HttpHeaders;
3132
import org.springframework.http.MediaType;
3233
import org.springframework.http.ReactiveHttpOutputMessage;
3334
import org.springframework.http.codec.HttpMessageWriter;
@@ -83,7 +84,9 @@ public Mono<Void> write(Publisher<? extends PartEvent> partDataStream, Resolvabl
8384
if (signal.hasValue()) {
8485
PartEvent value = signal.get();
8586
Assert.state(value != null, "Null value");
86-
return encodePartData(boundary, outputMessage.bufferFactory(), value, flux);
87+
Flux<DataBuffer> dataBuffers = flux.map(PartEvent::content)
88+
.filter(buffer -> buffer.readableByteCount() > 0);
89+
return encodePartData(boundary, outputMessage.bufferFactory(), value.headers(), dataBuffers);
8790
}
8891
else {
8992
return flux.cast(DataBuffer.class);
@@ -99,11 +102,11 @@ public Mono<Void> write(Publisher<? extends PartEvent> partDataStream, Resolvabl
99102
return outputMessage.writeWith(body);
100103
}
101104

102-
private Flux<DataBuffer> encodePartData(byte[] boundary, DataBufferFactory bufferFactory, PartEvent first, Flux<? extends PartEvent> flux) {
105+
private Flux<DataBuffer> encodePartData(byte[] boundary, DataBufferFactory bufferFactory, HttpHeaders headers, Flux<DataBuffer> body) {
103106
return Flux.concat(
104107
generateBoundaryLine(boundary, bufferFactory),
105-
generatePartHeaders(first.headers(), bufferFactory),
106-
flux.map(PartEvent::content),
108+
generatePartHeaders(headers, bufferFactory),
109+
body,
107110
generateNewLine(bufferFactory));
108111
}
109112

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
/*
2+
* Copyright 2002-2020 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.http.codec.multipart;
18+
19+
import java.nio.charset.StandardCharsets;
20+
import java.time.Duration;
21+
import java.util.Collections;
22+
import java.util.Map;
23+
24+
import org.junit.jupiter.api.Test;
25+
import reactor.core.publisher.Flux;
26+
import reactor.core.publisher.Mono;
27+
28+
import org.springframework.core.ResolvableType;
29+
import org.springframework.core.codec.StringDecoder;
30+
import org.springframework.core.testfixture.io.buffer.AbstractLeakCheckingTests;
31+
import org.springframework.http.HttpHeaders;
32+
import org.springframework.http.MediaType;
33+
import org.springframework.util.MultiValueMap;
34+
import org.springframework.web.testfixture.http.server.reactive.MockServerHttpResponse;
35+
36+
import static org.assertj.core.api.Assertions.assertThat;
37+
import static org.springframework.http.codec.multipart.MultipartHttpMessageWriterTests.parse;
38+
39+
/**
40+
* Unit tests for {@link PartHttpMessageWriter}.
41+
*
42+
* @author Arjen Poutsma
43+
*/
44+
public class PartEventHttpMessageWriterTests extends AbstractLeakCheckingTests {
45+
46+
private final PartEventHttpMessageWriter writer = new PartEventHttpMessageWriter();
47+
48+
private final MockServerHttpResponse response = new MockServerHttpResponse(this.bufferFactory);
49+
50+
51+
@Test
52+
public void canWrite() {
53+
assertThat(this.writer.canWrite(ResolvableType.forClass(PartEvent.class), MediaType.MULTIPART_FORM_DATA)).isTrue();
54+
assertThat(this.writer.canWrite(ResolvableType.forClass(FilePartEvent.class), MediaType.MULTIPART_FORM_DATA)).isTrue();
55+
assertThat(this.writer.canWrite(ResolvableType.forClass(FormPartEvent.class), MediaType.MULTIPART_FORM_DATA)).isTrue();
56+
}
57+
58+
@Test
59+
void write() {
60+
HttpHeaders headers = new HttpHeaders();
61+
headers.setContentType(MediaType.TEXT_PLAIN);
62+
Mono<FormPartEvent> formPartEvent = FormPartEvent.create("text part", "text");
63+
64+
Flux<FilePartEvent> filePartEvents =
65+
FilePartEvent.create("file part", "file.txt", MediaType.APPLICATION_OCTET_STREAM,
66+
Flux.just(
67+
this.bufferFactory.wrap("Aa".getBytes(StandardCharsets.UTF_8)),
68+
this.bufferFactory.wrap("Bb".getBytes(StandardCharsets.UTF_8)),
69+
this.bufferFactory.wrap("Cc".getBytes(StandardCharsets.UTF_8))
70+
));
71+
72+
Flux<PartEvent> partEvents = Flux.concat(
73+
formPartEvent,
74+
filePartEvents
75+
);
76+
77+
Map<String, Object> hints = Collections.emptyMap();
78+
this.writer.write(partEvents, null, MediaType.MULTIPART_FORM_DATA, this.response, hints)
79+
.block(Duration.ofSeconds(5));
80+
81+
MultiValueMap<String, Part> requestParts = parse(this.response, hints);
82+
assertThat(requestParts.size()).isEqualTo(2);
83+
84+
Part part = requestParts.getFirst("text part");
85+
assertThat(part.name()).isEqualTo("text part");
86+
assertThat(part.headers().getContentType().isCompatibleWith(MediaType.TEXT_PLAIN)).isTrue();
87+
String value = decodeToString(part);
88+
assertThat(value).isEqualTo("text");
89+
90+
part = requestParts.getFirst("file part");
91+
assertThat(part.name()).isEqualTo("file part");
92+
assertThat(((FilePart) part).filename()).isEqualTo("file.txt");
93+
assertThat(decodeToString(part)).isEqualTo("AaBbCc");
94+
}
95+
96+
@SuppressWarnings("ConstantConditions")
97+
private String decodeToString(Part part) {
98+
return StringDecoder.textPlainOnly().decodeToMono(part.content(),
99+
ResolvableType.forClass(String.class), MediaType.TEXT_PLAIN,
100+
Collections.emptyMap()).block(Duration.ZERO);
101+
}
102+
103+
}

0 commit comments

Comments
 (0)