From 95a90e061369609fcddf86153e85d501d4e36166 Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Wed, 20 Dec 2023 10:36:40 +0100 Subject: [PATCH] Fix NPE in `RollingFileManger` This PR fixes a `NullPointerException` in `RollingFileManager#createParentDir`. Closes #1645 --- .../rolling/RollingFileManagerTest.java | 60 ++++++++++++++++--- .../appender/rolling/RollingFileManager.java | 6 +- .../.2.x.x/fix_create_parent_dir.xml | 10 ++++ 3 files changed, 67 insertions(+), 9 deletions(-) create mode 100644 src/changelog/.2.x.x/fix_create_parent_dir.xml diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/rolling/RollingFileManagerTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/rolling/RollingFileManagerTest.java index 5ea2b53ff8c..4a660861c20 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/rolling/RollingFileManagerTest.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/rolling/RollingFileManagerTest.java @@ -16,7 +16,17 @@ */ package org.apache.logging.log4j.core.appender.rolling; -import java.io.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.fail; + +import java.io.File; +import java.io.FileInputStream; +import java.io.IOException; +import java.io.InputStreamReader; +import java.io.Reader; import java.nio.charset.StandardCharsets; import org.apache.logging.log4j.core.LoggerContext; import org.apache.logging.log4j.core.appender.RollingFileAppender; @@ -26,8 +36,8 @@ import org.apache.logging.log4j.core.layout.PatternLayout; import org.apache.logging.log4j.core.lookup.StrSubstitutor; import org.apache.logging.log4j.core.util.IOUtils; -import org.junit.Assert; import org.junit.Test; +import org.junitpioneer.jupiter.Issue; public class RollingFileManagerTest { @@ -75,14 +85,14 @@ public RolloverDescription rollover(final RollingFileManager manager) throws Sec .withPolicy(new SizeBasedTriggeringPolicy(100)) .build(); - Assert.assertNotNull(appender); + assertNotNull(appender); final String testContent = "Test"; try (final RollingFileManager manager = appender.getManager()) { - Assert.assertEquals(file.getAbsolutePath(), manager.getFileName()); + assertEquals(file.getAbsolutePath(), manager.getFileName()); manager.writeToDestination(testContent.getBytes(StandardCharsets.US_ASCII), 0, testContent.length()); } try (final Reader reader = new InputStreamReader(new FileInputStream(file), StandardCharsets.US_ASCII)) { - Assert.assertEquals(testContent, IOUtils.toString(reader)); + assertEquals(testContent, IOUtils.toString(reader)); } } } @@ -125,7 +135,7 @@ public RolloverDescription rollover(final RollingFileManager manager) throws Sec null, null, configuration); - Assert.assertNotNull(manager); + assertNotNull(manager); manager.initialize(); // Get the initialTime of this original log file @@ -139,9 +149,43 @@ public RolloverDescription rollover(final RollingFileManager manager) throws Sec manager.rollover(); // If the rollover fails, then the size should not be reset - Assert.assertNotEquals(0, manager.getFileSize()); + assertNotEquals(0, manager.getFileSize()); // The initialTime should not have changed - Assert.assertEquals(initialTime, manager.getFileTime()); + assertEquals(initialTime, manager.getFileTime()); + } + + @Test + @Issue("https://github.com/apache/logging-log4j2/issues/1645") + public void testCreateParentDir() { + final Configuration configuration = new NullConfiguration(); + final RollingFileManager manager = RollingFileManager.getFileManager( + null, + "testCreateParentDir.log.%d{yyyy-MM-dd}", + true, + false, + NoOpTriggeringPolicy.INSTANCE, + DirectWriteRolloverStrategy.newBuilder() + .withConfig(configuration) + .build(), + null, + PatternLayout.createDefaultLayout(configuration), + 0, + true, + true, + null, + null, + null, + configuration); + assertNotNull(manager); + try { + final File file = new File("file_in_current_dir.log"); + assertNull(file.getParentFile()); + manager.createParentDir(file); + } catch (final Throwable t) { + fail("createParentDir failed: " + t.getMessage()); + } finally { + manager.close(); + } } } diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/RollingFileManager.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/RollingFileManager.java index 9d938cc2d2f..a1030f3d045 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/RollingFileManager.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/RollingFileManager.java @@ -356,7 +356,11 @@ public String getFileName() { @Override protected void createParentDir(File file) { if (directWrite) { - file.getParentFile().mkdirs(); + final File parent = file.getParentFile(); + // If the parent is null the file is in the current working directory. + if (parent != null) { + parent.mkdirs(); + } } } diff --git a/src/changelog/.2.x.x/fix_create_parent_dir.xml b/src/changelog/.2.x.x/fix_create_parent_dir.xml new file mode 100644 index 00000000000..314076edfb7 --- /dev/null +++ b/src/changelog/.2.x.x/fix_create_parent_dir.xml @@ -0,0 +1,10 @@ + + + + + Fix NPE in `RollingFileManager`. + +