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 <[email protected]> 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: [email protected] For additional commands, e-mail: [email protected]
