Skip to content

Commit 3203d39

Browse files
committed
Remove Content-Length response header from errors
Prior to this commit, when errors happened before the response was committed, the `Content-Length` response header would be left as is. This can be problematic since the error can be handled later in the chain and the response body changed accordingly. For example, Spring Boot renders error pages in those cases. If the `Content-Length` is set, HTTP clients can get confused and only consider part of the error response body. This commit ensures that any `Content-Length` response header is removed in case of errors, if the response is not already committed. This is done at the `AbstractServerHttpResponse` level, since errors can be handled in multiple places and the response itself is the safest place to handle this case. As a consequence, this commit also removes `Content-Length` checks in `EncoderHttpMessageWriter` since we now consider that we should rely on the response body we're about to write rather than any previously set value. Issue: SPR-17502
1 parent ce5c65c commit 3203d39

File tree

3 files changed

+20
-10
lines changed

3 files changed

+20
-10
lines changed

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

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -118,14 +118,12 @@ public Mono<Void> write(Publisher<? extends T> inputStream, ResolvableType eleme
118118

119119
if (inputStream instanceof Mono) {
120120
HttpHeaders headers = message.getHeaders();
121-
if (headers.getFirst(HttpHeaders.CONTENT_LENGTH) == null) {
122-
return Mono.from(body)
123-
.defaultIfEmpty(message.bufferFactory().wrap(new byte[0]))
124-
.flatMap(buffer -> {
125-
headers.setContentLength(buffer.readableByteCount());
126-
return message.writeWith(Mono.just(buffer));
127-
});
128-
}
121+
return Mono.from(body)
122+
.defaultIfEmpty(message.bufferFactory().wrap(new byte[0]))
123+
.flatMap(buffer -> {
124+
headers.setContentLength(buffer.readableByteCount());
125+
return message.writeWith(Mono.just(buffer));
126+
});
129127
}
130128

131129
return (isStreamingMediaType(contentType) ?

spring-web/src/main/java/org/springframework/http/server/reactive/AbstractServerHttpResponse.java

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
* @author Rossen Stoyanchev
4646
* @author Juergen Hoeller
4747
* @author Sebastien Deleuze
48+
* @author Brian Clozel
4849
* @since 5.0
4950
*/
5051
public abstract class AbstractServerHttpResponse implements ServerHttpResponse {
@@ -174,13 +175,21 @@ public boolean isCommitted() {
174175
@Override
175176
public final Mono<Void> writeWith(Publisher<? extends DataBuffer> body) {
176177
return new ChannelSendOperator<>(body,
177-
writePublisher -> doCommit(() -> writeWithInternal(writePublisher)));
178+
writePublisher -> doCommit(() -> writeWithInternal(writePublisher)))
179+
.doOnError(t -> removeContentLength());
178180
}
179181

180182
@Override
181183
public final Mono<Void> writeAndFlushWith(Publisher<? extends Publisher<? extends DataBuffer>> body) {
182184
return new ChannelSendOperator<>(body,
183-
writePublisher -> doCommit(() -> writeAndFlushWithInternal(writePublisher)));
185+
writePublisher -> doCommit(() -> writeAndFlushWithInternal(writePublisher)))
186+
.doOnError(t -> removeContentLength());
187+
}
188+
189+
private void removeContentLength() {
190+
if (!this.isCommitted()) {
191+
this.getHeaders().remove(HttpHeaders.CONTENT_LENGTH);
192+
}
184193
}
185194

186195
@Override

spring-web/src/test/java/org/springframework/http/server/reactive/ServerHttpResponseTests.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import org.springframework.core.io.buffer.DataBuffer;
3030
import org.springframework.core.io.buffer.DefaultDataBuffer;
3131
import org.springframework.core.io.buffer.DefaultDataBufferFactory;
32+
import org.springframework.http.HttpHeaders;
3233
import org.springframework.http.ResponseCookie;
3334

3435
import static junit.framework.TestCase.assertTrue;
@@ -75,12 +76,14 @@ public void writeAndFlushWithFluxOfDefaultDataBuffer() throws Exception {
7576
@Test
7677
public void writeWithError() throws Exception {
7778
TestServerHttpResponse response = new TestServerHttpResponse();
79+
response.getHeaders().setContentLength(12);
7880
IllegalStateException error = new IllegalStateException("boo");
7981
response.writeWith(Flux.error(error)).onErrorResume(ex -> Mono.empty()).block();
8082

8183
assertFalse(response.statusCodeWritten);
8284
assertFalse(response.headersWritten);
8385
assertFalse(response.cookiesWritten);
86+
assertFalse(response.getHeaders().containsKey(HttpHeaders.CONTENT_LENGTH));
8487
assertTrue(response.body.isEmpty());
8588
}
8689

0 commit comments

Comments
 (0)