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
commit 609ff5f284e5e916c01b24feeb383cfeebc7d0cb Author: Carter Kozak <[email protected]> AuthorDate: Tue Aug 3 10:06:45 2021 -0400 LOG4J2-3083 Fix slf4j calling class lookup using both accessors `LoggerFactory.getLogger(name/class)` as well as `LoggerFactory.getILoggerFactory().getLogger(name)`. --- .../apache/logging/log4j/util/StackLocator.java | 17 ++++ .../apache/logging/log4j/util/StackLocator.java | 21 +++++ .../logging/log4j/util/StackLocatorUtil.java | 13 ++++ .../apache/logging/slf4j/Log4jLoggerFactory.java | 13 +++- .../logging/other/pkg/LoggerContextAnchorTest.java | 91 ++++++++++++++++++++++ .../apache/logging/slf4j/Log4jLoggerFactory.java | 15 +++- .../logging/other/pkg/LoggerContextAnchorTest.java | 91 ++++++++++++++++++++++ src/changes/changes.xml | 4 + 8 files changed, 258 insertions(+), 7 deletions(-) diff --git a/log4j-api-java9/src/main/java/org/apache/logging/log4j/util/StackLocator.java b/log4j-api-java9/src/main/java/org/apache/logging/log4j/util/StackLocator.java index 2740fcf..2f78dd7 100644 --- a/log4j-api-java9/src/main/java/org/apache/logging/log4j/util/StackLocator.java +++ b/log4j-api-java9/src/main/java/org/apache/logging/log4j/util/StackLocator.java @@ -19,6 +19,7 @@ package org.apache.logging.log4j.util; import java.util.List; import java.util.Stack; import java.util.function.Function; +import java.util.function.Predicate; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -40,6 +41,22 @@ public class StackLocator { private StackLocator() { } + public Class<?> getCallerClass(final Class<?> sentinelClass, final Predicate<Class<?>> callerPredicate) { + if (sentinelClass == null) { + throw new IllegalArgumentException("sentinelClass cannot be null"); + } + if (callerPredicate == null) { + throw new IllegalArgumentException("callerPredicate cannot be null"); + } + return walker.walk(s -> s + .map(StackWalker.StackFrame::getDeclaringClass) + // Skip until the sentinel class is found + .dropWhile(clazz -> !sentinelClass.equals(clazz)) + // Skip until the predicate evaluates to true, also ignoring recurrences of the sentinel + .dropWhile(clazz -> sentinelClass.equals(clazz) || !callerPredicate.test(clazz)) + .findFirst().orElse(null)); + } + public Class<?> getCallerClass(final String fqcn) { return getCallerClass(fqcn, ""); } diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/util/StackLocator.java b/log4j-api/src/main/java/org/apache/logging/log4j/util/StackLocator.java index cbe70a9..5b919bb 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/util/StackLocator.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/util/StackLocator.java @@ -18,6 +18,7 @@ package org.apache.logging.log4j.util; import java.lang.reflect.Method; import java.util.Stack; +import java.util.function.Predicate; /** * <em>Consider this class private.</em> Provides various methods to determine the caller class. <h3>Background</h3> @@ -96,6 +97,26 @@ public final class StackLocator { // TODO: return Object.class instead of null (though it will have a null ClassLoader) // (MS) I believe this would work without any modifications elsewhere, but I could be wrong + @PerformanceSensitive + public Class<?> getCallerClass(final Class<?> sentinelClass, final Predicate<Class<?>> callerPredicate) { + if (sentinelClass == null) { + throw new IllegalArgumentException("sentinelClass cannot be null"); + } + if (callerPredicate == null) { + throw new IllegalArgumentException("callerPredicate cannot be null"); + } + boolean foundSentinel = false; + Class<?> clazz; + for (int i = 2; null != (clazz = getCallerClass(i)); i++) { + if (sentinelClass.equals(clazz)) { + foundSentinel = true; + } else if (foundSentinel && callerPredicate.test(clazz)) { + return clazz; + } + } + return null; + } + // migrated from ReflectiveCallerClassUtility @PerformanceSensitive public Class<?> getCallerClass(final int depth) { diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/util/StackLocatorUtil.java b/log4j-api/src/main/java/org/apache/logging/log4j/util/StackLocatorUtil.java index 7f1dc7e..1b98a23 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/util/StackLocatorUtil.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/util/StackLocatorUtil.java @@ -18,6 +18,7 @@ package org.apache.logging.log4j.util; import java.util.NoSuchElementException; import java.util.Stack; +import java.util.function.Predicate; import org.apache.logging.log4j.status.StatusLogger; @@ -78,6 +79,18 @@ public final class StackLocatorUtil { return stackLocator.getCallerClass(fqcn, pkg, skipDepth); } + /** + * Search for a calling class. + * + * @param sentinelClass Sentinel class at which to begin searching + * @param callerPredicate Predicate checked after the sentinelClass is found + * @return the first matching class after <code>sentinelClass</code> is found. + */ + @PerformanceSensitive + public static Class<?> getCallerClass(final Class<?> sentinelClass, final Predicate<Class<?>> callerPredicate) { + return stackLocator.getCallerClass(sentinelClass, callerPredicate); + } + // added for use in LoggerAdapter implementations mainly @PerformanceSensitive public static Class<?> getCallerClass(final Class<?> anchor) { diff --git a/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jLoggerFactory.java b/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jLoggerFactory.java index 578d182..d884e6f 100644 --- a/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jLoggerFactory.java +++ b/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jLoggerFactory.java @@ -20,18 +20,23 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.LoggingException; import org.apache.logging.log4j.spi.AbstractLoggerAdapter; import org.apache.logging.log4j.spi.LoggerContext; +import org.apache.logging.log4j.status.StatusLogger; import org.apache.logging.log4j.util.StackLocatorUtil; import org.slf4j.ILoggerFactory; import org.slf4j.Logger; +import java.util.function.Predicate; + /** * Log4j implementation of SLF4J ILoggerFactory interface. */ public class Log4jLoggerFactory extends AbstractLoggerAdapter<Logger> implements ILoggerFactory { - private static final String FQCN = Log4jLoggerFactory.class.getName(); - private static final String PACKAGE = "org.slf4j"; + private static final StatusLogger LOGGER = StatusLogger.getLogger(); + private static final String SLF4J_PACKAGE = "org.slf4j"; private static final String TO_SLF4J_CONTEXT = "org.apache.logging.slf4j.SLF4JLoggerContext"; + private static final Predicate<Class<?>> CALLER_PREDICATE = clazz -> + !AbstractLoggerAdapter.class.equals(clazz) && !clazz.getName().startsWith(SLF4J_PACKAGE); @Override protected Logger newLogger(final String name, final LoggerContext context) { @@ -42,12 +47,14 @@ public class Log4jLoggerFactory extends AbstractLoggerAdapter<Logger> implements @Override protected LoggerContext getContext() { final Class<?> anchor = LogManager.getFactory().isClassLoaderDependent() - ? StackLocatorUtil.getCallerClass(FQCN, PACKAGE, 1) + ? StackLocatorUtil.getCallerClass(Log4jLoggerFactory.class, CALLER_PREDICATE) : null; + LOGGER.trace("Log4jLoggerFactory.getContext() found anchor {}", anchor); return anchor == null ? LogManager.getContext(false) : getContext(anchor); } + private LoggerContext validateContext(final LoggerContext context) { if (TO_SLF4J_CONTEXT.equals(context.getClass().getName())) { throw new LoggingException("log4j-slf4j-impl cannot be present with log4j-to-slf4j"); diff --git a/log4j-slf4j-impl/src/test/java/org/apache/logging/other/pkg/LoggerContextAnchorTest.java b/log4j-slf4j-impl/src/test/java/org/apache/logging/other/pkg/LoggerContextAnchorTest.java new file mode 100644 index 0000000..2b0fe91 --- /dev/null +++ b/log4j-slf4j-impl/src/test/java/org/apache/logging/other/pkg/LoggerContextAnchorTest.java @@ -0,0 +1,91 @@ +/* + * 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.other.pkg; + +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.status.StatusData; +import org.apache.logging.log4j.status.StatusListener; +import org.apache.logging.log4j.status.StatusLogger; +import org.junit.Test; +import org.slf4j.LoggerFactory; + +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; + +import static org.junit.Assert.assertEquals; + +/** + * Test LoggerContext lookups by verifying the anchor class representing calling code. + */ +public class LoggerContextAnchorTest { + private static final String PREFIX = "Log4jLoggerFactory.getContext() found anchor class "; + + @Test + public void testLoggerFactoryLookupClass() { + String fqcn = getAnchorFqcn(() -> LoggerFactory.getLogger(LoggerContextAnchorTest.class)); + assertEquals(getClass().getName(), fqcn); + } + + @Test + public void testLoggerFactoryLookupString() { + String fqcn = getAnchorFqcn(() -> LoggerFactory.getLogger("custom.logger")); + assertEquals(getClass().getName(), fqcn); + } + + @Test + public void testLoggerFactoryGetILoggerFactoryLookup() { + String fqcn = getAnchorFqcn(() -> LoggerFactory.getILoggerFactory().getLogger("custom.logger")); + assertEquals(getClass().getName(), fqcn); + } + + private static String getAnchorFqcn(Runnable runnable) { + List<String> results = new CopyOnWriteArrayList<>(); + StatusListener listener = new StatusListener() { + @Override + public void log(StatusData data) { + String formattedMessage = data.getMessage().getFormattedMessage(); + if (formattedMessage.startsWith(PREFIX)) { + results.add(formattedMessage.substring(PREFIX.length())); + } + } + + @Override + public Level getStatusLevel() { + return Level.TRACE; + } + + @Override + public void close() { + // nop + } + }; + StatusLogger statusLogger = StatusLogger.getLogger(); + statusLogger.registerListener(listener); + try { + runnable.run(); + if (results.isEmpty()) { + throw new AssertionError("Failed to locate an anchor lookup status message"); + } + if (results.size() > 1) { + throw new AssertionError("Found multiple anchor lines: " + results); + } + return results.get(0); + } finally { + statusLogger.removeListener(listener); + } + } +} diff --git a/log4j-slf4j18-impl/src/main/java/org/apache/logging/slf4j/Log4jLoggerFactory.java b/log4j-slf4j18-impl/src/main/java/org/apache/logging/slf4j/Log4jLoggerFactory.java index be29dad..99e6817 100644 --- a/log4j-slf4j18-impl/src/main/java/org/apache/logging/slf4j/Log4jLoggerFactory.java +++ b/log4j-slf4j18-impl/src/main/java/org/apache/logging/slf4j/Log4jLoggerFactory.java @@ -20,20 +20,26 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.LoggingException; import org.apache.logging.log4j.spi.AbstractLoggerAdapter; import org.apache.logging.log4j.spi.LoggerContext; +import org.apache.logging.log4j.status.StatusLogger; import org.apache.logging.log4j.util.StackLocatorUtil; import org.slf4j.ILoggerFactory; import org.slf4j.Logger; +import java.util.function.Predicate; + /** * Log4j implementation of SLF4J ILoggerFactory interface. */ public class Log4jLoggerFactory extends AbstractLoggerAdapter<Logger> implements ILoggerFactory { - private static final String FQCN = Log4jLoggerFactory.class.getName(); - private static final String PACKAGE = "org.slf4j"; - private final Log4jMarkerFactory markerFactory; + private static final StatusLogger LOGGER = StatusLogger.getLogger(); + private static final String SLF4J_PACKAGE = "org.slf4j"; + private static final Predicate<Class<?>> CALLER_PREDICATE = clazz -> + !AbstractLoggerAdapter.class.equals(clazz) && !clazz.getName().startsWith(SLF4J_PACKAGE); private static final String TO_SLF4J_CONTEXT = "org.apache.logging.slf4j.SLF4JLoggerContext"; + private final Log4jMarkerFactory markerFactory; + public Log4jLoggerFactory(final Log4jMarkerFactory markerFactory) { this.markerFactory = markerFactory; } @@ -48,8 +54,9 @@ public class Log4jLoggerFactory extends AbstractLoggerAdapter<Logger> implements @Override protected LoggerContext getContext() { final Class<?> anchor = LogManager.getFactory().isClassLoaderDependent() - ? StackLocatorUtil.getCallerClass(FQCN, PACKAGE, 1) + ? StackLocatorUtil.getCallerClass(Log4jLoggerFactory.class, CALLER_PREDICATE) : null; + LOGGER.trace("Log4jLoggerFactory.getContext() found anchor {}", anchor); return anchor == null ? LogManager.getContext(false) : getContext(anchor); diff --git a/log4j-slf4j18-impl/src/test/java/org/apache/logging/other/pkg/LoggerContextAnchorTest.java b/log4j-slf4j18-impl/src/test/java/org/apache/logging/other/pkg/LoggerContextAnchorTest.java new file mode 100644 index 0000000..2b0fe91 --- /dev/null +++ b/log4j-slf4j18-impl/src/test/java/org/apache/logging/other/pkg/LoggerContextAnchorTest.java @@ -0,0 +1,91 @@ +/* + * 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.other.pkg; + +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.status.StatusData; +import org.apache.logging.log4j.status.StatusListener; +import org.apache.logging.log4j.status.StatusLogger; +import org.junit.Test; +import org.slf4j.LoggerFactory; + +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; + +import static org.junit.Assert.assertEquals; + +/** + * Test LoggerContext lookups by verifying the anchor class representing calling code. + */ +public class LoggerContextAnchorTest { + private static final String PREFIX = "Log4jLoggerFactory.getContext() found anchor class "; + + @Test + public void testLoggerFactoryLookupClass() { + String fqcn = getAnchorFqcn(() -> LoggerFactory.getLogger(LoggerContextAnchorTest.class)); + assertEquals(getClass().getName(), fqcn); + } + + @Test + public void testLoggerFactoryLookupString() { + String fqcn = getAnchorFqcn(() -> LoggerFactory.getLogger("custom.logger")); + assertEquals(getClass().getName(), fqcn); + } + + @Test + public void testLoggerFactoryGetILoggerFactoryLookup() { + String fqcn = getAnchorFqcn(() -> LoggerFactory.getILoggerFactory().getLogger("custom.logger")); + assertEquals(getClass().getName(), fqcn); + } + + private static String getAnchorFqcn(Runnable runnable) { + List<String> results = new CopyOnWriteArrayList<>(); + StatusListener listener = new StatusListener() { + @Override + public void log(StatusData data) { + String formattedMessage = data.getMessage().getFormattedMessage(); + if (formattedMessage.startsWith(PREFIX)) { + results.add(formattedMessage.substring(PREFIX.length())); + } + } + + @Override + public Level getStatusLevel() { + return Level.TRACE; + } + + @Override + public void close() { + // nop + } + }; + StatusLogger statusLogger = StatusLogger.getLogger(); + statusLogger.registerListener(listener); + try { + runnable.run(); + if (results.isEmpty()) { + throw new AssertionError("Failed to locate an anchor lookup status message"); + } + if (results.size() > 1) { + throw new AssertionError("Found multiple anchor lines: " + results); + } + return results.get(0); + } finally { + statusLogger.removeListener(listener); + } + } +} diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 9b4c4cf..e5d41a9 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -81,6 +81,10 @@ Allow a PatternSelector to be specified on GelfLayout. </action> <!-- FIXES --> + <action issue="LOG4J2-3083" dev="ckozak" type="fix"> + log4j-slf4j-impl and log4j-slf4j18-impl correctly detect the calling class using both LoggerFactory.getLogger + methods as well as LoggerFactory.getILoggerFactory().getLogger. + </action> <action issue="LOG4J2-2816" dev="vy" type="fix" due-to="Jacob Shields"> Handle Disruptor event translation exceptions. </action>
