Skip to content

Commit 5342c6e

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 (Cherry-picked from 3203d39)
1 parent 18da718 commit 5342c6e

File tree

3 files changed

+15
-4
lines changed

3 files changed

+15
-4
lines changed

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
import org.springframework.core.ResolvableType;
2929
import org.springframework.core.codec.Encoder;
3030
import org.springframework.core.io.buffer.DataBuffer;
31-
import org.springframework.core.io.buffer.DataBufferFactory;
3231
import org.springframework.http.HttpHeaders;
3332
import org.springframework.http.MediaType;
3433
import org.springframework.http.ReactiveHttpOutputMessage;
@@ -106,7 +105,7 @@ public Mono<Void> write(Publisher<? extends T> inputStream, ResolvableType eleme
106105

107106
if (inputStream instanceof Mono) {
108107
HttpHeaders headers = message.getHeaders();
109-
if (headers.getContentLength() < 0 && !headers.containsKey(HttpHeaders.TRANSFER_ENCODING)) {
108+
if (!headers.containsKey(HttpHeaders.TRANSFER_ENCODING)) {
110109
return Mono.from(body)
111110
.defaultIfEmpty(message.bufferFactory().wrap(new byte[0]))
112111
.flatMap(buffer -> {

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 {
@@ -173,13 +174,21 @@ public boolean isCommitted() {
173174
@Override
174175
public final Mono<Void> writeWith(Publisher<? extends DataBuffer> body) {
175176
return new ChannelSendOperator<>(body,
176-
writePublisher -> doCommit(() -> writeWithInternal(writePublisher)));
177+
writePublisher -> doCommit(() -> writeWithInternal(writePublisher)))
178+
.doOnError(t -> removeContentLength());
177179
}
178180

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

185194
@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)