From e74699276bf3a38a13ff3ffefc06d8f0bf39280e Mon Sep 17 00:00:00 2001 From: Ceki Gulcu Date: Thu, 1 Jul 2021 22:37:39 +0200 Subject: [PATCH] Fix SLF4J-414 --- .../org/slf4j/helpers/BasicMDCAdapter.java | 6 ++++- .../main/java/org/slf4j/spi/MDCAdapter.java | 2 ++ .../slf4j/helpers/BasicMDCAdapterTest.java | 13 ++++++++++- .../org/slf4j/log4j12/Log4jMDCAdapter.java | 23 ++++++++++++++----- .../slf4j/log4j12/Log4jMDCAdapterTest.java | 13 +++++++++++ 5 files changed, 49 insertions(+), 8 deletions(-) create mode 100755 slf4j-log4j12/src/test/java/org/slf4j/log4j12/Log4jMDCAdapterTest.java diff --git a/slf4j-api/src/main/java/org/slf4j/helpers/BasicMDCAdapter.java b/slf4j-api/src/main/java/org/slf4j/helpers/BasicMDCAdapter.java index 027938d5..5e1ce694 100644 --- a/slf4j-api/src/main/java/org/slf4j/helpers/BasicMDCAdapter.java +++ b/slf4j-api/src/main/java/org/slf4j/helpers/BasicMDCAdapter.java @@ -141,6 +141,10 @@ public class BasicMDCAdapter implements MDCAdapter { } public void setContextMap(Map contextMap) { - inheritableThreadLocal.set(new HashMap(contextMap)); + Map copy = null; + if(contextMap != null) { + copy = new HashMap(contextMap); + } + inheritableThreadLocal.set(copy); } } diff --git a/slf4j-api/src/main/java/org/slf4j/spi/MDCAdapter.java b/slf4j-api/src/main/java/org/slf4j/spi/MDCAdapter.java index e3bb47ac..927e87b3 100644 --- a/slf4j-api/src/main/java/org/slf4j/spi/MDCAdapter.java +++ b/slf4j-api/src/main/java/org/slf4j/spi/MDCAdapter.java @@ -83,6 +83,8 @@ public interface MDCAdapter { * map and then copying the map passed as parameter. The context map * parameter must only contain keys and values of type String. * + * Implementations must support null valued map passed as parameter. + * * @param contextMap must contain only keys and values of type String * * @since 1.5.1 diff --git a/slf4j-api/src/test/java/org/slf4j/helpers/BasicMDCAdapterTest.java b/slf4j-api/src/test/java/org/slf4j/helpers/BasicMDCAdapterTest.java index 1fbc823b..5f2c4b9f 100644 --- a/slf4j-api/src/test/java/org/slf4j/helpers/BasicMDCAdapterTest.java +++ b/slf4j-api/src/test/java/org/slf4j/helpers/BasicMDCAdapterTest.java @@ -42,8 +42,13 @@ import org.slf4j.spi.MDCAdapter; * @author Lukasz Cwik */ public class BasicMDCAdapterTest { - MDCAdapter mdc = new BasicMDCAdapter(); + protected MDCAdapter mdc = instantiateMDC(); + protected MDCAdapter instantiateMDC() { + return new BasicMDCAdapter(); + } + + // leave MDC clean @After public void tearDown() throws Exception { mdc.clear(); @@ -103,6 +108,12 @@ public class BasicMDCAdapterTest { assertNull(mdc.get("childKey")); } + + @Test + public void testInvokingSetContextMap_WithANullMap_SLF4J_414() { + mdc.setContextMap(null); + } + @Test public void testMDCChildThreadCanOverwriteParentThread() throws Exception { mdc.put("sharedKey", "parentValue"); diff --git a/slf4j-log4j12/src/main/java/org/slf4j/log4j12/Log4jMDCAdapter.java b/slf4j-log4j12/src/main/java/org/slf4j/log4j12/Log4jMDCAdapter.java index 6634a296..9b8bd9b4 100644 --- a/slf4j-log4j12/src/main/java/org/slf4j/log4j12/Log4jMDCAdapter.java +++ b/slf4j-log4j12/src/main/java/org/slf4j/log4j12/Log4jMDCAdapter.java @@ -25,7 +25,6 @@ package org.slf4j.log4j12; import java.util.HashMap; -import java.util.Iterator; import java.util.Map; import org.apache.log4j.MDCFriend; @@ -39,6 +38,7 @@ public class Log4jMDCAdapter implements MDCAdapter { } } + @Override public void clear() { @SuppressWarnings("rawtypes") Map map = org.apache.log4j.MDC.getContext(); @@ -47,6 +47,7 @@ public class Log4jMDCAdapter implements MDCAdapter { } } + @Override public String get(String key) { return (String) org.apache.log4j.MDC.get(key); } @@ -63,10 +64,12 @@ public class Log4jMDCAdapter implements MDCAdapter { * @throws IllegalArgumentException * in case the "key" or "val" parameter is null */ + @Override public void put(String key, String val) { org.apache.log4j.MDC.put(key, val); } + @Override public void remove(String key) { org.apache.log4j.MDC.remove(key); } @@ -82,13 +85,21 @@ public class Log4jMDCAdapter implements MDCAdapter { } @SuppressWarnings({ "rawtypes", "unchecked" }) - public void setContextMap(Map contextMap) { + @Override + public void setContextMap(Map contextMap) { Map old = org.apache.log4j.MDC.getContext(); + + // we must cater for the case where the contextMap argument is null + if(contextMap == null) { + if(old != null) { + old.clear(); + } + return; + } + if (old == null) { - Iterator entrySetIterator = contextMap.entrySet().iterator(); - while (entrySetIterator.hasNext()) { - Map.Entry mapEntry = (Map.Entry) entrySetIterator.next(); - org.apache.log4j.MDC.put((String) mapEntry.getKey(), mapEntry.getValue()); + for (Map.Entry mapEntry : contextMap.entrySet()) { + org.apache.log4j.MDC.put(mapEntry.getKey(), mapEntry); } } else { old.clear(); diff --git a/slf4j-log4j12/src/test/java/org/slf4j/log4j12/Log4jMDCAdapterTest.java b/slf4j-log4j12/src/test/java/org/slf4j/log4j12/Log4jMDCAdapterTest.java new file mode 100755 index 00000000..7d2fdbaa --- /dev/null +++ b/slf4j-log4j12/src/test/java/org/slf4j/log4j12/Log4jMDCAdapterTest.java @@ -0,0 +1,13 @@ +package org.slf4j.log4j12; + +import org.slf4j.helpers.BasicMDCAdapterTest; +import org.slf4j.spi.MDCAdapter; + +public class Log4jMDCAdapterTest extends BasicMDCAdapterTest { + + protected MDCAdapter instantiateMDC() { + return new Log4jMDCAdapter(); + } + + +}