From 84ab87578cbb16788a68251d00a1bf62693420c1 Mon Sep 17 00:00:00 2001 From: Jiwoo Lee Date: Sat, 22 Aug 2026 18:52:50 +0900 Subject: [PATCH] GH-11352: Propagate a failing close() when writing files Fixes: https://github.com/spring-projects/spring-integration/issues/11352 cleanUpFileState() discarded every IOException, including the one raised by closing the buffered writer or stream. close() is where the buffer is flushed, so a full disk was reported as a successful write and the truncated temporary file was still promoted onto the result file. Let cleanUpFileState() propagate, and call it on the success path where the caller can see the failure. On the failure path the close error is attached as a suppressed exception instead, so the exception already on its way out is not masked. clearState() still runs either way and the APPEND_NO_FLUSH branch is untouched. Signed-off-by: Jiwoo Lee --- .../outbound/FileWritingMessageHandler.java | 48 ++++++++++++++----- .../FileWritingMessageHandlerTests.java | 48 +++++++++++++++++++ 2 files changed, 84 insertions(+), 12 deletions(-) diff --git a/spring-integration-file/src/main/java/org/springframework/integration/file/outbound/FileWritingMessageHandler.java b/spring-integration-file/src/main/java/org/springframework/integration/file/outbound/FileWritingMessageHandler.java index 9b8d2ca87b..e2f1d58e54 100644 --- a/spring-integration-file/src/main/java/org/springframework/integration/file/outbound/FileWritingMessageHandler.java +++ b/spring-integration-file/src/main/java/org/springframework/integration/file/outbound/FileWritingMessageHandler.java @@ -118,6 +118,7 @@ * @author Christian Tzolov * @author Ngoc Nhan * @author Thomas Knall + * @author Jiwoo Lee * * @since 7.0 */ @@ -674,24 +675,43 @@ private void appendStreamToFile(File fileToWriteTo, InputStream sourceFileInputS bos.write(System.lineSeparator().getBytes()); } } - finally { - cleanUpFileState(fileToWriteTo, state, bos); + catch (IOException | RuntimeException ex) { + cleanUpFileStateSuppressing(ex, fileToWriteTo, state, bos); + throw ex; } + cleanUpFileState(fileToWriteTo, state, bos); } - private void cleanUpFileState(File fileToWriteTo, @Nullable FileState state, @Nullable Closeable closeable) { - try { - if (state == null || this.flushTask == null) { + private void cleanUpFileState(File fileToWriteTo, @Nullable FileState state, @Nullable Closeable closeable) + throws IOException { + + if (state == null || this.flushTask == null) { + try { if (closeable != null) { closeable.close(); } - clearState(fileToWriteTo, state); } - else { - state.lastWrite = System.currentTimeMillis(); + finally { + clearState(fileToWriteTo, state); } } + else { + state.lastWrite = System.currentTimeMillis(); + } + } + + /** + * Clean up after a write that has already failed, adding any failure from the clean up + * to the exception which is on its way out, the way try-with-resources does. + */ + private void cleanUpFileStateSuppressing(Throwable primary, File fileToWriteTo, @Nullable FileState state, + @Nullable Closeable closeable) { + + try { + cleanUpFileState(fileToWriteTo, state, closeable); + } catch (IOException ex) { + primary.addSuppressed(ex); } } @@ -733,9 +753,11 @@ private void writeBytesToFile(File fileToWriteTo, boolean append, byte[] bytes) bos.write(System.lineSeparator().getBytes()); } } - finally { - cleanUpFileState(fileToWriteTo, state, bos); + catch (IOException | RuntimeException ex) { + cleanUpFileStateSuppressing(ex, fileToWriteTo, state, bos); + throw ex; } + cleanUpFileState(fileToWriteTo, state, bos); } private File handleStringMessage(String content, @Nullable File originalFile, File tempFile, File resultFile, @@ -776,9 +798,11 @@ private void writeStringToFile(File fileToWriteTo, boolean append, String conten writer.newLine(); } } - finally { - cleanUpFileState(fileToWriteTo, state, writer); + catch (IOException | RuntimeException ex) { + cleanUpFileStateSuppressing(ex, fileToWriteTo, state, writer); + throw ex; } + cleanUpFileState(fileToWriteTo, state, writer); } private File determineFileToWrite(File resultFile, File tempFile) { diff --git a/spring-integration-file/src/test/java/org/springframework/integration/file/outbound/FileWritingMessageHandlerTests.java b/spring-integration-file/src/test/java/org/springframework/integration/file/outbound/FileWritingMessageHandlerTests.java index 05f33c3832..a7f9c89c46 100644 --- a/spring-integration-file/src/test/java/org/springframework/integration/file/outbound/FileWritingMessageHandlerTests.java +++ b/spring-integration-file/src/test/java/org/springframework/integration/file/outbound/FileWritingMessageHandlerTests.java @@ -17,12 +17,16 @@ package org.springframework.integration.file.outbound; import java.io.BufferedOutputStream; +import java.io.BufferedWriter; import java.io.ByteArrayInputStream; import java.io.File; import java.io.FileInputStream; +import java.io.FileNotFoundException; import java.io.FileOutputStream; +import java.io.FileWriter; import java.io.IOException; import java.io.InputStream; +import java.io.Writer; import java.nio.file.Files; import java.nio.file.InvalidPathException; import java.nio.file.attribute.PosixFilePermission; @@ -90,6 +94,7 @@ * @author Alen Turkovic * @author Glenn Renfro * @author Thomas Knall + * @author Jiwoo Lee */ public class FileWritingMessageHandlerTests implements TestApplicationContextAware { @@ -776,4 +781,47 @@ protected File messageToFile(Message result) { return (File) result.getPayload(); } + @Test + void closeFailureIsReportedAndTheFileIsNotPromoted(@TempDir File localRoot) throws Exception { + File out = new File(localRoot, "out"); + // A writer whose close() fails the way a full disk does: the buffered content + // never reaches the file and the IOException surfaces from close(). + FileWritingMessageHandler handler = new FileWritingMessageHandler(out) { + + @Override + protected BufferedWriter createWriter(File fileToWriteTo, boolean append) + throws FileNotFoundException { + + Writer target; + try { + target = new FileWriter(fileToWriteTo, append); + } + catch (IOException ex) { + throw new FileNotFoundException(ex.getMessage()); + } + return new BufferedWriter(target) { + + @Override + public void close() throws IOException { + target.close(); + throw new IOException("No space left on device"); + } + + }; + } + + }; + handler.setFileNameGenerator(message -> "payload.txt"); + handler.setOutputChannel(new NullChannel()); + handler.setBeanFactory(TEST_INTEGRATION_CONTEXT); + handler.setApplicationContext(new GenericApplicationContext()); + handler.afterPropertiesSet(); + + assertThatExceptionOfType(MessageHandlingException.class) + .isThrownBy(() -> handler.handleMessage(new GenericMessage<>("important payload"))) + .withStackTraceContaining("java.io.IOException: No space left on device"); + + assertThat(new File(out, "payload.txt")).doesNotExist(); + } + }