From de208cd3729d16e3f8993280bab702d4b9fea5a7 Mon Sep 17 00:00:00 2001
From: Brian Clozel <bclozel@pivotal.io>
Date: Wed, 29 Nov 2017 11:39:28 +0100
Subject: [PATCH] Do not handle WebFlux errors if response is committed

If the WebFlux handler commits the response but still sends an error
signal in the reactive pipeline, Spring Boot error handling should not
try to handle that error: once committed, it is impossible to change the
response status or the response headers. Writing to the body might also
lead to invalid responses.

This commit skips error handling if the response is committed and
delegates to Spring Framework's `HttpWebHandlerAdapter` which will log
the error.

Fixes gh-11168
---
 .../AbstractErrorWebExceptionHandler.java      | 17 +++++++++++------
 ...rrorWebExceptionHandlerIntegrationTest.java | 18 ++++++++++++++++++
 2 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/error/AbstractErrorWebExceptionHandler.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/error/AbstractErrorWebExceptionHandler.java
index 5c61a3854d..18aea21c8e 100644
--- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/error/AbstractErrorWebExceptionHandler.java
+++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/error/AbstractErrorWebExceptionHandler.java
@@ -220,12 +220,17 @@ public abstract class AbstractErrorWebExceptionHandler
 
 	@Override
 	public Mono<Void> handle(ServerWebExchange exchange, Throwable throwable) {
-		this.errorAttributes.storeErrorInformation(throwable, exchange);
-		ServerRequest request = ServerRequest.create(exchange, this.messageReaders);
-		return getRoutingFunction(this.errorAttributes).route(request)
-				.switchIfEmpty(Mono.error(throwable))
-				.flatMap((handler) -> handler.handle(request))
-				.flatMap((response) -> write(exchange, response));
+		if (!exchange.getResponse().isCommitted()) {
+			this.errorAttributes.storeErrorInformation(throwable, exchange);
+			ServerRequest request = ServerRequest.create(exchange, this.messageReaders);
+			return getRoutingFunction(this.errorAttributes).route(request)
+					.switchIfEmpty(Mono.error(throwable))
+					.flatMap((handler) -> handler.handle(request))
+					.flatMap((response) -> write(exchange, response));
+		}
+		else {
+			return Mono.error(throwable);
+		}
 	}
 
 	private Mono<? extends Void> write(ServerWebExchange exchange,
diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/error/DefaultErrorWebExceptionHandlerIntegrationTest.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/error/DefaultErrorWebExceptionHandlerIntegrationTest.java
index 37eb53f346..c319c7d8a1 100644
--- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/error/DefaultErrorWebExceptionHandlerIntegrationTest.java
+++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/error/DefaultErrorWebExceptionHandlerIntegrationTest.java
@@ -52,6 +52,7 @@ import org.springframework.web.bind.annotation.RequestBody;
 import org.springframework.web.bind.annotation.ResponseBody;
 import org.springframework.web.bind.annotation.RestController;
 import org.springframework.web.server.ResponseStatusException;
+import org.springframework.web.server.ServerWebExchange;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.hamcrest.Matchers.allOf;
@@ -188,6 +189,16 @@ public class DefaultErrorWebExceptionHandlerIntegrationTest {
 				containsString("IllegalStateException")));
 	}
 
+	@Test
+	public void responseCommitted() throws Exception {
+		load();
+		this.webTestClient.get().uri("/commit").exchange()
+				.expectStatus().isEqualTo(HttpStatus.OK)
+				.expectBody().isEmpty();
+		this.output.expect(not(containsString("java.lang.UnsupportedOperationException")));
+		this.output.expect(containsString("java.lang.IllegalStateException: already committed!"));
+	}
+
 	private void load(String... arguments) {
 		List<String> args = new ArrayList<>();
 		args.add("--server.port=0");
@@ -235,6 +246,13 @@ public class DefaultErrorWebExceptionHandlerIntegrationTest {
 				return Mono.error(new ResponseStatusException(HttpStatus.BAD_REQUEST));
 			}
 
+			@GetMapping("/commit")
+			public Mono<Void> commit(ServerWebExchange exchange) {
+				return exchange
+						.getResponse().writeWith(Mono.empty())
+						.then(Mono.error(new IllegalStateException("already committed!")));
+			}
+
 			@PostMapping(path = "/bind", produces = "application/json")
 			@ResponseBody
 			public String bodyValidation(@Valid @RequestBody DummyBody body) {