You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2026/10/13 12:08:18 UTC
(tomcat) 02/04: Report errors to upgraded connection WriteListeners at most once
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit adeeac81f2c96ee758d7baf0068544282c2d731c
Author: opencode <re...@apache.org>
AuthorDate: Tue Oct 13 13:39:52 2026 +0200
Report errors to upgraded connection WriteListeners at most once
A single failure to flush the output of an upgraded connection was reported
to the registered WriteListener more than once. UpgradeServletOutputStream
flushInternal() calls onError() for any Throwable raised by the socket flush
and then re-throws. onWritePossible() catches that re-thrown exception and
calls onError() a second time. Additionally the close() performed by
onError() itself re-enters flushInternal(), whose failure triggers a further
onError(). Since onError() had no one-shot guard and never cleared the
listener, listener.onError() could therefore be invoked repeatedly for one
failure, contrary to the Servlet spec which allows at most one onError() and
no call-backs afterwards.
UpgradeServletOutputStream now sets a terminal errorReported flag before
notifying the listener and suppresses any further call-backs, including
onWritePossible() from later OPEN_WRITE dispatches, once an error has been
reported. The equivalent onError() guard on UpgradeServletInputStream was
extended in the same way for symmetry.
A new mock-based test verifies that a failing flush reports the error to the
listener exactly once and that a subsequent onWritePossible() dispatch
triggers no further socket processing.
---
.../http11/upgrade/UpgradeServletInputStream.java | 2 +-
.../http11/upgrade/UpgradeServletOutputStream.java | 14 ++++++-
.../apache/coyote/http11/upgrade/TestUpgrade.java | 45 ++++++++++++++++++++++
webapps/docs/changelog.xml | 6 +++
4 files changed, 65 insertions(+), 2 deletions(-)
diff --git a/java/org/apache/coyote/http11/upgrade/UpgradeServletInputStream.java b/java/org/apache/coyote/http11/upgrade/UpgradeServletInputStream.java
index a0eb1c8299..65a5fcb92f 100644
--- a/java/org/apache/coyote/http11/upgrade/UpgradeServletInputStream.java
+++ b/java/org/apache/coyote/http11/upgrade/UpgradeServletInputStream.java
@@ -292,7 +292,7 @@ public class UpgradeServletInputStream extends ServletInputStream {
private void onError(Throwable t) {
- if (listener == null) {
+ if (listener == null || errorReported) {
return;
}
errorReported = true;
diff --git a/java/org/apache/coyote/http11/upgrade/UpgradeServletOutputStream.java b/java/org/apache/coyote/http11/upgrade/UpgradeServletOutputStream.java
index 93240fcded..fa7499f874 100644
--- a/java/org/apache/coyote/http11/upgrade/UpgradeServletOutputStream.java
+++ b/java/org/apache/coyote/http11/upgrade/UpgradeServletOutputStream.java
@@ -79,6 +79,12 @@ public class UpgradeServletOutputStream extends ServletOutputStream {
*/
private volatile boolean closed = false;
+ /**
+ * Whether an error has been reported to the listener. No further
+ * call-backs are permitted once the error has been reported.
+ */
+ private volatile boolean errorReported = false;
+
// Start in blocking-mode
/**
* The async write listener.
@@ -248,6 +254,11 @@ public class UpgradeServletOutputStream extends ServletOutputStream {
final void onWritePossible() {
+ if (errorReported) {
+ // No further call-backs are permitted once an error has been
+ // reported.
+ return;
+ }
try {
if (flushing) {
flushInternal(false, true);
@@ -293,9 +304,10 @@ public class UpgradeServletOutputStream extends ServletOutputStream {
private void onError(Throwable t) {
- if (listener == null) {
+ if (listener == null || errorReported) {
return;
}
+ errorReported = true;
ClassLoader oldCL = processor.getUpgradeToken().contextBind().bind(null);
try {
listener.onError(t);
diff --git a/test/org/apache/coyote/http11/upgrade/TestUpgrade.java b/test/org/apache/coyote/http11/upgrade/TestUpgrade.java
index f13ee3a5e9..5b3a8de92b 100644
--- a/test/org/apache/coyote/http11/upgrade/TestUpgrade.java
+++ b/test/org/apache/coyote/http11/upgrade/TestUpgrade.java
@@ -190,6 +190,51 @@ public class TestUpgrade extends TomcatBaseTest {
EasyMock.verify(socketWrapper, listener);
}
+ @Test
+ @SuppressWarnings("unchecked")
+ public void testWriteErrorReportedOnlyOnce() throws Exception {
+ SocketWrapperBase<?> socketWrapper = EasyMock.createMock(SocketWrapperBase.class);
+ WriteListener listener = EasyMock.createMock(WriteListener.class);
+
+ ContextBind contextBind = new ContextBind() {
+ @Override
+ public ClassLoader bind(ClassLoader originalClassLoader) {
+ return null;
+ }
+
+ @Override
+ public void unbind(ClassLoader originalClassLoader) {
+ // NO-OP
+ }
+ };
+ UpgradeProcessorBase processor =
+ new TesterUpgradeProcessor(new UpgradeToken(null, contextBind, null, "test"));
+
+ IOException error = new IOException("Deliberately triggered for the test");
+
+ socketWrapper.registerWriteInterest();
+ // The flush fails. That failure is seen twice, once by the caller and
+ // once by the close that reports the error. The listener must only be
+ // notified once.
+ EasyMock.expect(socketWrapper.flush(false)).andThrow(error);
+ EasyMock.expect(socketWrapper.flush(false)).andThrow(error);
+ listener.onError(error);
+ EasyMock.expectLastCall().once();
+
+ EasyMock.replay(socketWrapper, listener);
+
+ UpgradeServletOutputStream sos = new UpgradeServletOutputStream(processor, socketWrapper,
+ new UpgradeInfo());
+ sos.setWriteListener(listener);
+
+ sos.onWritePossible();
+ // The call-back for the next dispatch must not trigger any further
+ // socket processing.
+ sos.onWritePossible();
+
+ EasyMock.verify(socketWrapper, listener);
+ }
+
@Test
@SuppressWarnings("unchecked")
public void testOnAllDataReadReportedOnlyOnce() throws Exception {
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index b9fce895d0..6047fe710f 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -471,6 +471,12 @@
reported to the listener, or once the end of the data has been
reported. (remm)
</fix>
+ <fix>
+ Report an error to the <code>WriteListener</code> of an upgraded
+ connection at most once and stop triggering further call-backs once
+ an error has been reported. A single failure to flush the output was
+ previously reported to the listener multiple times. (remm)
+ </fix>
</changelog>
</subsection>
<subsection name="Jasper">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org