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/25 19:20:20 UTC
[logging-log4j2] branch release-2.x 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 release-2.x
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
The following commit(s) were added to refs/heads/release-2.x by this push:
new 487588b LOG4J2-3289: Fix log4j-to-slf4j re-interpolation of formatted message data
487588b is described below
commit 487588b7c34bc0b540e769d98c42d018fa1bc1b8
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 5ea8b7b..be1a15a 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
@@ -86,10 +86,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) {
@@ -222,31 +225,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 9ba6463..0d9995c 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;
@@ -160,6 +162,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 8f0e24a..8255faa 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -31,6 +31,9 @@
-->
<release version="2.17.1" date="2021-MM-dd" description="GA Release 2.17.1">
<!-- FIXES -->
+ <action issue="LOG4J2-3289" dev="ckozak" type="fix">
+ log4j-to-slf4j no longer re-interpolates formatted message contents.
+ </action>
<action issue="LOG4J2-3288" dev="ckozak" type="fix">
Interpolator non-plugin constructor registers all lookups from log4j-core, previously it was missing event, sd, and bundle.
</action>
Re: [logging-log4j2] branch release-2.x updated: LOG4J2-3289: Fix log4j-to-slf4j re-interpolation of formatted message data
Posted by Apache <ra...@dslextreme.com>.
Please don’t forget to apply this to the master branch
Ralph
> On Dec 25, 2021, at 12:20 PM, ckozak@apache.org wrote:
>
> This is an automated email from the ASF dual-hosted git repository.
>
> ckozak pushed a commit to branch release-2.x
> in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
>
>
> The following commit(s) were added to refs/heads/release-2.x by this push:
> new 487588b LOG4J2-3289: Fix log4j-to-slf4j re-interpolation of formatted message data
> 487588b is described below
>
> commit 487588b7c34bc0b540e769d98c42d018fa1bc1b8
> 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 5ea8b7b..be1a15a 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
> @@ -86,10 +86,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) {
> @@ -222,31 +225,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 9ba6463..0d9995c 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;
>
> @@ -160,6 +162,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 8f0e24a..8255faa 100644
> --- a/src/changes/changes.xml
> +++ b/src/changes/changes.xml
> @@ -31,6 +31,9 @@
> -->
> <release version="2.17.1" date="2021-MM-dd" description="GA Release 2.17.1">
> <!-- FIXES -->
> + <action issue="LOG4J2-3289" dev="ckozak" type="fix">
> + log4j-to-slf4j no longer re-interpolates formatted message contents.
> + </action>
> <action issue="LOG4J2-3288" dev="ckozak" type="fix">
> Interpolator non-plugin constructor registers all lookups from log4j-core, previously it was missing event, sd, and bundle.
> </action>
>