From 05db5f9527254632b59aed2a1d78a32c5ab74f16 Mon Sep 17 00:00:00 2001
From: Gary Gregory <garydgregory@gmail.com>
Date: Mon, 27 Dec 2021 12:42:26 -0500
Subject: [PATCH] Refactor to reuse existing code.

---
 .../db/jdbc/DataSourceConnectionSource.java   | 20 +++++-----
 .../logging/log4j/core/net/JndiManager.java   |  9 ++++-
 .../AbstractJdbcAppenderDataSourceTest.java   | 13 ++++---
 .../db/jdbc/AbstractJdbcDataSourceTest.java   | 39 +++++++++++++++++++
 .../jdbc/DataSourceConnectionSourceTest.java  |  2 +-
 .../JdbcAppenderMapMessageDataSourceTest.java |  2 +-
 .../core/appender/mom/JmsAppenderTest.java    |  3 +-
 .../log4j/core/net/JndiManagerTest.java       |  9 ++++-
 src/site/xdoc/manual/appenders.xml            | 10 +++--
 src/site/xdoc/manual/configuration.xml.vm     | 12 +++++-
 10 files changed, 93 insertions(+), 26 deletions(-)
 create mode 100644 log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/AbstractJdbcDataSourceTest.java

diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/DataSourceConnectionSource.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/DataSourceConnectionSource.java
index a791df9897..6a55a84d3c 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/DataSourceConnectionSource.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/DataSourceConnectionSource.java
@@ -18,8 +18,8 @@
 
 import java.sql.Connection;
 import java.sql.SQLException;
+import java.util.Objects;
 
-import javax.naming.InitialContext;
 import javax.naming.NamingException;
 import javax.sql.DataSource;
 
@@ -28,6 +28,7 @@
 import org.apache.logging.log4j.core.config.plugins.Plugin;
 import org.apache.logging.log4j.core.config.plugins.PluginAttribute;
 import org.apache.logging.log4j.core.config.plugins.PluginFactory;
+import org.apache.logging.log4j.core.net.JndiManager;
 import org.apache.logging.log4j.status.StatusLogger;
 import org.apache.logging.log4j.util.Strings;
 
@@ -42,7 +43,7 @@
     private final String description;
 
     private DataSourceConnectionSource(final String dataSourceName, final DataSource dataSource) {
-        this.dataSource = dataSource;
+        this.dataSource = Objects.requireNonNull(dataSource, "dataSource");
         this.description = "dataSource{ name=" + dataSourceName + ", value=" + dataSource + " }";
     }
 
@@ -59,25 +60,26 @@ public String toString() {
     /**
      * Factory method for creating a connection source within the plugin manager.
      *
-     * @param jndiName The full JNDI path where the data source is bound. Should start with java:/comp/env or
-     *                 environment-equivalent.
+     * @param jndiName The full JNDI path where the data source is bound. Must start with java:/comp/env or environment-equivalent.
      * @return the created connection source.
      */
     @PluginFactory
     public static DataSourceConnectionSource createConnectionSource(@PluginAttribute("jndiName") final String jndiName) {
+        if (!JndiManager.isJndiJdbcEnabled()) {
+            LOGGER.error("JNDI must be enabled by setting log4j2.enableJndiJdbc=true");
+            return null;
+        }
         if (Strings.isEmpty(jndiName)) {
             LOGGER.error("No JNDI name provided.");
             return null;
         }
-
         try {
-            final InitialContext context = new InitialContext();
-            final DataSource dataSource = (DataSource) context.lookup(jndiName);
+            @SuppressWarnings("resource")
+            final DataSource dataSource = JndiManager.getDefaultManager(DataSourceConnectionSource.class.getCanonicalName()).lookup(jndiName);
             if (dataSource == null) {
-                LOGGER.error("No data source found with JNDI name [" + jndiName + "].");
+                LOGGER.error("No DataSource found with JNDI name [" + jndiName + "].");
                 return null;
             }
-
             return new DataSourceConnectionSource(jndiName, dataSource);
         } catch (final NamingException e) {
             LOGGER.error(e.getMessage(), e);
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/net/JndiManager.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/net/JndiManager.java
index d73d4137b6..5e807b28c9 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/net/JndiManager.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/net/JndiManager.java
@@ -43,6 +43,7 @@
     private static final String JAVA_SCHEME = "java";
 
     private static final boolean JNDI_CONTEXT_SELECTOR_ENABLED = isJndiEnabled("ContextSelector");
+    private static final boolean JNDI_JDBC_ENABLED = isJndiEnabled("Jdbc");
     private static final boolean JNDI_JMS_ENABLED = isJndiEnabled("Jms");
     private static final boolean JNDI_LOOKUP_ENABLED = isJndiEnabled("Lookup");
 
@@ -53,13 +54,17 @@ private static boolean isJndiEnabled(final String subKey) {
     }
 
     public static boolean isJndiEnabled() {
-        return isJndiContextSelectorEnabled() || isJndiJmsEnabled() || isJndiLookupEnabled();
+        return isJndiContextSelectorEnabled() || isJndiJdbcEnabled() || isJndiJmsEnabled() || isJndiLookupEnabled();
     }
 
     public static boolean isJndiContextSelectorEnabled() {
         return JNDI_CONTEXT_SELECTOR_ENABLED;
     }
 
+    public static boolean isJndiJdbcEnabled() {
+        return JNDI_JDBC_ENABLED;
+    }
+
     public static boolean isJndiJmsEnabled() {
         return JNDI_JMS_ENABLED;
     }
@@ -208,7 +213,7 @@ protected boolean releaseSub(final long timeout, final TimeUnit timeUnit) {
             }
             LOGGER.warn("Unsupported JNDI URI - {}", name);
         } catch (URISyntaxException ex) {
-            LOGGER.warn("Invalid  JNDI URI - {}", name);
+            LOGGER.warn("Invalid JNDI URI - {}", name);
         }
         return null;
     }
diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/AbstractJdbcAppenderDataSourceTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/AbstractJdbcAppenderDataSourceTest.java
index c4a73121b3..adf9e90155 100644
--- a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/AbstractJdbcAppenderDataSourceTest.java
+++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/AbstractJdbcAppenderDataSourceTest.java
@@ -16,12 +16,19 @@
  */
 package org.apache.logging.log4j.core.appender.db.jdbc;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.BDDMockito.given;
+import static org.mockito.Mockito.mock;
+
 import java.io.ByteArrayOutputStream;
 import java.io.PrintWriter;
 import java.sql.Connection;
 import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.sql.Statement;
+
 import javax.sql.DataSource;
 
 import org.apache.logging.log4j.LogManager;
@@ -35,14 +42,10 @@
 import org.junit.Test;
 import org.junit.rules.RuleChain;
 
-import static org.junit.Assert.*;
-import static org.mockito.BDDMockito.given;
-import static org.mockito.Mockito.mock;
-
 /**
  * Abstract unit test for JdbcAppender using a {@link DataSource} configuration.
  */
-public abstract class AbstractJdbcAppenderDataSourceTest {
+public abstract class AbstractJdbcAppenderDataSourceTest extends AbstractJdbcDataSourceTest {
 
     @Rule
     public final RuleChain rules;
diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/AbstractJdbcDataSourceTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/AbstractJdbcDataSourceTest.java
new file mode 100644
index 0000000000..78bebee1a1
--- /dev/null
+++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/AbstractJdbcDataSourceTest.java
@@ -0,0 +1,39 @@
+/*
+ * 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.log4j.core.appender.db.jdbc;
+
+import javax.sql.DataSource;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+
+/**
+ * Abstract unit test for JDBC using a {@link DataSource} configuration.
+ */
+public abstract class AbstractJdbcDataSourceTest {
+
+    @AfterClass
+    public static void afterClass() throws Exception {
+        System.clearProperty("log4j2.enableJndiJdbc");
+    }
+
+    @BeforeClass
+    public static void beforeClass() throws Exception {
+        System.setProperty("log4j2.enableJndiJdbc", "true");
+    }
+
+}
diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/DataSourceConnectionSourceTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/DataSourceConnectionSourceTest.java
index a49404a83f..ea9183a23d 100644
--- a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/DataSourceConnectionSourceTest.java
+++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/DataSourceConnectionSourceTest.java
@@ -37,7 +37,7 @@
 import org.junit.runners.Parameterized;
 
 @RunWith(Parameterized.class)
-public class DataSourceConnectionSourceTest {
+public class DataSourceConnectionSourceTest extends AbstractJdbcDataSourceTest {
 
     @Parameterized.Parameters(name = "{0}")
     public static Object[][] data() {
diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcAppenderMapMessageDataSourceTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcAppenderMapMessageDataSourceTest.java
index bee9786da7..77dcf31a41 100644
--- a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcAppenderMapMessageDataSourceTest.java
+++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcAppenderMapMessageDataSourceTest.java
@@ -46,7 +46,7 @@
 /**
  * Unit tests {@link MapMessage}s for JdbcAppender using a {@link DataSource} configuration.
  */
-public class JdbcAppenderMapMessageDataSourceTest {
+public class JdbcAppenderMapMessageDataSourceTest extends AbstractJdbcDataSourceTest {
 
     @Rule
     public final RuleChain rules;
diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/mom/JmsAppenderTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/mom/JmsAppenderTest.java
index 4c5b1a9eb7..253cb06a12 100644
--- a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/mom/JmsAppenderTest.java
+++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/mom/JmsAppenderTest.java
@@ -48,6 +48,7 @@
 import org.apache.logging.log4j.message.Message;
 import org.apache.logging.log4j.message.SimpleMessage;
 import org.apache.logging.log4j.message.StringMapMessage;
+import org.junit.AfterClass;
 import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Rule;
@@ -84,7 +85,7 @@
     @Rule
     public RuleChain rules = RuleChain.outerRule(jndiRule).around(ctx);
 
-    @BeforeClass
+    @AfterClass
     public static void afterClass() throws Exception {
         System.clearProperty("log4j2.enableJndiJms");
     }
diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/net/JndiManagerTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/net/JndiManagerTest.java
index e421734b4a..0b3501cf15 100644
--- a/log4j-core/src/test/java/org/apache/logging/log4j/core/net/JndiManagerTest.java
+++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/net/JndiManagerTest.java
@@ -29,14 +29,19 @@
  */
 public class JndiManagerTest {
 
+    @Test
+    public void testIsJndiContextSelectorEnabled() {
+        assertFalse(JndiManager.isJndiContextSelectorEnabled());
+    }
+
     @Test
     public void testIsJndiEnabled() {
         assertFalse(JndiManager.isJndiEnabled());
     }
 
     @Test
-    public void testIsJndiContextSelectorEnabled() {
-        assertFalse(JndiManager.isJndiContextSelectorEnabled());
+    public void testIsJndiJdbcEnabled() {
+        assertFalse(JndiManager.isJndiJdbcEnabled());
     }
 
     @Test
diff --git a/src/site/xdoc/manual/appenders.xml b/src/site/xdoc/manual/appenders.xml
index c0f8c6dbd5..42fb1b8c71 100644
--- a/src/site/xdoc/manual/appenders.xml
+++ b/src/site/xdoc/manual/appenders.xml
@@ -995,9 +995,13 @@ CREATE TABLE logs (
         </subsection>
         <a name="JDBCAppender"/>
         <subsection name="JDBCAppender">
-          <p>The JDBCAppender writes log events to a relational database table using standard JDBC. It can be configured
-            to obtain JDBC connections using a JNDI <code>DataSource</code> or a custom factory method. Whichever
-            approach you take, it <strong><em>must</em></strong> be backed by a connection pool. Otherwise, logging
+          <p>The JDBC Appender writes log events to a relational database table using standard JDBC. It can be configured
+            to obtain JDBC connections using a JNDI <code>DataSource</code> or a custom factory method.</p>
+          <p>The JDBC Appender configured with a <code>DataSource</code> requires JNDI support so as of release 2.17.1 
+            this appender will not function unless <code>log4j2.enableJndiJdbc=true</code> is configured as a system property 
+            or environment variable. See the <a href="./configuration.html#enableJndiJdbc">enableJndiJdbc</a> system property.</p>
+          <p>
+            Whichever approach you take, it <strong><em>must</em></strong> be backed by a connection pool. Otherwise, logging
             performance will suffer greatly. If batch statements are supported by the configured JDBC driver and a
             <code>bufferSize</code> is configured to be a positive number, then log events will be batched. Note that as
             of Log4j 2.8, there are two ways to configure log event to column mappings: the original <code>ColumnConfig</code>
diff --git a/src/site/xdoc/manual/configuration.xml.vm b/src/site/xdoc/manual/configuration.xml.vm
index 7ddd97c8bd..247eade524 100644
--- a/src/site/xdoc/manual/configuration.xml.vm
+++ b/src/site/xdoc/manual/configuration.xml.vm
@@ -2156,12 +2156,20 @@ public class AwesomeTest {
       When true, the Log4j context selector that uses the JNDI java protocol is enabled. When false, the default, they are disabled.
     </td>
   </tr>
+  <tr>
+    <td><a name="enableJndiJdbc"/>log4j2.enableJndiJdbc</td>
+    <td>LOG4J_ENABLE_JNDI_JDBC</td>
+    <td>false</td>
+    <td>
+      When true, a Log4j JDBC Appender configured with a <code>DataSource</code> which uses JNDI's java protocol is enabled. When false, the default, they are disabled.
+    </td>
+  </tr>
   <tr>
     <td><a name="enableJndiJms"/>log4j2.enableJndiJms</td>
     <td>LOG4J_ENABLE_JNDI_JMS</td>
     <td>false</td>
     <td>
-      When true, the Log4j JMS Appender that uses JNDI's java protocol is enabled. When false, the default, they are disabled.
+      When true, a Log4j JMS Appender that uses JNDI's java protocol is enabled. When false, the default, they are disabled.
     </td>
   </tr>
   <tr>
@@ -2169,7 +2177,7 @@ public class AwesomeTest {
     <td>LOG4J_ENABLE_JNDI_LOOKUP</td>
     <td>false</td>
     <td>
-      When true, the Log4j lookup that uses JNDI's java protocol is enabled. When false, the default, they are disabled.
+      When true, a Log4j lookup that uses JNDI's java protocol is enabled. When false, the default, they are disabled.
     </td>
   </tr>
   <tr>
