Fix JNDI failures

apache · May 22, 2024 · 940da61 · 940da61
1 parent cd83a8c
commit 940da61
Show file tree

Hide file tree

Showing 5 changed files with 40 additions and 19 deletions.
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/Interpolator.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/Interpolator.java
@@ -136,7 +136,9 @@ public String lookup(final LogEvent event, String var) {
                 if (lookup instanceof LoggerContextAware) {
                     ((LoggerContextAware) lookup).setLoggerContext(loggerContext.get());
                 }
-                value = event == null ? lookup.lookup(name) : lookup.lookup(event, name);
+                if (lookup != null) {
+                    value = event == null ? lookup.lookup(name) : lookup.lookup(event, name);
+                }
             }
 
             if (value != null) {

diff --git a/...-jndi-test/src/test/java/org/apache/logging/log4j/jndi/lookup/JndiDisabledLookupTest.java b/...-jndi-test/src/test/java/org/apache/logging/log4j/jndi/lookup/JndiDisabledLookupTest.java
@@ -16,19 +16,30 @@
  */
 package org.apache.logging.log4j.jndi.lookup;
 
-import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.assertj.core.api.Assertions.assertThat;
 
-import org.junit.Test;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.status.StatusLogger;
+import org.junit.jupiter.api.Test;
+import org.junitpioneer.jupiter.SetSystemProperty;
 
 /**
  * JndiDisabledLookupTest
  *
  * Verifies the Lookups are disabled without the log4j2.enableJndiLookup property set to true.
  */
-public class JndiDisabledLookupTest {
+@SetSystemProperty(key = "log4j2.status.entries", value = "10")
+@SetSystemProperty(key = "log4j2.StatusLogger.level", value = "WARN")
+class JndiDisabledLookupTest {
 
     @Test
-    public void testLookup() {
-        assertThrows(IllegalStateException.class, JndiLookup::new);
+    void testLookup() {
+        assertThat(JndiLookup.createLookup()).isNull();
+        assertThat(StatusLogger.getLogger().getStatusData()).anySatisfy(data -> {
+            assertThat(data.getLevel()).isEqualTo(Level.ERROR);
+            assertThat(data.getMessage().getFormattedMessage())
+                    .isEqualTo(
+                            "Ignoring request to use JNDI lookup. JNDI must be enabled by setting log4j.enableJndiLookup=true");
+        });
     }
 }
diff --git a/log4j-jndi-test/src/test/java/org/apache/logging/log4j/jndi/lookup/JndiLookupTest.java b/log4j-jndi-test/src/test/java/org/apache/logging/log4j/jndi/lookup/JndiLookupTest.java
@@ -60,7 +60,7 @@ private Map<String, Object> createBindings() {
 
     @Test
     public void testLookup() {
-        final StrLookup lookup = new JndiLookup();
+        final StrLookup lookup = JndiLookup.createLookup();
 
         String contextName = lookup.lookup(TEST_CONTEXT_RESOURCE_NAME);
         assertEquals(TEST_CONTEXT_NAME, contextName);
@@ -75,7 +75,7 @@ public void testLookup() {
     @Test
     public void testNonStringLookup() throws Exception {
         // LOG4J2-1310
-        final StrLookup lookup = new JndiLookup();
+        final StrLookup lookup = JndiLookup.createLookup();
         final String integralValue = lookup.lookup(TEST_INTEGRAL_NAME);
         assertEquals(String.valueOf(TEST_INTEGRAL_VALUE), integralValue);
         final String collectionValue = lookup.lookup(TEST_STRINGS_NAME);

diff --git a/...ndi-test/src/test/java/org/apache/logging/log4j/jndi/lookup/JndiRestrictedLookupTest.java b/...ndi-test/src/test/java/org/apache/logging/log4j/jndi/lookup/JndiRestrictedLookupTest.java
@@ -64,7 +64,7 @@ public void testBadUriLookup() throws Exception {
         final int port = embeddedLdapRule.embeddedServerPort();
         final Context context = embeddedLdapRule.context();
         context.bind("cn=" + RESOURCE + "," + DOMAIN_DSN, new Fruit("Test Message"));
-        final StrLookup lookup = new JndiLookup();
+        final StrLookup lookup = JndiLookup.createLookup();
         final String result = lookup.lookup(
                 LDAP_URL + port + "/" + "cn=" + RESOURCE + "," + DOMAIN_DSN + "?Type=A Type&Name=1100110&Char=!");
         if (result != null) {
@@ -78,7 +78,7 @@ public void testReferenceLookup() throws Exception {
         final int port = embeddedLdapRule.embeddedServerPort();
         final Context context = embeddedLdapRule.context();
         context.bind("cn=" + RESOURCE + "," + DOMAIN_DSN, new Fruit("Test Message"));
-        final StrLookup lookup = new JndiLookup();
+        final StrLookup lookup = JndiLookup.createLookup();
         final String result = lookup.lookup(LDAP_URL + port + "/" + "cn=" + RESOURCE + "," + DOMAIN_DSN);
         if (result != null) {
             fail("Lookup returned an object");
@@ -91,7 +91,7 @@ public void testSerializableLookup() throws Exception {
         final int port = embeddedLdapRule.embeddedServerPort();
         final Context context = embeddedLdapRule.context();
         context.bind("cn=" + TEST_STRING + "," + DOMAIN_DSN, "Test Message");
-        final StrLookup lookup = new JndiLookup();
+        final StrLookup lookup = JndiLookup.createLookup();
         final String result = lookup.lookup(LDAP_URL + port + "/" + "cn=" + TEST_STRING + "," + DOMAIN_DSN);
         if (result != null) {
             fail("LDAP is enabled");
@@ -104,7 +104,7 @@ public void testBadSerializableLookup() throws Exception {
         final int port = embeddedLdapRule.embeddedServerPort();
         final Context context = embeddedLdapRule.context();
         context.bind("cn=" + TEST_MESSAGE + "," + DOMAIN_DSN, new SerializableMessage("Test Message"));
-        final StrLookup lookup = new JndiLookup();
+        final StrLookup lookup = JndiLookup.createLookup();
         final String result = lookup.lookup(LDAP_URL + port + "/" + "cn=" + TEST_MESSAGE + "," + DOMAIN_DSN);
         if (result != null) {
             fail("Lookup returned an object");
@@ -113,7 +113,7 @@ public void testBadSerializableLookup() throws Exception {
 
     @Test
     public void testDnsLookup() throws Exception {
-        final StrLookup lookup = new JndiLookup();
+        final StrLookup lookup = JndiLookup.createLookup();
         final String result = lookup.lookup("dns:/" + DOMAIN);
         if (result != null) {
             fail("No DNS data returned");

diff --git a/log4j-jndi/src/main/java/org/apache/logging/log4j/jndi/lookup/JndiLookup.java b/log4j-jndi/src/main/java/org/apache/logging/log4j/jndi/lookup/JndiLookup.java
@@ -24,7 +24,9 @@
 import org.apache.logging.log4j.core.LogEvent;
 import org.apache.logging.log4j.core.lookup.AbstractLookup;
 import org.apache.logging.log4j.core.lookup.Lookup;
+import org.apache.logging.log4j.core.lookup.StrLookup;
 import org.apache.logging.log4j.jndi.JndiManager;
+import org.apache.logging.log4j.plugins.Factory;
 import org.apache.logging.log4j.plugins.Plugin;
 import org.apache.logging.log4j.status.StatusLogger;
 
@@ -33,22 +35,28 @@
  */
 @Lookup
 @Plugin("jndi")
-public class JndiLookup extends AbstractLookup {
+public final class JndiLookup extends AbstractLookup {
 
     private static final Logger LOGGER = StatusLogger.getLogger();
     private static final Marker LOOKUP = MarkerManager.getMarker("LOOKUP");
 
     /** JNDI resource path prefix used in a J2EE container */
     static final String CONTAINER_JNDI_RESOURCE_PATH_PREFIX = "java:comp/env/";
 
-    private final boolean disabled;
+    @Factory
+    public static StrLookup createLookup() {
+        if (JndiManager.isJndiLookupEnabled()) {
+            return new JndiLookup();
+        }
+        LOGGER.error(
+                "Ignoring request to use JNDI lookup. JNDI must be enabled by setting log4j.enableJndiLookup=true");
+        return null;
+    }
 
     /**
      * Constructs a new instance.
      */
-    public JndiLookup() {
-        this.disabled = !JndiManager.isJndiLookupEnabled();
-    }
+    private JndiLookup() {}
 
     /**
      * Looks up the value of the JNDI resource.
@@ -59,7 +67,7 @@ public JndiLookup() {
      */
     @Override
     public String lookup(final LogEvent event, final String key) {
-        if (disabled || key == null) {
+        if (key == null) {
             return null;
         }
         final String jndiName = convertJndiName(key);