Skip to content

Commit

Permalink
Remove Content-* response headers for error handling
Browse files Browse the repository at this point in the history
Prior to this commit, when WebFlux handlers added `"Content-*"` response
headers and an error happened while handling the request, all those
headers would not be cleared from the response before error handling.

This commit clears those headers from the response in two places:
* when invoking the handler and adapting the response
* when writing the response body

Not removing those headers might break HTTP clients since they're given
wrong information about how to interpret the HTTP response body: the
error response body might be very different from the original one.

Fixes gh-24238
  • Loading branch information
bclozel committed Jan 6, 2020
1 parent 59ade91 commit f9c1565
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 21 deletions.
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2020 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -1827,6 +1827,22 @@ public static String encodeBasicAuth(String username, String password, @Nullable
return new String(encodedBytes, charset);
}

/**
* Remove the well-known {@code "Content-*"} HTTP headers from the given instance.
* <p>Such headers should be cleared, if possible, from the response if the intended
* body can't be written due to errors.
* @since 5.2.3
*/
public static void clearContentHeaders(HttpHeaders headers) {
headers.remove(HttpHeaders.CONTENT_DISPOSITION);
headers.remove(HttpHeaders.CONTENT_ENCODING);
headers.remove(HttpHeaders.CONTENT_LANGUAGE);
headers.remove(HttpHeaders.CONTENT_LENGTH);
headers.remove(HttpHeaders.CONTENT_LOCATION);
headers.remove(HttpHeaders.CONTENT_RANGE);
headers.remove(HttpHeaders.CONTENT_TYPE);
}

// Package-private: used in ResponseCookie
static String formatDate(long date) {
Instant instant = Instant.ofEpochMilli(date);
Expand Down
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2020 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -181,21 +181,22 @@ public final Mono<Void> writeWith(Publisher<? extends DataBuffer> body) {
if (body instanceof Mono) {
return ((Mono<? extends DataBuffer>) body).flatMap(buffer ->
doCommit(() -> writeWithInternal(Mono.just(buffer)))
.doOnDiscard(PooledDataBuffer.class, DataBufferUtils::release));
.doOnDiscard(PooledDataBuffer.class, DataBufferUtils::release))
.doOnError(t -> clearContentHeaders());
}
return new ChannelSendOperator<>(body, inner -> doCommit(() -> writeWithInternal(inner)))
.doOnError(t -> removeContentLength());
.doOnError(t -> clearContentHeaders());
}

@Override
public final Mono<Void> writeAndFlushWith(Publisher<? extends Publisher<? extends DataBuffer>> body) {
return new ChannelSendOperator<>(body, inner -> doCommit(() -> writeAndFlushWithInternal(inner)))
.doOnError(t -> removeContentLength());
.doOnError(t -> clearContentHeaders());
}

private void removeContentLength() {
private void clearContentHeaders() {
if (!this.isCommitted()) {
this.getHeaders().remove(HttpHeaders.CONTENT_LENGTH);
HttpHeaders.clearContentHeaders(this.getHeaders());
}
}

Expand Down
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2020 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -30,18 +30,20 @@
import org.springframework.core.io.buffer.DefaultDataBuffer;
import org.springframework.core.io.buffer.DefaultDataBufferFactory;
import org.springframework.http.HttpHeaders;
import org.springframework.http.MediaType;
import org.springframework.http.ResponseCookie;

import static org.assertj.core.api.Assertions.assertThat;

/**
* @author Rossen Stoyanchev
* @author Sebastien Deleuze
* @author Brian Clozel
*/
public class ServerHttpResponseTests {

@Test
public void writeWith() throws Exception {
void writeWith() throws Exception {
TestServerHttpResponse response = new TestServerHttpResponse();
response.writeWith(Flux.just(wrap("a"), wrap("b"), wrap("c"))).block();

Expand All @@ -56,7 +58,7 @@ public void writeWith() throws Exception {
}

@Test // SPR-14952
public void writeAndFlushWithFluxOfDefaultDataBuffer() throws Exception {
void writeAndFlushWithFluxOfDefaultDataBuffer() throws Exception {
TestServerHttpResponse response = new TestServerHttpResponse();
Flux<Flux<DefaultDataBuffer>> flux = Flux.just(Flux.just(wrap("foo")));
response.writeAndFlushWith(flux).block();
Expand All @@ -70,21 +72,35 @@ public void writeAndFlushWithFluxOfDefaultDataBuffer() throws Exception {
}

@Test
public void writeWithError() throws Exception {
TestServerHttpResponse response = new TestServerHttpResponse();
response.getHeaders().setContentLength(12);
void writeWithFluxError() throws Exception {
IllegalStateException error = new IllegalStateException("boo");
response.writeWith(Flux.error(error)).onErrorResume(ex -> Mono.empty()).block();
writeWithError(Flux.error(error));
}

@Test
void writeWithMonoError() throws Exception {
IllegalStateException error = new IllegalStateException("boo");
writeWithError(Mono.error(error));
}

void writeWithError(Publisher<DataBuffer> body) throws Exception {
TestServerHttpResponse response = new TestServerHttpResponse();
HttpHeaders headers = response.getHeaders();
headers.setContentType(MediaType.APPLICATION_JSON);
headers.set(HttpHeaders.CONTENT_ENCODING, "gzip");
headers.setContentLength(12);
response.writeWith(body).onErrorResume(ex -> Mono.empty()).block();

assertThat(response.statusCodeWritten).isFalse();
assertThat(response.headersWritten).isFalse();
assertThat(response.cookiesWritten).isFalse();
assertThat(response.getHeaders().containsKey(HttpHeaders.CONTENT_LENGTH)).isFalse();
assertThat(headers).doesNotContainKeys(HttpHeaders.CONTENT_TYPE, HttpHeaders.CONTENT_LENGTH,
HttpHeaders.CONTENT_ENCODING);
assertThat(response.body.isEmpty()).isTrue();
}

@Test
public void setComplete() throws Exception {
void setComplete() throws Exception {
TestServerHttpResponse response = new TestServerHttpResponse();
response.setComplete().block();

Expand All @@ -95,7 +111,7 @@ public void setComplete() throws Exception {
}

@Test
public void beforeCommitWithComplete() throws Exception {
void beforeCommitWithComplete() throws Exception {
ResponseCookie cookie = ResponseCookie.from("ID", "123").build();
TestServerHttpResponse response = new TestServerHttpResponse();
response.beforeCommit(() -> Mono.fromRunnable(() -> response.getCookies().add(cookie.getName(), cookie)));
Expand All @@ -113,7 +129,7 @@ public void beforeCommitWithComplete() throws Exception {
}

@Test
public void beforeCommitActionWithSetComplete() throws Exception {
void beforeCommitActionWithSetComplete() throws Exception {
ResponseCookie cookie = ResponseCookie.from("ID", "123").build();
TestServerHttpResponse response = new TestServerHttpResponse();
response.beforeCommit(() -> {
Expand All @@ -130,7 +146,6 @@ public void beforeCommitActionWithSetComplete() throws Exception {
}



private DefaultDataBuffer wrap(String a) {
return new DefaultDataBufferFactory().wrap(ByteBuffer.wrap(a.getBytes(StandardCharsets.UTF_8)));
}
Expand Down
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2018 the original author or authors.
* Copyright 2002-2020 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -211,7 +211,7 @@ private Mono<HandlerResult> handleException(Throwable exception, HandlerMethod h
// Success and error responses may use different content types
exchange.getAttributes().remove(HandlerMapping.PRODUCIBLE_MEDIA_TYPES_ATTRIBUTE);
if (!exchange.getResponse().isCommitted()) {
exchange.getResponse().getHeaders().remove(HttpHeaders.CONTENT_TYPE);
HttpHeaders.clearContentHeaders(exchange.getResponse().getHeaders());
}

InvocableHandlerMethod invocable = this.methodResolver.getExceptionHandlerMethod(exception, handlerMethod);
Expand Down

0 comments on commit f9c1565

Please sign in to comment.