From f13d654e543fc5535605818c088b41155417e85f Mon Sep 17 00:00:00 2001 From: Tyler King Date: Wed, 25 Sep 2024 13:04:49 -0500 Subject: [PATCH 1/4] Reproduce issue #2592 --- .../rolling/action/FileRenameActionTest.java | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/rolling/action/FileRenameActionTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/rolling/action/FileRenameActionTest.java index 7309e6e7ccf..ebbd93a9b35 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/rolling/action/FileRenameActionTest.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/rolling/action/FileRenameActionTest.java @@ -23,6 +23,7 @@ import java.io.PrintStream; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; +import org.junitpioneer.jupiter.Issue; public class FileRenameActionTest { @@ -97,4 +98,28 @@ public void testNoParent() throws Exception { assertTrue(dest.exists(), "Renamed file does not exist"); assertFalse(file.exists(), "Old file exists"); } + + @Test + @Issue("https://github.com/apache/logging-log4j2/issues/2592") + public void testRenameForMissingFile() throws Exception { + final File file = new File("fileRename.log"); + final File dest = new File(tempDir, "newFile.log"); + FileRenameAction action = new FileRenameAction(file, dest, true); + boolean renameResult = action.execute(); + assertTrue(renameResult, "Rename action returned false"); + assertTrue(dest.exists(), "Renamed file does not exist"); + assertFalse(file.exists(), "Old file exists"); + } + + @Test + @Issue("https://github.com/apache/logging-log4j2/issues/2592") + public void testRenameForMissingFileWithoutEmptyFilesRenaming() throws Exception { + final File file = new File("fileRename.log"); + final File dest = new File(tempDir, "newFile.log"); + FileRenameAction action = new FileRenameAction(file, dest, false); + boolean renameResult = action.execute(); + assertTrue(renameResult, "Rename action returned false"); + assertFalse(dest.exists(), "Renamed file should not exist"); + assertFalse(file.exists(), "Old file exists"); + } } From 01cd8ed6352971ccc1faa701e86af8cd10de4759 Mon Sep 17 00:00:00 2001 From: Tyler King Date: Wed, 25 Sep 2024 13:05:00 -0500 Subject: [PATCH 2/4] Fix FileRenameAction when the source file doesn't exist --- .../rolling/action/FileRenameAction.java | 42 +++++++++++++++---- ...leRenameAction_for_missing_source_file.xml | 27 ++++++++++++ 2 files changed, 62 insertions(+), 7 deletions(-) create mode 100644 src/changelog/.2.x.x/2592_fix_FileRenameAction_for_missing_source_file.xml diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/FileRenameAction.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/FileRenameAction.java index e7bb2376db7..208ebe333f0 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/FileRenameAction.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/FileRenameAction.java @@ -25,6 +25,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.nio.file.StandardCopyOption; +import org.apache.commons.io.input.NullInputStream; /** * File rename action. @@ -166,12 +167,38 @@ public static boolean execute(final File source, final File destination, final b } } } catch (final IOException exCopy) { - LOGGER.error( - "Unable to copy file {} to {}: {} {}", - source.getAbsolutePath(), - destination.getAbsolutePath(), - exCopy.getClass().getName(), - exCopy.getMessage()); + if (source.isFile()) { + LOGGER.error( + "Unable to copy file {} to {}: {} {}", + source.getAbsolutePath(), + destination.getAbsolutePath(), + exCopy.getClass().getName(), + exCopy.getMessage()); + } else { + LOGGER.debug( + "Unable to copy file {} to {}: {} {} - will try to create destination since source file does not exist", + source.getAbsolutePath(), + destination.getAbsolutePath(), + exCopy.getClass().getName(), + exCopy.getMessage()); + try { + Files.copy( + new NullInputStream(), + destination.toPath(), + StandardCopyOption.REPLACE_EXISTING); + result = true; + LOGGER.trace( + "Created file {} since {} does not exist", + destination.getAbsolutePath(), + source.getAbsolutePath()); + } catch (final IOException exCreate) { + LOGGER.error( + "Unable to create file {}: {} {}", + destination.getAbsolutePath(), + exCreate.getClass().getName(), + exCreate.getMessage()); + } + } } } else { LOGGER.trace( @@ -191,7 +218,8 @@ public static boolean execute(final File source, final File destination, final b } } else { try { - return source.delete(); + Files.deleteIfExists(source.toPath()); + return true; } catch (final Exception exDelete) { LOGGER.error( "Unable to delete empty file {}: {} {}", diff --git a/src/changelog/.2.x.x/2592_fix_FileRenameAction_for_missing_source_file.xml b/src/changelog/.2.x.x/2592_fix_FileRenameAction_for_missing_source_file.xml new file mode 100644 index 00000000000..75ac3d6a112 --- /dev/null +++ b/src/changelog/.2.x.x/2592_fix_FileRenameAction_for_missing_source_file.xml @@ -0,0 +1,27 @@ + + + + + + + Fixed `FileRenameAction` when the source file doesn't exist + + \ No newline at end of file From d663f3b9ae3fe4e156adbddff0a230a61f008d81 Mon Sep 17 00:00:00 2001 From: Tyler King Date: Wed, 25 Sep 2024 16:36:18 -0500 Subject: [PATCH 3/4] Fix change entry file --- ...leRenameAction_for_missing_source_file.xml | 25 +++---------------- 1 file changed, 3 insertions(+), 22 deletions(-) diff --git a/src/changelog/.2.x.x/2592_fix_FileRenameAction_for_missing_source_file.xml b/src/changelog/.2.x.x/2592_fix_FileRenameAction_for_missing_source_file.xml index 75ac3d6a112..9cf98b3e3e4 100644 --- a/src/changelog/.2.x.x/2592_fix_FileRenameAction_for_missing_source_file.xml +++ b/src/changelog/.2.x.x/2592_fix_FileRenameAction_for_missing_source_file.xml @@ -1,27 +1,8 @@ - - - - Fixed `FileRenameAction` when the source file doesn't exist - + Fixed `FileRenameAction` when the source file doesn't exist \ No newline at end of file From f395d72de806c3020a52a809fd83582f902515b4 Mon Sep 17 00:00:00 2001 From: Tyler King Date: Fri, 27 Sep 2024 08:21:38 -0500 Subject: [PATCH 4/4] Remove useage of commons-io NullInputStream --- .../log4j/core/appender/rolling/action/FileRenameAction.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/FileRenameAction.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/FileRenameAction.java index 208ebe333f0..9d84d02eb54 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/FileRenameAction.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/FileRenameAction.java @@ -17,6 +17,7 @@ package org.apache.logging.log4j.core.appender.rolling.action; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import java.io.ByteArrayInputStream; import java.io.File; import java.io.IOException; import java.io.PrintWriter; @@ -25,7 +26,6 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.nio.file.StandardCopyOption; -import org.apache.commons.io.input.NullInputStream; /** * File rename action. @@ -183,7 +183,7 @@ public static boolean execute(final File source, final File destination, final b exCopy.getMessage()); try { Files.copy( - new NullInputStream(), + new ByteArrayInputStream(new byte[0]), destination.toPath(), StandardCopyOption.REPLACE_EXISTING); result = true;