Browse Source

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 3203d39821)
pull/23430/head
Brian Clozel 6 years ago
parent
commit
5342c6e493
  1. 3
      spring-web/src/main/java/org/springframework/http/codec/EncoderHttpMessageWriter.java
  2. 13
      spring-web/src/main/java/org/springframework/http/server/reactive/AbstractServerHttpResponse.java
  3. 3
      spring-web/src/test/java/org/springframework/http/server/reactive/ServerHttpResponseTests.java

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

@ -28,7 +28,6 @@ import reactor.core.publisher.Mono; @@ -28,7 +28,6 @@ import reactor.core.publisher.Mono;
import org.springframework.core.ResolvableType;
import org.springframework.core.codec.Encoder;
import org.springframework.core.io.buffer.DataBuffer;
import org.springframework.core.io.buffer.DataBufferFactory;
import org.springframework.http.HttpHeaders;
import org.springframework.http.MediaType;
import org.springframework.http.ReactiveHttpOutputMessage;
@ -106,7 +105,7 @@ public class EncoderHttpMessageWriter<T> implements HttpMessageWriter<T> { @@ -106,7 +105,7 @@ public class EncoderHttpMessageWriter<T> implements HttpMessageWriter<T> {
if (inputStream instanceof Mono) {
HttpHeaders headers = message.getHeaders();
if (headers.getContentLength() < 0 && !headers.containsKey(HttpHeaders.TRANSFER_ENCODING)) {
if (!headers.containsKey(HttpHeaders.TRANSFER_ENCODING)) {
return Mono.from(body)
.defaultIfEmpty(message.bufferFactory().wrap(new byte[0]))
.flatMap(buffer -> {

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

@ -45,6 +45,7 @@ import org.springframework.util.MultiValueMap; @@ -45,6 +45,7 @@ import org.springframework.util.MultiValueMap;
* @author Rossen Stoyanchev
* @author Juergen Hoeller
* @author Sebastien Deleuze
* @author Brian Clozel
* @since 5.0
*/
public abstract class AbstractServerHttpResponse implements ServerHttpResponse {
@ -173,13 +174,21 @@ public abstract class AbstractServerHttpResponse implements ServerHttpResponse { @@ -173,13 +174,21 @@ public abstract class AbstractServerHttpResponse implements ServerHttpResponse {
@Override
public final Mono<Void> writeWith(Publisher<? extends DataBuffer> body) {
return new ChannelSendOperator<>(body,
writePublisher -> doCommit(() -> writeWithInternal(writePublisher)));
writePublisher -> doCommit(() -> writeWithInternal(writePublisher)))
.doOnError(t -> removeContentLength());
}
@Override
public final Mono<Void> writeAndFlushWith(Publisher<? extends Publisher<? extends DataBuffer>> body) {
return new ChannelSendOperator<>(body,
writePublisher -> doCommit(() -> writeAndFlushWithInternal(writePublisher)));
writePublisher -> doCommit(() -> writeAndFlushWithInternal(writePublisher)))
.doOnError(t -> removeContentLength());
}
private void removeContentLength() {
if (!this.isCommitted()) {
this.getHeaders().remove(HttpHeaders.CONTENT_LENGTH);
}
}
@Override

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

@ -29,6 +29,7 @@ import reactor.core.publisher.Mono; @@ -29,6 +29,7 @@ import reactor.core.publisher.Mono;
import org.springframework.core.io.buffer.DataBuffer;
import org.springframework.core.io.buffer.DefaultDataBuffer;
import org.springframework.core.io.buffer.DefaultDataBufferFactory;
import org.springframework.http.HttpHeaders;
import org.springframework.http.ResponseCookie;
import static junit.framework.TestCase.assertTrue;
@ -75,12 +76,14 @@ public class ServerHttpResponseTests { @@ -75,12 +76,14 @@ public class ServerHttpResponseTests {
@Test
public void writeWithError() throws Exception {
TestServerHttpResponse response = new TestServerHttpResponse();
response.getHeaders().setContentLength(12);
IllegalStateException error = new IllegalStateException("boo");
response.writeWith(Flux.error(error)).onErrorResume(ex -> Mono.empty()).block();
assertFalse(response.statusCodeWritten);
assertFalse(response.headersWritten);
assertFalse(response.cookiesWritten);
assertFalse(response.getHeaders().containsKey(HttpHeaders.CONTENT_LENGTH));
assertTrue(response.body.isEmpty());
}

Loading…
Cancel
Save