ResponseBodyEmitter allows complete after non-IOException

Closes gh-30687
spring-projects · Oct 4, 2023 · a3636af · a3636af
1 parent 37d03e5
commit a3636af
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 16 deletions.
diff --git a/.../main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseBodyEmitter.java b/.../main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseBodyEmitter.java
@@ -87,7 +87,7 @@ public class ResponseBodyEmitter {
 	 * that may come for example from an application try-catch block on the
 	 * thread of the I/O error.
 	 */
-	private boolean sendFailed;
+	private boolean ioErrorOnSend;
 
 	private final DefaultCallback timeoutCallback = new DefaultCallback();
 
@@ -198,11 +198,10 @@ public synchronized void send(Object object, @Nullable MediaType mediaType) thro
 				this.handler.send(object, mediaType);
 			}
 			catch (IOException ex) {
-				this.sendFailed = true;
+				this.ioErrorOnSend = true;
 				throw ex;
 			}
 			catch (Throwable ex) {
-				this.sendFailed = true;
 				throw new IllegalStateException("Failed to send " + object, ex);
 			}
 		}
@@ -235,11 +234,10 @@ private void sendInternal(Set<DataWithMediaType> items) throws IOException {
 				this.handler.send(items);
 			}
 			catch (IOException ex) {
-				this.sendFailed = true;
+				this.ioErrorOnSend = true;
 				throw ex;
 			}
 			catch (Throwable ex) {
-				this.sendFailed = true;
 				throw new IllegalStateException("Failed to send " + items, ex);
 			}
 		}
@@ -257,8 +255,8 @@ private void sendInternal(Set<DataWithMediaType> items) throws IOException {
 	 * related events such as an error while {@link #send(Object) sending}.
 	 */
 	public synchronized void complete() {
-		// Ignore, after send failure
-		if (this.sendFailed) {
+		// Ignore complete after IO failure on send
+		if (this.ioErrorOnSend) {
 			return;
 		}
 		this.complete = true;
@@ -279,8 +277,8 @@ public synchronized void complete() {
 	 * {@link #send(Object) sending}.
 	 */
 	public synchronized void completeWithError(Throwable ex) {
-		// Ignore, after send failure
-		if (this.sendFailed) {
+		// Ignore complete after IO failure on send
+		if (this.ioErrorOnSend) {
 			return;
 		}
 		this.complete = true;

diff --git a/.../java/org/springframework/web/servlet/mvc/method/annotation/ResponseBodyEmitterTests.java b/.../java/org/springframework/web/servlet/mvc/method/annotation/ResponseBodyEmitterTests.java
@@ -92,10 +92,9 @@ void sendBeforeHandlerInitializedWithError() throws Exception {
 	}
 
 	@Test
-	void sendFailsAfterComplete() throws Exception {
+	void sendFailsAfterComplete() {
 		this.emitter.complete();
-		assertThatIllegalStateException().isThrownBy(() ->
-				this.emitter.send("foo"));
+		assertThatIllegalStateException().isThrownBy(() -> this.emitter.send("foo"));
 	}
 
 	@Test
@@ -143,14 +142,47 @@ void sendWithError() throws Exception {
 		verify(this.handler).onCompletion(any());
 		verifyNoMoreInteractions(this.handler);
 
-		IOException failure = new IOException();
-		willThrow(failure).given(this.handler).send("foo", MediaType.TEXT_PLAIN);
-		assertThatIOException().isThrownBy(() ->
-				this.emitter.send("foo", MediaType.TEXT_PLAIN));
+		willThrow(new IOException()).given(this.handler).send("foo", MediaType.TEXT_PLAIN);
+		assertThatIOException().isThrownBy(() -> this.emitter.send("foo", MediaType.TEXT_PLAIN));
 		verify(this.handler).send("foo", MediaType.TEXT_PLAIN);
 		verifyNoMoreInteractions(this.handler);
 	}
 
+	@Test // gh-30687
+	void completeIgnoredAfterIOException() throws Exception {
+		this.emitter.initialize(this.handler);
+		verify(this.handler).onTimeout(any());
+		verify(this.handler).onError(any());
+		verify(this.handler).onCompletion(any());
+		verifyNoMoreInteractions(this.handler);
+
+		willThrow(new IOException()).given(this.handler).send("foo", MediaType.TEXT_PLAIN);
+		assertThatIOException().isThrownBy(() -> this.emitter.send("foo", MediaType.TEXT_PLAIN));
+		verify(this.handler).send("foo", MediaType.TEXT_PLAIN);
+		verifyNoMoreInteractions(this.handler);
+
+		this.emitter.complete();
+		verifyNoMoreInteractions(this.handler);
+	}
+
+	@Test // gh-30687
+	void completeAfterNonIOException() throws Exception {
+		this.emitter.initialize(this.handler);
+		verify(this.handler).onTimeout(any());
+		verify(this.handler).onError(any());
+		verify(this.handler).onCompletion(any());
+		verifyNoMoreInteractions(this.handler);
+
+		willThrow(new IllegalStateException()).given(this.handler).send("foo", MediaType.TEXT_PLAIN);
+		assertThatIllegalStateException().isThrownBy(() -> this.emitter.send("foo", MediaType.TEXT_PLAIN));
+		verify(this.handler).send("foo", MediaType.TEXT_PLAIN);
+		verifyNoMoreInteractions(this.handler);
+
+		this.emitter.complete();
+		verify(this.handler).complete();
+		verifyNoMoreInteractions(this.handler);
+	}
+
 	@Test
 	void onTimeoutBeforeHandlerInitialized() throws Exception {
 		Runnable runnable = mock();