You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by rg...@apache.org on 2019/05/07 05:05:04 UTC

[logging-log4j2] branch release-2.x updated: LOG4J2-2597 - Throw better exception message when both log4j-slf4j-impl and log4j-to-slf4j are present

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

rgoers 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 aa73eb5  LOG4J2-2597 - Throw better exception message when both log4j-slf4j-impl and log4j-to-slf4j are present
aa73eb5 is described below

commit aa73eb528e35217ce29d520449666e3d655323b6
Author: Ralph Goers <rg...@apache.org>
AuthorDate: Mon May 6 22:04:34 2019 -0700

    LOG4J2-2597 - Throw better exception message when both log4j-slf4j-impl and log4j-to-slf4j are present
---
 log4j-slf4j-impl/pom.xml                           | 42 +++++++++++++++++++++
 .../apache/logging/slf4j/Log4jLoggerFactory.java   | 11 +++++-
 .../org/apache/logging/slf4j/OverflowTest.java     | 43 ++++++++++++++++++++++
 log4j-slf4j18-impl/pom.xml                         | 42 +++++++++++++++++++++
 .../apache/logging/slf4j/Log4jLoggerFactory.java   | 12 ++++--
 .../org/apache/logging/slf4j/OverflowTest.java     | 43 ++++++++++++++++++++++
 src/changes/changes.xml                            |  3 ++
 7 files changed, 191 insertions(+), 5 deletions(-)

diff --git a/log4j-slf4j-impl/pom.xml b/log4j-slf4j-impl/pom.xml
index de3ec3c..00f77c5 100644
--- a/log4j-slf4j-impl/pom.xml
+++ b/log4j-slf4j-impl/pom.xml
@@ -77,6 +77,12 @@
       <scope>test</scope>
     </dependency>
     <dependency>
+      <groupId>org.apache.logging.log4j</groupId>
+      <artifactId>log4j-to-slf4j</artifactId>
+      <scope>test</scope>
+      <version>${project.version}</version>
+    </dependency>
+    <dependency>
       <groupId>junit</groupId>
       <artifactId>junit</artifactId>
       <scope>test</scope>
@@ -150,6 +156,42 @@
         </executions>
       </plugin>
       <plugin>
+        <groupId>org.apache.maven.plugins</groupId>
+        <artifactId>maven-surefire-plugin</artifactId>
+        <executions>
+          <execution>
+            <id>loop-test</id>
+            <phase>test</phase>
+            <goals>
+              <goal>test</goal>
+            </goals>
+            <configuration>
+              <includes>
+                <include>**/OverflowTest.java</include>
+              </includes>
+            </configuration>
+          </execution>
+          <execution>
+            <id>default-test</id>
+            <phase>test</phase>
+            <goals>
+              <goal>test</goal>
+            </goals>
+            <configuration>
+              <includes>
+                <include>**/*Test.java</include>
+              </includes>
+              <excludes>
+                <exclude>**/OverflowTest.java</exclude>
+              </excludes>
+              <classpathDependencyExcludes>
+                <classpathDependencyExcludes>org.apache.logging.log4j:log4j-to-slf4j</classpathDependencyExcludes>
+              </classpathDependencyExcludes>
+            </configuration>
+          </execution>
+        </executions>
+      </plugin>
+      <plugin>
         <groupId>org.apache.felix</groupId>
         <artifactId>maven-bundle-plugin</artifactId>
         <configuration>
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 37cff9f..b54ed9e 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
@@ -17,6 +17,7 @@
 package org.apache.logging.slf4j;
 
 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.util.StackLocatorUtil;
@@ -30,11 +31,12 @@ public class Log4jLoggerFactory extends AbstractLoggerAdapter<Logger> implements
 
     private static final String FQCN = Log4jLoggerFactory.class.getName();
     private static final String PACKAGE = "org.slf4j";
+    private static final String TO_SLF4J_CONTEXT = "org.apache.logging.slf4j.SLF4JLoggerContext";
 
     @Override
     protected Logger newLogger(final String name, final LoggerContext context) {
         final String key = Logger.ROOT_LOGGER_NAME.equals(name) ? LogManager.ROOT_LOGGER_NAME : name;
-        return new Log4jLogger(context.getLogger(key), name);
+        return new Log4jLogger(validateContext(context).getLogger(key), name);
     }
 
     @Override
@@ -42,5 +44,10 @@ public class Log4jLoggerFactory extends AbstractLoggerAdapter<Logger> implements
         final Class<?> anchor = StackLocatorUtil.getCallerClass(FQCN, PACKAGE);
         return anchor == null ? LogManager.getContext() : getContext(StackLocatorUtil.getCallerClass(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");
+        }
+        return context;
+    }
 }
diff --git a/log4j-slf4j-impl/src/test/java/org/apache/logging/slf4j/OverflowTest.java b/log4j-slf4j-impl/src/test/java/org/apache/logging/slf4j/OverflowTest.java
new file mode 100644
index 0000000..bcb9d0b
--- /dev/null
+++ b/log4j-slf4j-impl/src/test/java/org/apache/logging/slf4j/OverflowTest.java
@@ -0,0 +1,43 @@
+/*
+ * 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.slf4j;
+
+import org.apache.logging.log4j.LoggingException;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.junit.Assert.fail;
+
+/**
+ * Tests StackOverflow when slf4j-impl and to-slf4j are both present.
+ */
+public class OverflowTest {
+
+    @Test
+    public void log() {
+        try {
+            final Logger logger = LoggerFactory.getLogger(OverflowTest.class);
+            fail("Failed to detect inclusion of log4j-to-slf4j");
+        } catch (LoggingException ex) {
+            // Expected exception.
+        } catch (StackOverflowError error) {
+            fail("Failed to detect inclusion of log4j-to-slf4j, caught StackOverflowError");
+        }
+    }
+
+}
diff --git a/log4j-slf4j18-impl/pom.xml b/log4j-slf4j18-impl/pom.xml
index fe22019..f6a08c8 100644
--- a/log4j-slf4j18-impl/pom.xml
+++ b/log4j-slf4j18-impl/pom.xml
@@ -78,6 +78,12 @@
       <scope>test</scope>
     </dependency>
     <dependency>
+      <groupId>org.apache.logging.log4j</groupId>
+      <artifactId>log4j-to-slf4j</artifactId>
+      <scope>test</scope>
+      <version>${project.version}</version>
+    </dependency>
+    <dependency>
       <groupId>junit</groupId>
       <artifactId>junit</artifactId>
       <scope>test</scope>
@@ -101,6 +107,42 @@
         </executions>
       </plugin>
       <plugin>
+        <groupId>org.apache.maven.plugins</groupId>
+        <artifactId>maven-surefire-plugin</artifactId>
+        <executions>
+          <execution>
+            <id>loop-test</id>
+            <phase>test</phase>
+            <goals>
+              <goal>test</goal>
+            </goals>
+            <configuration>
+              <includes>
+                <include>**/OverflowTest.java</include>
+              </includes>
+            </configuration>
+          </execution>
+          <execution>
+            <id>default-test</id>
+            <phase>test</phase>
+            <goals>
+              <goal>test</goal>
+            </goals>
+            <configuration>
+              <includes>
+                <include>**/*Test.java</include>
+              </includes>
+              <excludes>
+                <exclude>**/OverflowTest.java</exclude>
+              </excludes>
+              <classpathDependencyExcludes>
+                <classpathDependencyExcludes>org.apache.logging.log4j:log4j-to-slf4j</classpathDependencyExcludes>
+              </classpathDependencyExcludes>
+            </configuration>
+          </execution>
+        </executions>
+      </plugin>
+      <plugin>
         <groupId>org.apache.felix</groupId>
         <artifactId>maven-bundle-plugin</artifactId>
         <configuration>
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 e7865a8..d0d5ae6 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
@@ -17,6 +17,7 @@
 package org.apache.logging.slf4j;
 
 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.util.StackLocatorUtil;
@@ -31,6 +32,7 @@ public class Log4jLoggerFactory extends AbstractLoggerAdapter<Logger> implements
     private static final String FQCN = Log4jLoggerFactory.class.getName();
     private static final String PACKAGE = "org.slf4j";
     private final Log4jMarkerFactory markerFactory;
+    private static final String TO_SLF4J_CONTEXT = "org.apache.logging.slf4j.SLF4JLoggerContext";
 
     public Log4jLoggerFactory(final Log4jMarkerFactory markerFactory) {
         this.markerFactory = markerFactory;
@@ -40,7 +42,7 @@ public class Log4jLoggerFactory extends AbstractLoggerAdapter<Logger> implements
     @Override
     protected Logger newLogger(final String name, final LoggerContext context) {
         final String key = Logger.ROOT_LOGGER_NAME.equals(name) ? LogManager.ROOT_LOGGER_NAME : name;
-        return new Log4jLogger(markerFactory, context.getLogger(key), name);
+        return new Log4jLogger(markerFactory, validateContext(context).getLogger(key), name);
     }
 
     @Override
@@ -49,10 +51,14 @@ public class Log4jLoggerFactory extends AbstractLoggerAdapter<Logger> implements
         return anchor == null ? LogManager.getContext() : getContext(StackLocatorUtil.getCallerClass(anchor));
     }
 
-
     Log4jMarkerFactory getMarkerFactory() {
         return markerFactory;
     }
 
-
+    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");
+        }
+        return context;
+    }
 }
diff --git a/log4j-slf4j18-impl/src/test/java/org/apache/logging/slf4j/OverflowTest.java b/log4j-slf4j18-impl/src/test/java/org/apache/logging/slf4j/OverflowTest.java
new file mode 100644
index 0000000..bcb9d0b
--- /dev/null
+++ b/log4j-slf4j18-impl/src/test/java/org/apache/logging/slf4j/OverflowTest.java
@@ -0,0 +1,43 @@
+/*
+ * 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.slf4j;
+
+import org.apache.logging.log4j.LoggingException;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.junit.Assert.fail;
+
+/**
+ * Tests StackOverflow when slf4j-impl and to-slf4j are both present.
+ */
+public class OverflowTest {
+
+    @Test
+    public void log() {
+        try {
+            final Logger logger = LoggerFactory.getLogger(OverflowTest.class);
+            fail("Failed to detect inclusion of log4j-to-slf4j");
+        } catch (LoggingException ex) {
+            // Expected exception.
+        } catch (StackOverflowError error) {
+            fail("Failed to detect inclusion of log4j-to-slf4j, caught StackOverflowError");
+        }
+    }
+
+}
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 77bf934..e65abda 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -30,6 +30,9 @@
          - "remove" - Removed
     -->
     <release version="2.12.0" date="2019-MM-DD" description="GA Release 2.12.0">
+      <action issue="LOG4J2-2597" dev="rgoers" type="fix">
+        Throw better exception message when both log4j-slf4j-impl and log4j-to-slf4j are present.
+      </action>
       <action issue="LOG4J2-913" dev="rgoers" type="add">
         Add support for reconfiguration via HTTP(S), Docker, and Spring Cloud Configuration.
       </action>