From 05339176c3270beeee7cb43b3fbc99d0925926ca Mon Sep 17 00:00:00 2001 From: Constantin Hirsch <63846074+SiegfriedTobias1@users.noreply.github.com> Date: Mon, 5 Oct 2020 16:52:01 +0200 Subject: [PATCH 1/3] Fix NPE in MDCContextMap Accomodate for the fact that MDC.getCopyOfContextMap() may return null --- .../main/java/org/apache/logging/slf4j/MDCContextMap.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) 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 f03ee54797e..b04d3582362 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 @@ -71,7 +71,8 @@ public void clear() { @Override public boolean containsKey(final String key) { - return MDC.getCopyOfContextMap().containsKey(key); + Map map = MDC.getCopyOfContextMap(); + return map != null && map.containsKey(key); } @Override @@ -88,7 +89,8 @@ public Map getImmutableMapOrNull() { @Override public boolean isEmpty() { - return MDC.getCopyOfContextMap().isEmpty(); + Map map = MDC.getCopyOfContextMap(); + return map == null || map.isEmpty(); } @Override From cd4f02c7e63183420eb3f9f6e582ecbcef676e51 Mon Sep 17 00:00:00 2001 From: Constantin Hirsch <63846074+SiegfriedTobias1@users.noreply.github.com> Date: Mon, 5 Oct 2020 18:34:44 +0200 Subject: [PATCH 2/3] Add test for MDC NPE fix LOG4J2-2939; --- .../test/java/org/apache/logging/slf4j/LoggerTest.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/log4j-to-slf4j/src/test/java/org/apache/logging/slf4j/LoggerTest.java b/log4j-to-slf4j/src/test/java/org/apache/logging/slf4j/LoggerTest.java index e994d192c92..35f72c08033 100644 --- a/log4j-to-slf4j/src/test/java/org/apache/logging/slf4j/LoggerTest.java +++ b/log4j-to-slf4j/src/test/java/org/apache/logging/slf4j/LoggerTest.java @@ -32,6 +32,7 @@ import org.junit.Before; import org.junit.ClassRule; import org.junit.Test; +import org.slf4j.MDC; import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; @@ -165,5 +166,12 @@ public void mdc() { assertThat(list.strList, hasSize(2)); assertTrue("Incorrect year", list.strList.get(0).startsWith("2010")); } + + @Test + public void mdcNullBacked() { + assertNull("Setup wrong", MDC.getCopyOfContextMap()); + assertTrue(ThreadContext.isEmpty()); + assertFalse(ThreadContext.containsKey("something")); + } } From 544e57c2730a91027610f8d330f0859f66c438ff Mon Sep 17 00:00:00 2001 From: Constantin Hirsch <63846074+SiegfriedTobias1@users.noreply.github.com> Date: Tue, 6 Oct 2020 12:55:47 +0200 Subject: [PATCH 3/3] Add more tests for MDC NPE fix LOG4J2-2939; --- .../org/apache/logging/slf4j/LoggerTest.java | 35 +++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/log4j-to-slf4j/src/test/java/org/apache/logging/slf4j/LoggerTest.java b/log4j-to-slf4j/src/test/java/org/apache/logging/slf4j/LoggerTest.java index 35f72c08033..9cf7deb78d0 100644 --- a/log4j-to-slf4j/src/test/java/org/apache/logging/slf4j/LoggerTest.java +++ b/log4j-to-slf4j/src/test/java/org/apache/logging/slf4j/LoggerTest.java @@ -168,10 +168,41 @@ public void mdc() { } @Test - public void mdcNullBacked() { + public void mdcNullBackedIsEmpty() { assertNull("Setup wrong", MDC.getCopyOfContextMap()); assertTrue(ThreadContext.isEmpty()); + } + + @Test + public void mdcNullBackedContainsKey() { + assertNull("Setup wrong", MDC.getCopyOfContextMap()); assertFalse(ThreadContext.containsKey("something")); } -} + @Test + public void mdcNullBackedContainsNullKey() { + assertNull("Setup wrong", MDC.getCopyOfContextMap()); + assertFalse(ThreadContext.containsKey(null)); + } + + @Test + public void mdcContainsNullKey() { + try { + ThreadContext.put("some", "thing"); + assertNotNull("Setup wrong", MDC.getCopyOfContextMap()); + assertFalse(ThreadContext.containsKey(null)); + } finally { + ThreadContext.clearMap(); + } + } + + @Test + public void mdcCannotContainNullKey() { + try { + ThreadContext.put(null, "something"); + fail("should throw"); + } catch (IllegalArgumentException | NullPointerException e) { + // expected + } + } +}