This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 10.1.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 685f4f1f22a35d67303aa8c2e74732ff7fea57e1 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 1e66986b5b..1c78fdbe80 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 5dcced0546..5f6085b920 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().getContextBind().bind(false, 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 055a48c377..84e8f16bd7 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 7fe78526d1..bdf8cebf8d 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -284,6 +284,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="Web applications"> --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
