You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by ck...@apache.org on 2021/12/26 20:42:09 UTC

[logging-log4j2] branch master updated: LOG4J2-3289: Fix log4j-to-slf4j re-interpolation of formatted message data

This is an automated email from the ASF dual-hosted git repository.

ckozak pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git


The following commit(s) were added to refs/heads/master by this push:
     new 212054e  LOG4J2-3289: Fix log4j-to-slf4j re-interpolation of formatted message data
212054e is described below

commit 212054e205ba87f1ca0825aa0e5354afa2687381
Author: Carter Kozak <ck...@apache.org>
AuthorDate: Sat Dec 25 11:48:20 2021 -0500

    LOG4J2-3289: Fix log4j-to-slf4j re-interpolation of formatted message data
---
 .../java/org/apache/logging/slf4j/SLF4JLogger.java | 28 ++++++++++--------
 .../java/org/apache/logging/slf4j/LoggerTest.java  | 33 ++++++++++++++++++++++
 src/changes/changes.xml                            |  3 ++
 3 files changed, 52 insertions(+), 12 deletions(-)

diff --git a/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/SLF4JLogger.java b/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/SLF4JLogger.java
index 33e40c1..0420f72 100644
--- a/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/SLF4JLogger.java
+++ b/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/SLF4JLogger.java
@@ -89,10 +89,13 @@ public class SLF4JLogger extends AbstractLogger {
         return locationAwareLogger != null ? locationAwareLogger : logger;
     }
 
-    private org.slf4j.Marker getMarker(final Marker marker) {
-        if (marker == null) {
-            return null;
-        }
+    private static org.slf4j.Marker getMarker(final Marker marker) {
+        // No marker is provided in the common case, small methods
+        // are optimized more effectively.
+        return marker == null ? null : convertMarker(marker);
+    }
+
+    private static org.slf4j.Marker convertMarker(final Marker marker) {
         final org.slf4j.Marker slf4jMarker = MarkerFactory.getMarker(marker.getName());
         final Marker[] parents = marker.getParents();
         if (parents != null) {
@@ -225,31 +228,32 @@ public class SLF4JLogger extends AbstractLogger {
 
     @Override
     public void logMessage(final String fqcn, final Level level, final Marker marker, final Message message, final Throwable t) {
+        org.slf4j.Marker slf4jMarker = getMarker(marker);
+        String formattedMessage = message.getFormattedMessage();
         if (locationAwareLogger != null) {
             if (message instanceof LoggerNameAwareMessage) {
                 ((LoggerNameAwareMessage) message).setLoggerName(getName());
             }
-            locationAwareLogger.log(getMarker(marker), fqcn, convertLevel(level), message.getFormattedMessage(),
-                    message.getParameters(), t);
+            locationAwareLogger.log(slf4jMarker, fqcn, convertLevel(level), formattedMessage, null, t);
         } else {
             switch (level.getStandardLevel()) {
                 case DEBUG :
-                    logger.debug(getMarker(marker), message.getFormattedMessage(), message.getParameters(), t);
+                    logger.debug(slf4jMarker, formattedMessage, t);
                     break;
                 case TRACE :
-                    logger.trace(getMarker(marker), message.getFormattedMessage(), message.getParameters(), t);
+                    logger.trace(slf4jMarker, formattedMessage, t);
                     break;
                 case INFO :
-                    logger.info(getMarker(marker), message.getFormattedMessage(), message.getParameters(), t);
+                    logger.info(slf4jMarker, formattedMessage, t);
                     break;
                 case WARN :
-                    logger.warn(getMarker(marker), message.getFormattedMessage(), message.getParameters(), t);
+                    logger.warn(slf4jMarker, formattedMessage, t);
                     break;
                 case ERROR :
-                    logger.error(getMarker(marker), message.getFormattedMessage(), message.getParameters(), t);
+                    logger.error(slf4jMarker, formattedMessage, t);
                     break;
                 default :
-                    logger.error(getMarker(marker), message.getFormattedMessage(), message.getParameters(), t);
+                    logger.error(slf4jMarker, formattedMessage, t);
                     break;
             }
         }
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 9cf7deb..7485e8a 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
@@ -17,6 +17,8 @@
 */
 package org.apache.logging.slf4j;
 
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Proxy;
 import java.util.Date;
 import java.util.List;
 
@@ -145,6 +147,37 @@ public class LoggerTest {
     public void debugWithParms() {
         logger.debug("Hello, {}", "World");
         assertThat(list.strList, hasSize(1));
+        String message = list.strList.get(0);
+        assertEquals("Hello, World", message);
+    }
+
+    @Test
+    public void paramIncludesSubstitutionMarker_locationAware() {
+        logger.info("Hello, {}", "foo {} bar");
+        assertThat(list.strList, hasSize(1));
+        String message = list.strList.get(0);
+        assertEquals("Hello, foo {} bar", message);
+    }
+
+    @Test
+    public void paramIncludesSubstitutionMarker_nonLocationAware() {
+        final org.slf4j.Logger slf4jLogger = CTX.getLogger();
+        Logger nonLocationAwareLogger = new SLF4JLogger(
+                slf4jLogger.getName(),
+                (org.slf4j.Logger) Proxy.newProxyInstance(
+                        getClass().getClassLoader(),
+                        new Class<?>[]{org.slf4j.Logger.class},
+                        (proxy, method, args) -> {
+                            try {
+                                return method.invoke(slf4jLogger, args);
+                            } catch (InvocationTargetException e) {
+                                throw e.getCause();
+                            }
+                        }));
+        nonLocationAwareLogger.info("Hello, {}", "foo {} bar");
+        assertThat(list.strList, hasSize(1));
+        String message = list.strList.get(0);
+        assertEquals("Hello, foo {} bar", message);
     }
 
     @Test
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 370bb80..eefd920 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -172,6 +172,9 @@
       </action>
     </release>
     <release version="2.17.1" date="2021-MM-DD" description="GA Release 2.17.1">
+      <action issue="LOG4J2-3289" dev="ckozak" type="fix">
+        log4j-to-slf4j no longer re-interpolates formatted message contents.
+      </action>
       <action issue="LOG4J2-3270" dev="ggregory" type="fix" due-to="Lee Dongjin">
         Reduce ignored package scope of KafkaAppender.
       </action>