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/log4j-to-slf4j/pom.xml b/log4j-to-slf4j/pom.xml index 24f9d421008..bc01ffe9c19 100644 --- a/log4j-to-slf4j/pom.xml +++ b/log4j-to-slf4j/pom.xml @@ -103,6 +103,11 @@ log4j-api-test test + + org.mockito + mockito-core + test + diff --git a/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/MDCContextMap.java b/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/MDCContextMap.java index 7f8424c6a24..4f2ef190bb4 100644 --- a/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/MDCContextMap.java +++ b/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/MDCContextMap.java @@ -16,6 +16,7 @@ */ package org.apache.logging.slf4j; +import java.util.HashMap; import java.util.Map; import java.util.Map.Entry; import org.apache.logging.log4j.spi.CleanableThreadContextMap; @@ -75,13 +76,12 @@ public boolean containsKey(final String key) { } @Override - @SuppressWarnings("unchecked") // nothing we can do about this, restricted by SLF4J API public Map getCopy() { - return MDC.getCopyOfContextMap(); + final Map contextMap = MDC.getCopyOfContextMap(); + return contextMap != null ? contextMap : new HashMap<>(); } @Override - @SuppressWarnings("unchecked") // nothing we can do about this, restricted by SLF4J API public Map getImmutableMapOrNull() { return MDC.getCopyOfContextMap(); } diff --git a/log4j-to-slf4j/src/test/java/org/apache/logging/slf4j/MDCContextMapTest.java b/log4j-to-slf4j/src/test/java/org/apache/logging/slf4j/MDCContextMapTest.java new file mode 100644 index 00000000000..6673de4597a --- /dev/null +++ b/log4j-to-slf4j/src/test/java/org/apache/logging/slf4j/MDCContextMapTest.java @@ -0,0 +1,50 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.logging.slf4j; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; + +import org.apache.logging.log4j.spi.ThreadContextMap; +import org.junit.jupiter.api.Test; +import org.junitpioneer.jupiter.Issue; +import org.slf4j.MDCTestHelper; +import org.slf4j.spi.MDCAdapter; + +class MDCContextMapTest { + + @Test + @Issue("https://github.com/apache/logging-log4j2/issues/1426") + void nonNullGetCopy() { + final ThreadContextMap contextMap = new MDCContextMap(); + final MDCAdapter mockAdapter = mock(MDCAdapter.class); + when(mockAdapter.getCopyOfContextMap()).thenReturn(null); + final MDCAdapter adapter = MDCTestHelper.replaceMDCAdapter(mockAdapter); + try { + assertThat(contextMap.getImmutableMapOrNull()).isNull(); + assertThat(contextMap.getCopy()).isNotNull(); + verify(mockAdapter, times(2)).getCopyOfContextMap(); + verifyNoMoreInteractions(mockAdapter); + } finally { + MDCTestHelper.replaceMDCAdapter(adapter); + } + } +} diff --git a/log4j-to-slf4j/src/test/java/org/slf4j/MDCTestHelper.java b/log4j-to-slf4j/src/test/java/org/slf4j/MDCTestHelper.java new file mode 100644 index 00000000000..0256131b8f2 --- /dev/null +++ b/log4j-to-slf4j/src/test/java/org/slf4j/MDCTestHelper.java @@ -0,0 +1,28 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.slf4j; + +import org.slf4j.spi.MDCAdapter; + +public class MDCTestHelper { + + public static MDCAdapter replaceMDCAdapter(final MDCAdapter adapter) { + final MDCAdapter old = MDC.mdcAdapter; + MDC.mdcAdapter = adapter; + return old; + } +} diff --git a/src/changelog/.2.x.x/fix_NPE_in_closeable_thread_context.xml b/src/changelog/.2.x.x/fix_NPE_in_closeable_thread_context.xml new file mode 100644 index 00000000000..9bd2fc1b0ee --- /dev/null +++ b/src/changelog/.2.x.x/fix_NPE_in_closeable_thread_context.xml @@ -0,0 +1,10 @@ + + + + + Fix NPE in `CloseableThreadContext`. + + 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`. + +