You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by pk...@apache.org on 2023/12/21 13:49:04 UTC
(logging-log4j2) branch main updated (0dc01025f1 -> 419fc5e51b)
This is an automated email from the ASF dual-hosted git repository.
pkarwasz pushed a change to branch main
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
from 0dc01025f1 Remove incorrectly passed configuration in `HtmlLayoutBuilder`
add c6d06f40c7 Mark `JdkMapAdapterStringMap` as frozen if map is immutable
new 5c3430216b Fix NPE in `RollingFileManger`
new 419fc5e51b Fix NPE caused by MDCContextMap
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../appender/rolling/RollingFileManagerTest.java | 109 ++++++++++++++++++++-
.../core/impl/JdkMapAdapterStringMapTest.java | 20 ++++
.../core/appender/rolling/RollingFileManager.java | 6 +-
.../log4j/core/impl/JdkMapAdapterStringMap.java | 5 +
log4j-to-slf4j/pom.xml | 5 +
.../org/apache/logging/slf4j/MDCContextMap.java | 6 +-
.../apache/logging/slf4j/MDCContextMapTest.java | 50 ++++++++++
.../src/test/java/org/slf4j/MDCTestHelper.java | 12 ++-
...JdkMapAdapterStringMap_detect_immutability.xml} | 6 +-
...xml => fix_NPE_in_closeable_thread_context.xml} | 6 +-
...nager_support.xml => fix_create_parent_dir.xml} | 6 +-
11 files changed, 209 insertions(+), 22 deletions(-)
create mode 100644 log4j-to-slf4j/src/test/java/org/apache/logging/slf4j/MDCContextMapTest.java
copy log4j-1.2-api/src/test/java/org/apache/log4j/util/UnexpectedFormatException.java => log4j-to-slf4j/src/test/java/org/slf4j/MDCTestHelper.java (76%)
copy src/changelog/.3.x.x/{change_basic_auth_encoding.xml => 2098_JdkMapAdapterStringMap_detect_immutability.xml} (67%)
copy src/changelog/.3.x.x/{remove_security_manager_support.xml => fix_NPE_in_closeable_thread_context.xml} (68%)
copy src/changelog/.3.x.x/{remove_security_manager_support.xml => fix_create_parent_dir.xml} (68%)
(logging-log4j2) 02/02: Fix NPE caused by MDCContextMap
Posted by pk...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
pkarwasz pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit 419fc5e51ba8c22ffcfe7a0570d64aac8a6639bd
Author: Piotr P. Karwasz <pi...@karwasz.org>
AuthorDate: Wed Dec 20 12:38:10 2023 +0100
Fix NPE caused by MDCContextMap
---
log4j-to-slf4j/pom.xml | 5 +++
.../org/apache/logging/slf4j/MDCContextMap.java | 6 +--
.../apache/logging/slf4j/MDCContextMapTest.java | 50 ++++++++++++++++++++++
.../src/test/java/org/slf4j/MDCTestHelper.java | 28 ++++++++++++
.../.3.x.x/fix_NPE_in_closeable_thread_context.xml | 10 +++++
5 files changed, 96 insertions(+), 3 deletions(-)
diff --git a/log4j-to-slf4j/pom.xml b/log4j-to-slf4j/pom.xml
index 03d2128d6f..f92ff4d06d 100644
--- a/log4j-to-slf4j/pom.xml
+++ b/log4j-to-slf4j/pom.xml
@@ -98,6 +98,11 @@
<artifactId>log4j-api-test</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.mockito</groupId>
+ <artifactId>mockito-core</artifactId>
+ <scope>test</scope>
+ </dependency>
</dependencies>
<build>
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 186cb62608..f2e3463e7e 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.ThreadContextMap;
@@ -75,13 +76,12 @@ public class MDCContextMap implements ThreadContextMap {
}
@Override
- @SuppressWarnings("unchecked") // nothing we can do about this, restricted by SLF4J API
public Map<String, String> getCopy() {
- return MDC.getCopyOfContextMap();
+ final Map<String, String> contextMap = MDC.getCopyOfContextMap();
+ return contextMap != null ? contextMap : new HashMap<>();
}
@Override
- @SuppressWarnings("unchecked") // nothing we can do about this, restricted by SLF4J API
public Map<String, String> 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 0000000000..6673de4597
--- /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 0000000000..0256131b8f
--- /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/.3.x.x/fix_NPE_in_closeable_thread_context.xml b/src/changelog/.3.x.x/fix_NPE_in_closeable_thread_context.xml
new file mode 100644
index 0000000000..9bd2fc1b0e
--- /dev/null
+++ b/src/changelog/.3.x.x/fix_NPE_in_closeable_thread_context.xml
@@ -0,0 +1,10 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xmlns="http://logging.apache.org/log4j/changelog"
+ xsi:schemaLocation="http://logging.apache.org/log4j/changelog https://logging.apache.org/log4j/changelog-0.1.2.xsd"
+ type="fixed">
+ <issue id="1426" link="https://github.com/apache/logging-log4j2/pull/1426"/>
+ <description format="asciidoc">
+ Fix NPE in `CloseableThreadContext`.
+ </description>
+</entry>
(logging-log4j2) 01/02: Fix NPE in `RollingFileManger`
Posted by pk...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
pkarwasz pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit 5c3430216ba478b7cb6db6e73f05652c4b225dbb
Author: Piotr P. Karwasz <pi...@karwasz.org>
AuthorDate: Wed Dec 20 10:36:40 2023 +0100
Fix NPE in `RollingFileManger`
This PR fixes a `NullPointerException` in
`RollingFileManager#createParentDir`.
Closes #1645
---
.../appender/rolling/RollingFileManagerTest.java | 109 ++++++++++++++++++++-
.../core/appender/rolling/RollingFileManager.java | 6 +-
src/changelog/.3.x.x/fix_create_parent_dir.xml | 10 ++
3 files changed, 120 insertions(+), 5 deletions(-)
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 2861d704df..51b22edfdb 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,16 +16,25 @@
*/
package org.apache.logging.log4j.core.appender.rolling;
+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.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import org.apache.logging.log4j.core.LoggerContext;
import org.apache.logging.log4j.core.appender.RollingFileAppender;
+import org.apache.logging.log4j.core.appender.rolling.action.AbstractAction;
import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.NullConfiguration;
+import org.apache.logging.log4j.core.layout.PatternLayout;
import org.apache.logging.log4j.core.lookup.StrSubstitutor;
-import org.junit.Assert;
import org.junit.Test;
+import org.junitpioneer.jupiter.Issue;
public class RollingFileManagerTest {
@@ -73,14 +82,106 @@ public class RollingFileManagerTest {
.setPolicy(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());
}
final String actualContents = Files.readString(file.toPath(), StandardCharsets.US_ASCII);
- Assert.assertEquals(testContent, actualContents);
+ assertEquals(testContent, actualContents);
+ }
+ }
+
+ /**
+ * Test that a synchronous action failure does not cause a rollover. Addresses Issue #1445.
+ */
+ @Test
+ public void testSynchronousActionFailure() throws IOException {
+ class FailingSynchronousAction extends AbstractAction {
+ @Override
+ public boolean execute() {
+ return false;
+ }
+ }
+ class FailingSynchronousStrategy implements RolloverStrategy {
+ @Override
+ public RolloverDescription rollover(final RollingFileManager manager) throws SecurityException {
+ return new RolloverDescriptionImpl(manager.getFileName(), false, new FailingSynchronousAction(), null);
+ }
+ }
+
+ final Configuration configuration = new NullConfiguration();
+
+ // Create the manager.
+ final File file = File.createTempFile("testSynchronousActionFailure", "log");
+ final RollingFileManager manager = RollingFileManager.getFileManager(
+ file.getAbsolutePath(),
+ "testSynchronousActionFailure.log.%d{yyyy-MM-dd}",
+ true,
+ false,
+ OnStartupTriggeringPolicy.createPolicy(1),
+ new FailingSynchronousStrategy(),
+ null,
+ PatternLayout.createDefaultLayout(),
+ 0,
+ true,
+ false,
+ null,
+ null,
+ null,
+ configuration);
+ assertNotNull(manager);
+ manager.initialize();
+
+ // Get the initialTime of this original log file
+ final long initialTime = manager.getFileTime();
+
+ // Log something to ensure that the existing file size is > 0
+ final String testContent = "Test";
+ manager.writeToDestination(testContent.getBytes(StandardCharsets.US_ASCII), 0, testContent.length());
+
+ // Trigger rollover that will fail
+ manager.rollover();
+
+ // If the rollover fails, then the size should not be reset
+ assertNotEquals(0, manager.getFileSize());
+
+ // The initialTime should not have changed
+ 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()
+ .setConfig(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 923fc8b13f..65e5a728c0 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
@@ -253,7 +253,11 @@ public class RollingFileManager extends FileManager {
@Override
protected void createParentDir(final 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/.3.x.x/fix_create_parent_dir.xml b/src/changelog/.3.x.x/fix_create_parent_dir.xml
new file mode 100644
index 0000000000..314076edfb
--- /dev/null
+++ b/src/changelog/.3.x.x/fix_create_parent_dir.xml
@@ -0,0 +1,10 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xmlns="http://logging.apache.org/log4j/changelog"
+ xsi:schemaLocation="http://logging.apache.org/log4j/changelog https://logging.apache.org/log4j/changelog-0.1.2.xsd"
+ type="fixed">
+ <issue id="1645" link="https://github.com/apache/logging-log4j2/pull/1645"/>
+ <description format="asciidoc">
+ Fix NPE in `RollingFileManager`.
+ </description>
+</entry>