This is an automated email from the ASF dual-hosted git repository.
ffang pushed a commit to branch 4.1.x-fixes
in repository https://gitbox.apache.org/repos/asf/cxf.git
The following commit(s) were added to refs/heads/4.1.x-fixes by this push:
new bcec379beb1 [CXF-9251]Logging: don't cache writes after
LoggingOutputStream is closed (#3553)
bcec379beb1 is described below
commit bcec379beb169a8494d0a81d0e2a82aa6fa0266e
Author: Freeman(Yue) Fang <[email protected]>
AuthorDate: Wed Oct 7 14:32:40 2026 -0400
[CXF-9251]Logging: don't cache writes after LoggingOutputStream is closed
(#3553)
Since #3541, LoggingOutputStream closes itself (and logs) when a write
fails, and any later close() is a no-op. If something writes to the
stream after that, and the underlying stream silently accepts writes
after close (as Tomcat 10.1 does), CacheAndWriteOutputStream caches
those bytes again. Because totalLength is never reset, they spill into
a new temp file and the stream is registered with the
DelayedCachedOutputStreamCleaner again. When the cleaner fires, its
close() is a no-op, so the temp file is never deleted, not even by
the cleanup thread.
This happens on a client connection reset during a large response:
the write fails, the stream closes itself, and the fault chain
(SoapOutEndingInterceptor) then writes the closing tags to the same
stream.
Once closed, the write methods now only pass the bytes through to the
flow-through stream and no longer cache them, so nothing is spilled to
disk after the payload has been logged.
Add 2 tests to LoggingOutInterceptorTest (write after close, write
after a failed write) that check the temp file is deleted after
forceClean(); both fail without this change.
(cherry picked from commit b5ba52bd7b6dea4cf9ad23e5bf028389258cb8c0)
---
.../cxf/ext/logging/LoggingOutputStream.java | 20 +++++
.../cxf/ext/logging/LoggingOutInterceptorTest.java | 89 ++++++++++++++++++++++
2 files changed, 109 insertions(+)
diff --git
a/rt/features/logging/src/main/java/org/apache/cxf/ext/logging/LoggingOutputStream.java
b/rt/features/logging/src/main/java/org/apache/cxf/ext/logging/LoggingOutputStream.java
index 202f577672b..ac24ac1a528 100644
---
a/rt/features/logging/src/main/java/org/apache/cxf/ext/logging/LoggingOutputStream.java
+++
b/rt/features/logging/src/main/java/org/apache/cxf/ext/logging/LoggingOutputStream.java
@@ -80,9 +80,19 @@ public class LoggingOutputStream extends
CacheAndWriteOutputStream {
* We override the write() methods in order to catch some error that would
not
* close the CachedOutputStream (ex. IOException "Connection reset by
peer").
* This caused ghost/delayed OUT log and possible memory-leak due to
DelayedCachedOutputStreamCleaner
+ *
+ * Once closed (and logged), any late write (for example the fault chain
writing the closing tags
+ * through a writer still wrapping this stream) is only passed through to
the flow-through stream and
+ * is not cached anymore: some containers (e.g. Tomcat 10.1) silently
accept writes after close, and
+ * caching them would spill into a new temp file that close() (now a
no-op) could never delete.
*/
@Override
public void write(byte[] b) throws IOException {
+ if (closed.get()) {
+ // already closed and logged: pass through only, do not cache again
+ getFlowThroughStream().write(b);
+ return;
+ }
try {
super.write(b);
} catch (RuntimeException | IOException ex) {
@@ -93,6 +103,11 @@ public class LoggingOutputStream extends
CacheAndWriteOutputStream {
@Override
public void write(byte[] b, int off, int len) throws IOException {
+ if (closed.get()) {
+ // already closed and logged: pass through only, do not cache again
+ getFlowThroughStream().write(b, off, len);
+ return;
+ }
try {
super.write(b, off, len);
} catch (RuntimeException | IOException ex) {
@@ -103,6 +118,11 @@ public class LoggingOutputStream extends
CacheAndWriteOutputStream {
@Override
public void write(int b) throws IOException {
+ if (closed.get()) {
+ // already closed and logged: pass through only, do not cache again
+ getFlowThroughStream().write(b);
+ return;
+ }
try {
super.write(b);
} catch (RuntimeException | IOException ex) {
diff --git
a/rt/features/logging/src/test/java/org/apache/cxf/ext/logging/LoggingOutInterceptorTest.java
b/rt/features/logging/src/test/java/org/apache/cxf/ext/logging/LoggingOutInterceptorTest.java
index 87f82c5def7..e94f726432f 100644
---
a/rt/features/logging/src/test/java/org/apache/cxf/ext/logging/LoggingOutInterceptorTest.java
+++
b/rt/features/logging/src/test/java/org/apache/cxf/ext/logging/LoggingOutInterceptorTest.java
@@ -20,10 +20,12 @@
package org.apache.cxf.ext.logging;
import java.io.ByteArrayOutputStream;
+import java.io.File;
import java.io.IOException;
import java.io.OutputStream;
import java.io.UncheckedIOException;
import java.nio.charset.StandardCharsets;
+import java.util.concurrent.atomic.AtomicBoolean;
import org.apache.cxf.Bus;
import org.apache.cxf.BusFactory;
@@ -306,6 +308,93 @@ public class LoggingOutInterceptorTest {
assertThat(event.getPayload(), equalToIgnoringCase(buf.toString()));
}
+ @Test
+ public void shouldDeleteTempFileWhenWrittenAfterClose() throws IOException
{
+ message.put(Message.ENDPOINT_ADDRESS, "http://localhost:9001/";);
+ message.put(Message.REQUEST_URI, "/api");
+
+ final StringBuilder buf = content();
+ String ct = "multipart/related; type=\"application/xop+xml\"; "
+ + "boundary=\"----=_Part_0_2180223.1203118300920\"";
+
+ final byte[] bytes = buf.toString().getBytes(StandardCharsets.UTF_8);
+ final OutputStream os = new ByteArrayOutputStream();
+ message.setContent(OutputStream.class, os);
+ message.put(Message.CONTENT_TYPE, ct);
+
+ interceptor.setInMemThreshold(1);
+ interceptor.addBinaryContentMediaTypes("application/xop+xml");
+ interceptor.setLogMultipart(true);
+ interceptor.setLogBinary(true);
+ interceptor.handleMessage(message);
+
+ final OutputStream cached = message.getContent(OutputStream.class);
+ cached.write(bytes, 0, bytes.length);
+ cached.close();
+ assertThat(sender.getEvents(), hasSize(1));
+ assertThat(cleaner.size(), equalTo(0));
+
+ // A late writer still holding the old stream reference, with a
flow-through stream that
+ // accepts writes after close: it must not be cached (and spilled to a
new temp file) again
+ cached.write(bytes, 0, bytes.length);
+ assertThat(((ByteArrayOutputStream) os).size(), equalTo(2 *
bytes.length));
+ final File tempFile = ((CachedOutputStream) cached).getTempFile();
+
+ cleaner.forceClean();
+ assertThat(cleaner.size(), equalTo(0));
+ assertThat(sender.getEvents(), hasSize(1));
+ assertThat("temp file leaked: " + tempFile, tempFile != null &&
tempFile.exists(), equalTo(false));
+ }
+
+ @Test
+ public void shouldDeleteTempFileWhenWrittenAfterFailedWrite() throws
IOException {
+ message.put(Message.ENDPOINT_ADDRESS, "http://localhost:9001/";);
+ message.put(Message.REQUEST_URI, "/api");
+
+ final StringBuilder buf = content();
+ String ct = "multipart/related; type=\"application/xop+xml\"; "
+ + "boundary=\"----=_Part_0_2180223.1203118300920\"";
+
+ final byte[] bytes = buf.toString().getBytes(StandardCharsets.UTF_8);
+ final AtomicBoolean failNextWrite = new AtomicBoolean();
+ final OutputStream os = new ByteArrayOutputStream() {
+ @Override
+ public synchronized void write(byte[] b, int off, int len) {
+ if (failNextWrite.compareAndSet(true, false)) {
+ throw new UncheckedIOException(new
IOException("Simulated"));
+ }
+ super.write(b, off, len);
+ }
+ };
+ message.setContent(OutputStream.class, os);
+ message.put(Message.CONTENT_TYPE, ct);
+
+ interceptor.setInMemThreshold(1);
+ interceptor.addBinaryContentMediaTypes("application/xop+xml");
+ interceptor.setLogMultipart(true);
+ interceptor.setLogBinary(true);
+ interceptor.handleMessage(message);
+
+ final OutputStream cached = message.getContent(OutputStream.class);
+ cached.write(bytes, 0, bytes.length);
+ failNextWrite.set(true);
+ assertThrows(UncheckedIOException.class, () -> cached.write(bytes, 0,
bytes.length));
+ assertThat(sender.getEvents(), hasSize(1));
+ assertThat(cleaner.size(), equalTo(0));
+
+ // e.g. the fault chain (SoapOutEndingInterceptor) writing the closing
tags through the
+ // XMLStreamWriter that still wraps this stream, with a flow-through
stream that accepts writes
+ // after close (as Tomcat 10.1 does): it must not be cached (and
spilled to a new temp file) again
+ cached.write(bytes, 0, bytes.length);
+ assertThat(((ByteArrayOutputStream) os).size(), equalTo(2 *
bytes.length));
+ final File tempFile = ((CachedOutputStream) cached).getTempFile();
+
+ cleaner.forceClean();
+ assertThat(cleaner.size(), equalTo(0));
+ assertThat(sender.getEvents(), hasSize(1));
+ assertThat("temp file leaked: " + tempFile, tempFile != null &&
tempFile.exists(), equalTo(false));
+ }
+
private static StringBuilder content() {
StringBuilder buf = new StringBuilder(512);
buf.append("------=_Part_0_2180223.1203118300920\n");
