You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by gg...@apache.org on 2022/01/04 22:13:41 UTC

[logging-log4j2] branch release-2.x updated: [LOG4J2-3316] Log4j 1.2 bridge should ignore case in properties file keys.

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

ggregory 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 a2d5200  [LOG4J2-3316] Log4j 1.2 bridge should ignore case in properties file keys.
a2d5200 is described below

commit a2d5200cbc0b79ffda57f455ee26c272d2aee17c
Author: Gary Gregory <ga...@gmail.com>
AuthorDate: Tue Jan 4 17:13:38 2022 -0500

    [LOG4J2-3316] Log4j 1.2 bridge should ignore case in properties file
    keys.
---
 log4j-1.2-api/pom.xml                              |  4 ++
 .../org/apache/log4j/builders/AbstractBuilder.java | 45 +++++++++++-------
 .../log4j/config/PropertiesConfiguration.java      |  3 +-
 .../log4j/config/PropertiesConfigurationTest.java  | 53 +++++++++++++++-------
 .../log4j-FileAppender-with-props.properties       | 20 ++++++++
 5 files changed, 90 insertions(+), 35 deletions(-)

diff --git a/log4j-1.2-api/pom.xml b/log4j-1.2-api/pom.xml
index e61f759..3228734 100644
--- a/log4j-1.2-api/pom.xml
+++ b/log4j-1.2-api/pom.xml
@@ -43,6 +43,10 @@
       <groupId>org.junit.jupiter</groupId>
       <artifactId>junit-jupiter-engine</artifactId>
     </dependency>
+    <dependency>
+      <groupId>org.apache.commons</groupId>
+      <artifactId>commons-lang3</artifactId>
+    </dependency>
     <!-- Place Felix before Equinox because Felix is signed. -->
     <dependency>
       <groupId>org.apache.felix</groupId>
diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/AbstractBuilder.java b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/AbstractBuilder.java
index 9cd7851..735d556 100644
--- a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/AbstractBuilder.java
+++ b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/AbstractBuilder.java
@@ -16,6 +16,13 @@
  */
 package org.apache.log4j.builders;
 
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Properties;
+
 import org.apache.log4j.bridge.FilterAdapter;
 import org.apache.log4j.bridge.FilterWrapper;
 import org.apache.log4j.helpers.OptionConverter;
@@ -28,12 +35,6 @@ import org.apache.logging.log4j.core.lookup.ConfigurationStrSubstitutor;
 import org.apache.logging.log4j.core.lookup.StrSubstitutor;
 import org.apache.logging.log4j.status.StatusLogger;
 
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Properties;
-
 /**
  * Base class for Log4j 1 component builders.
  */
@@ -49,39 +50,46 @@ public abstract class AbstractBuilder {
     protected static final String RELATIVE = "RELATIVE";
 
     private final String prefix;
-    private final Properties props;
+    private final Properties properties;
     private final StrSubstitutor strSubstitutor;
 
     public AbstractBuilder() {
         this.prefix = null;
-        this.props = new Properties();
-        strSubstitutor = new ConfigurationStrSubstitutor(System.getProperties());
+        this.properties = new Properties();
+        this.strSubstitutor = new ConfigurationStrSubstitutor(System.getProperties());
     }
 
     public AbstractBuilder(String prefix, Properties props) {
         this.prefix = prefix + ".";
-        this.props = props;
+        this.properties = (Properties) props.clone();
         Map<String, String> map = new HashMap<>();
         System.getProperties().forEach((k, v) -> map.put(k.toString(), v.toString()));
         props.forEach((k, v) -> map.put(k.toString(), v.toString()));
-        strSubstitutor = new ConfigurationStrSubstitutor(map);
+        // normalize keys to lower case for case-insensitive access.
+        props.forEach((k, v) -> map.put(toLowerCase(k.toString()), v.toString()));
+        props.entrySet().forEach(e -> this.properties.put(toLowerCase(e.getKey().toString()), e.getValue()));
+        this.strSubstitutor = new ConfigurationStrSubstitutor(map);
     }
 
     public String getProperty(String key) {
-        return strSubstitutor.replace(props.getProperty(prefix + key));
+        return getProperty(key, null);
     }
 
     public String getProperty(String key, String defaultValue) {
-        return strSubstitutor.replace(props.getProperty(prefix + key, defaultValue));
+        String fullKey = prefix + key;
+        String value = properties.getProperty(fullKey);
+        value = value != null ? value : properties.getProperty(toLowerCase(fullKey), defaultValue);
+        return strSubstitutor.replace(value);
     }
 
     public boolean getBooleanProperty(String key) {
-        return Boolean.parseBoolean(strSubstitutor.replace(props.getProperty(prefix + key, Boolean.FALSE.toString())));
+        return Boolean.parseBoolean(getProperty(key, Boolean.FALSE.toString()));
     }
 
     public int getIntegerProperty(String key, int defaultValue) {
-        String value = getProperty(key);
+        String value = null;
         try {
+            value = getProperty(key);
             if (value != null) {
                 return Integer.parseInt(value);
             }
@@ -92,7 +100,7 @@ public abstract class AbstractBuilder {
     }
 
     public Properties getProperties() {
-        return props;
+        return properties;
     }
 
 
@@ -126,4 +134,9 @@ public abstract class AbstractBuilder {
         }
         return null;
     }
+
+    String toLowerCase(final String value) {
+        return value == null ? null : value.toLowerCase(Locale.ROOT);
+    }
+
 }
diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/config/PropertiesConfiguration.java b/log4j-1.2-api/src/main/java/org/apache/log4j/config/PropertiesConfiguration.java
index 1ca70d4..696caa5 100644
--- a/log4j-1.2-api/src/main/java/org/apache/log4j/config/PropertiesConfiguration.java
+++ b/log4j-1.2-api/src/main/java/org/apache/log4j/config/PropertiesConfiguration.java
@@ -65,7 +65,7 @@ public class PropertiesConfiguration  extends Log4j1Configuration {
 
     private static final String INTERNAL_ROOT_NAME = "root";
 
-    private final Map<String, Appender> registry;
+    private final Map<String, Appender> registry = new HashMap<>();
 
     /**
      * Constructor.
@@ -76,7 +76,6 @@ public class PropertiesConfiguration  extends Log4j1Configuration {
     public PropertiesConfiguration(final LoggerContext loggerContext, final ConfigurationSource source,
             int monitorIntervalSeconds) {
         super(loggerContext, source, monitorIntervalSeconds);
-        registry = new HashMap<>();
     }
 
     @Override
diff --git a/log4j-1.2-api/src/test/java/org/apache/log4j/config/PropertiesConfigurationTest.java b/log4j-1.2-api/src/test/java/org/apache/log4j/config/PropertiesConfigurationTest.java
index 59757cf..5ae781a 100644
--- a/log4j-1.2-api/src/test/java/org/apache/log4j/config/PropertiesConfigurationTest.java
+++ b/log4j-1.2-api/src/test/java/org/apache/log4j/config/PropertiesConfigurationTest.java
@@ -16,6 +16,7 @@
  */
 package org.apache.log4j.config;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
@@ -23,6 +24,7 @@ import java.io.File;
 import java.util.List;
 import java.util.Map;
 
+import org.apache.commons.lang3.SystemUtils;
 import org.apache.log4j.ListAppender;
 import org.apache.log4j.LogManager;
 import org.apache.log4j.Logger;
@@ -31,6 +33,7 @@ import org.apache.log4j.bridge.FilterAdapter;
 import org.apache.log4j.spi.LoggingEvent;
 import org.apache.logging.log4j.core.Appender;
 import org.apache.logging.log4j.core.LoggerContext;
+import org.apache.logging.log4j.core.appender.FileAppender;
 import org.apache.logging.log4j.core.config.Configuration;
 import org.apache.logging.log4j.core.filter.Filterable;
 import org.junit.Test;
@@ -44,9 +47,9 @@ public class PropertiesConfigurationTest {
     public void testConfigureNullPointerException() throws Exception {
         try (LoggerContext loggerContext = TestConfigurator.configure("target/test-classes/LOG4J2-3247.properties")) {
             // [LOG4J2-3247] configure() should not throw an NPE.
-            Configuration configuration = loggerContext.getConfiguration();
+            final Configuration configuration = loggerContext.getConfiguration();
             assertNotNull(configuration);
-            Appender appender = configuration.getAppender("CONSOLE");
+            final Appender appender = configuration.getAppender("CONSOLE");
             assertNotNull(appender);
         }
     }
@@ -55,12 +58,12 @@ public class PropertiesConfigurationTest {
     public void testConsoleAppenderFilter() throws Exception {
         try (LoggerContext loggerContext = TestConfigurator.configure("target/test-classes/LOG4J2-3247.properties")) {
             // LOG4J2-3281 PropertiesConfiguration.buildAppender not adding filters to appender
-            Configuration configuration = loggerContext.getConfiguration();
+            final Configuration configuration = loggerContext.getConfiguration();
             assertNotNull(configuration);
-            Appender appender = configuration.getAppender("CONSOLE");
+            final Appender appender = configuration.getAppender("CONSOLE");
             assertNotNull(appender);
-            Filterable filterable = (Filterable) appender;
-            FilterAdapter filter = (FilterAdapter) filterable.getFilter();
+            final Filterable filterable = (Filterable) appender;
+            final FilterAdapter filter = (FilterAdapter) filterable.getFilter();
             assertNotNull(filter);
             assertTrue(filter.getFilter() instanceof NeutralFilterFixture);
         }
@@ -70,12 +73,12 @@ public class PropertiesConfigurationTest {
     public void testCustomAppenderFilter() throws Exception {
         try (LoggerContext loggerContext = TestConfigurator.configure("target/test-classes/LOG4J2-3281.properties")) {
             // LOG4J2-3281 PropertiesConfiguration.buildAppender not adding filters to appender
-            Configuration configuration = loggerContext.getConfiguration();
+            final Configuration configuration = loggerContext.getConfiguration();
             assertNotNull(configuration);
-            Appender appender = configuration.getAppender("CUSTOM");
+            final Appender appender = configuration.getAppender("CUSTOM");
             assertNotNull(appender);
-            Filterable filterable = (Filterable) appender;
-            FilterAdapter filter = (FilterAdapter) filterable.getFilter();
+            final Filterable filterable = (Filterable) appender;
+            final FilterAdapter filter = (FilterAdapter) filterable.getFilter();
             assertNotNull(filter);
             assertTrue(filter.getFilter() instanceof NeutralFilterFixture);
         }
@@ -84,13 +87,13 @@ public class PropertiesConfigurationTest {
     @Test
     public void testListAppender() throws Exception {
         try (LoggerContext loggerContext = TestConfigurator.configure("target/test-classes/log4j1-list.properties")) {
-            Logger logger = LogManager.getLogger("test");
+            final Logger logger = LogManager.getLogger("test");
             logger.debug("This is a test of the root logger");
-            Configuration configuration = loggerContext.getConfiguration();
-            Map<String, Appender> appenders = configuration.getAppenders();
+            final Configuration configuration = loggerContext.getConfiguration();
+            final Map<String, Appender> appenders = configuration.getAppenders();
             ListAppender eventAppender = null;
             ListAppender messageAppender = null;
-            for (Map.Entry<String, Appender> entry : appenders.entrySet()) {
+            for (final Map.Entry<String, Appender> entry : appenders.entrySet()) {
                 if (entry.getKey().equals("list")) {
                     messageAppender = (ListAppender) ((AppenderAdapter.Adapter) entry.getValue()).getAppender();
                 } else if (entry.getKey().equals("events")) {
@@ -99,9 +102,9 @@ public class PropertiesConfigurationTest {
             }
             assertNotNull("No Event Appender", eventAppender);
             assertNotNull("No Message Appender", messageAppender);
-            List<LoggingEvent> events = eventAppender.getEvents();
+            final List<LoggingEvent> events = eventAppender.getEvents();
             assertTrue("No events", events != null && events.size() > 0);
-            List<String> messages = messageAppender.getMessages();
+            final List<String> messages = messageAppender.getMessages();
             assertTrue("No messages", messages != null && messages.size() > 0);
         }
     }
@@ -109,7 +112,7 @@ public class PropertiesConfigurationTest {
     @Test
     public void testProperties() throws Exception {
         try (LoggerContext loggerContext = TestConfigurator.configure("target/test-classes/log4j1-file.properties")) {
-            Logger logger = LogManager.getLogger("test");
+            final Logger logger = LogManager.getLogger("test");
             logger.debug("This is a test of the root logger");
             File file = new File("target/temp.A1");
             assertTrue("File A1 was not created", file.exists());
@@ -120,4 +123,20 @@ public class PropertiesConfigurationTest {
         }
     }
 
+    @Test
+    public void testSystemProperties() throws Exception {
+        try (LoggerContext loggerContext = TestConfigurator.configure("target/test-classes/config-1.2/log4j-FileAppender-with-props.properties")) {
+            // [LOG4J2-3312] Bridge does not convert properties.
+            final Configuration configuration = loggerContext.getConfiguration();
+            assertNotNull(configuration);
+            final String name = "FILE_APPENDER";
+            final Appender appender = configuration.getAppender(name);
+            assertNotNull(name, appender);
+            assertTrue(appender instanceof FileAppender);
+            final FileAppender fileAppender = (FileAppender) appender;
+            // Two slashes because that's how the config file is setup.
+            assertEquals(SystemUtils.getJavaIoTmpDir() + "//hadoop.log", fileAppender.getFileName());
+        }
+    }
+
 }
\ No newline at end of file
diff --git a/log4j-1.2-api/src/test/resources/config-1.2/log4j-FileAppender-with-props.properties b/log4j-1.2-api/src/test/resources/config-1.2/log4j-FileAppender-with-props.properties
new file mode 100644
index 0000000..b98c194
--- /dev/null
+++ b/log4j-1.2-api/src/test/resources/config-1.2/log4j-FileAppender-with-props.properties
@@ -0,0 +1,20 @@
+###############################################################################
+#
+# Log4J 1.2 Configuration.
+#
+
+hadoop.log.file=hadoop.log
+
+log4j.rootLogger=TRACE, FILE_APPENDER
+
+#
+# Rolling File Appender
+#
+hadoop.log.maxfilesize=256MB
+hadoop.log.maxbackupindex=20
+log4j.appender.FILE_APPENDER=org.apache.log4j.FileAppender
+log4j.appender.FILE_APPENDER.file=${java.io.tmpdir}/${hadoop.log.file}
+log4j.appender.FILE_APPENDER.layout=org.apache.log4j.PatternLayout
+
+# Pattern format: Date LogLevel LoggerName LogMessage
+log4j.appender.FILE_APPENDER.layout.ConversionPattern=%d{ISO8601} %p %c: %m%n