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

ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git


The following commit(s) were added to refs/heads/master by this push:
     new 3f3556d  [LOG4J2-3281] PropertiesConfiguration.buildAppender not 
adding filters to custom appender.
3f3556d is described below

commit 3f3556d2294a2395f3f3043d41014846ecc3d524
Author: Gary Gregory <[email protected]>
AuthorDate: Fri Dec 31 09:57:56 2021 -0500

    [LOG4J2-3281] PropertiesConfiguration.buildAppender not adding filters
    to custom appender.
    
    Cherry pick and resolve from release-2.x.
---
 .../org/apache/log4j/bridge/FilterAdapter.java     |  5 ++
 .../org/apache/log4j/builders/BuilderManager.java  | 22 ++++----
 .../log4j/config/PropertiesConfiguration.java      | 37 ++++++++-----
 .../java/org/apache/log4j/CustomNoopAppender.java  | 39 ++++++++++++++
 .../apache/log4j/config/NeutralFilterFixture.java  | 32 ++++++++++++
 .../log4j/config/PropertiesConfigurationTest.java  | 61 ++++++++++++++++++----
 .../src/test/resources/LOG4J2-3247.properties      | 35 +++++++++++++
 .../src/test/resources/LOG4J2-3281.properties      | 25 +++++++++
 8 files changed, 225 insertions(+), 31 deletions(-)

diff --git 
a/log4j-1.2-api/src/main/java/org/apache/log4j/bridge/FilterAdapter.java 
b/log4j-1.2-api/src/main/java/org/apache/log4j/bridge/FilterAdapter.java
index 2dff272..4851125 100644
--- a/log4j-1.2-api/src/main/java/org/apache/log4j/bridge/FilterAdapter.java
+++ b/log4j-1.2-api/src/main/java/org/apache/log4j/bridge/FilterAdapter.java
@@ -32,6 +32,10 @@ public class FilterAdapter extends AbstractFilter {
         this.filter = filter;
     }
 
+    public Filter getFilter() {
+        return filter;
+    }
+
     @Override
     public void start() {
         filter.activateOptions();
@@ -53,4 +57,5 @@ public class FilterAdapter extends AbstractFilter {
         }
         return Result.NEUTRAL;
     }
+
 }
diff --git 
a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/BuilderManager.java 
b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/BuilderManager.java
index ec3a5fb..2c63b7d 100644
--- a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/BuilderManager.java
+++ b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/BuilderManager.java
@@ -36,6 +36,7 @@ import org.w3c.dom.Element;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Properties;
 
 /**
@@ -46,7 +47,7 @@ public class BuilderManager {
     public static final String CATEGORY = "Log4j Builder";
     private static final Logger LOGGER = StatusLogger.getLogger();
     private final Map<String, PluginType<?>> plugins;
-    private static final Class<?>[] constructorParams = new Class[] { 
String.class, Properties.class};
+    private static final Class<?>[] constructorParams = new Class[] { 
String.class, Properties.class };
 
     public BuilderManager() {
         final PluginManager manager = new PluginManager(CATEGORY);
@@ -61,7 +62,7 @@ public class BuilderManager {
                 @SuppressWarnings("unchecked")
                 AppenderBuilder builder = (AppenderBuilder) 
LoaderUtil.newInstanceOf(plugin.getPluginClass());
                 return builder.parseAppender(appenderElement, config);
-            } catch (InstantiationException | IllegalAccessException | 
InvocationTargetException ex) {
+            } catch (ReflectiveOperationException ex) {
                 LOGGER.warn("Unable to load plugin: {} due to: {}", 
plugin.getKey(), ex.getMessage());
             }
         }
@@ -70,6 +71,8 @@ public class BuilderManager {
 
     public Appender parseAppender(String name, String className, String 
prefix, String layoutPrefix,
             String filterPrefix, Properties props, PropertiesConfiguration 
config) {
+        Objects.requireNonNull(plugins, "plugins");
+        Objects.requireNonNull(className, "className");
         PluginType<?> plugin = plugins.get(className.toLowerCase());
         if (plugin != null) {
             AppenderBuilder builder = createBuilder(plugin, prefix, props);
@@ -87,7 +90,7 @@ public class BuilderManager {
                 @SuppressWarnings("unchecked")
                 FilterBuilder builder = (FilterBuilder) 
LoaderUtil.newInstanceOf(plugin.getPluginClass());
                 return builder.parseFilter(filterElement, config);
-            } catch (InstantiationException | IllegalAccessException | 
InvocationTargetException ex) {
+            } catch (ReflectiveOperationException ex) {
                 LOGGER.warn("Unable to load plugin: {} due to: {}", 
plugin.getKey(), ex.getMessage());
             }
         }
@@ -112,7 +115,7 @@ public class BuilderManager {
                 @SuppressWarnings("unchecked")
                 LayoutBuilder builder = (LayoutBuilder) 
LoaderUtil.newInstanceOf(plugin.getPluginClass());
                 return builder.parseLayout(layoutElement, config);
-            } catch (InstantiationException | IllegalAccessException | 
InvocationTargetException ex) {
+            } catch (ReflectiveOperationException ex) {
                 LOGGER.warn("Unable to load plugin: {} due to: {}", 
plugin.getKey(), ex.getMessage());
             }
         }
@@ -136,7 +139,7 @@ public class BuilderManager {
                 @SuppressWarnings("unchecked")
                 RewritePolicyBuilder builder = (RewritePolicyBuilder) 
LoaderUtil.newInstanceOf(plugin.getPluginClass());
                 return builder.parseRewritePolicy(rewriteElement, config);
-            } catch (InstantiationException | IllegalAccessException | 
InvocationTargetException ex) {
+            } catch (ReflectiveOperationException ex) {
                 LOGGER.warn("Unable to load plugin: {} due to: {}", 
plugin.getKey(), ex.getMessage());
             }
         }
@@ -161,12 +164,11 @@ public class BuilderManager {
                 Constructor<T> constructor =
                         (Constructor<T>) 
clazz.getConstructor(constructorParams);
                 return constructor.newInstance(prefix, props);
-            } else {
-                @SuppressWarnings("unchecked")
-                T builder = (T) LoaderUtil.newInstanceOf(clazz);
-                return builder;
             }
-        } catch (NoSuchMethodException | InstantiationException | 
IllegalAccessException | InvocationTargetException ex) {
+            @SuppressWarnings("unchecked")
+            T builder = (T) LoaderUtil.newInstanceOf(clazz);
+            return builder;
+        } catch (ReflectiveOperationException ex) {
             LOGGER.warn("Unable to load plugin: {} due to: {}", 
plugin.getKey(), ex.getMessage());
             return null;
         }
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 5e0a946..876da0c 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
@@ -16,6 +16,20 @@
  */
 package org.apache.log4j.config;
 
+import java.io.IOException;
+import java.io.InputStream;
+import java.lang.reflect.InvocationTargetException;
+import java.util.ArrayList;
+import java.util.Enumeration;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Properties;
+import java.util.SortedMap;
+import java.util.StringTokenizer;
+import java.util.TreeMap;
+
 import org.apache.log4j.Appender;
 import org.apache.log4j.Layout;
 import org.apache.log4j.LogManager;
@@ -80,6 +94,7 @@ public class PropertiesConfiguration  extends 
Log4j1Configuration {
         registry = new HashMap<>();
     }
 
+    @Override
     public void doConfigure() {
         InputStream is = getConfigurationSource().getInputStream();
         Properties props = new Properties();
@@ -331,9 +346,9 @@ public class PropertiesConfiguration  extends 
Log4j1Configuration {
      * Parse non-root elements, such non-root categories and renderers.
      */
     private void parseLoggers(Properties props) {
-        Enumeration enumeration = props.propertyNames();
+        Enumeration<?> enumeration = props.propertyNames();
         while (enumeration.hasMoreElements()) {
-            String key = (String) enumeration.nextElement();
+            String key = Objects.toString(enumeration.nextElement(), null);
             if (key.startsWith(CATEGORY_PREFIX) || 
key.startsWith(LOGGER_PREFIX)) {
                 String loggerName = null;
                 if (key.startsWith(CATEGORY_PREFIX)) {
@@ -455,10 +470,8 @@ public class PropertiesConfiguration  extends 
Log4j1Configuration {
                 appender.setErrorHandler(eh);
             }
         }
-        parseAppenderFilters(props, filterPrefix, appenderName);
-        String[] keys = new String[] {
-                layoutPrefix,
-        };
+        appender.addFilter(parseAppenderFilters(props, filterPrefix, 
appenderName));
+        String[] keys = new String[] { layoutPrefix };
         addProperties(appender, keys, props, prefix);
         if (appender instanceof AppenderWrapper) {
             addAppender(((AppenderWrapper) appender).getAppender());
@@ -506,7 +519,7 @@ public class PropertiesConfiguration  extends 
Log4j1Configuration {
 
     public void addProperties(final Object obj, final String[] keys, final 
Properties props, final String prefix) {
         final Properties edited = new Properties();
-        props.stringPropertyNames().stream().filter((name) -> {
+        props.stringPropertyNames().stream().filter(name -> {
             if (name.startsWith(prefix)) {
                 for (String key : keys) {
                     if (name.equals(key)) {
@@ -516,7 +529,7 @@ public class PropertiesConfiguration  extends 
Log4j1Configuration {
                 return true;
             }
             return false;
-        }).forEach((name) -> edited.put(name, props.getProperty(name)));
+        }).forEach(name -> edited.put(name, props.getProperty(name)));
         PropertySetter.setProperties(obj, edited, prefix + ".");
     }
 
@@ -527,7 +540,7 @@ public class PropertiesConfiguration  extends 
Log4j1Configuration {
         // name-value pairs associated to that filter
         int fIdx = filterPrefix.length();
         SortedMap<String, List<NameValue>> filters = new TreeMap<>();
-        Enumeration e = props.keys();
+        Enumeration<?> e = props.keys();
         String name = "";
         while (e.hasMoreElements()) {
             String key = (String) e.nextElement();
@@ -559,13 +572,12 @@ public class PropertiesConfiguration  extends 
Log4j1Configuration {
                 }
             }
             if (filter != null) {
-                if (head != null) {
+                if (head == null) {
                     head = filter;
-                    next = filter;
                 } else {
                     next.setNext(filter);
-                    next = filter;
                 }
+                next = filter;
             }
         }
         return head;
@@ -603,6 +615,7 @@ public class PropertiesConfiguration  extends 
Log4j1Configuration {
             this.value = value;
         }
 
+        @Override
         public String toString() {
             return key + "=" + value;
         }
diff --git 
a/log4j-1.2-api/src/test/java/org/apache/log4j/CustomNoopAppender.java 
b/log4j-1.2-api/src/test/java/org/apache/log4j/CustomNoopAppender.java
new file mode 100644
index 0000000..49de446
--- /dev/null
+++ b/log4j-1.2-api/src/test/java/org/apache/log4j/CustomNoopAppender.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.log4j;
+
+import org.apache.log4j.spi.LoggingEvent;
+
+public class CustomNoopAppender extends AppenderSkeleton {
+
+    @Override
+    public void close() {
+        // Noop
+    }
+
+    @Override
+    public boolean requiresLayout() {
+        return false;
+    }
+
+    @Override
+    protected void append(LoggingEvent event) {
+        // Noop
+    }
+
+}
diff --git 
a/log4j-1.2-api/src/test/java/org/apache/log4j/config/NeutralFilterFixture.java 
b/log4j-1.2-api/src/test/java/org/apache/log4j/config/NeutralFilterFixture.java
new file mode 100644
index 0000000..f6a20b2
--- /dev/null
+++ 
b/log4j-1.2-api/src/test/java/org/apache/log4j/config/NeutralFilterFixture.java
@@ -0,0 +1,32 @@
+/*
+ * 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.log4j.config;
+
+import org.apache.log4j.spi.Filter;
+import org.apache.log4j.spi.LoggingEvent;
+
+/**
+ * A test fixture used by {@code src/test/resources/LOG4J2-3247.properties}.
+ */
+public class NeutralFilterFixture extends Filter {
+
+    @Override
+    public int decide(LoggingEvent event) {
+        return NEUTRAL;
+    }
+
+}
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 72ff293..ede1b32 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,27 +16,29 @@
  */
 package org.apache.log4j.config;
 
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.InputStream;
+import java.util.List;
+import java.util.Map;
+
 import org.apache.log4j.ListAppender;
 import org.apache.log4j.LogManager;
 import org.apache.log4j.Logger;
 import org.apache.log4j.bridge.AppenderAdapter;
+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.config.Configuration;
 import org.apache.logging.log4j.core.config.ConfigurationSource;
 import org.apache.logging.log4j.core.config.Configurator;
+import org.apache.logging.log4j.core.filter.Filterable;
 import org.junit.Test;
 
-import java.io.File;
-import java.io.FileInputStream;
-import java.io.InputStream;
-import java.util.List;
-import java.util.Map;
-
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
-
 /**
  * Test configuration from Properties.
  */
@@ -56,6 +58,47 @@ public class PropertiesConfigurationTest {
     }
 
     @Test
+    public void testConfigureNullPointerException() throws Exception {
+        try (LoggerContext loggerContext = 
configure("target/test-classes/LOG4J2-3247.properties")) {
+            // [LOG4J2-3247] configure() should not throw an NPE.
+            Configuration configuration = loggerContext.getConfiguration();
+            assertNotNull(configuration);
+            Appender appender = configuration.getAppender("CONSOLE");
+            assertNotNull(appender);
+        }
+    }
+
+    @Test
+    public void testConsoleAppenderFilter() throws Exception {
+        try (LoggerContext loggerContext = 
configure("target/test-classes/LOG4J2-3247.properties")) {
+            // LOG4J2-3281 PropertiesConfiguration.buildAppender not adding 
filters to appender
+            Configuration configuration = loggerContext.getConfiguration();
+            assertNotNull(configuration);
+            Appender appender = configuration.getAppender("CONSOLE");
+            assertNotNull(appender);
+            Filterable filterable = (Filterable) appender;
+            FilterAdapter filter = (FilterAdapter) filterable.getFilter();
+            assertNotNull(filter);
+            assertTrue(filter.getFilter() instanceof NeutralFilterFixture);
+        }
+    }
+
+    @Test
+    public void testCustomAppenderFilter() throws Exception {
+        try (LoggerContext loggerContext = 
configure("target/test-classes/LOG4J2-3281.properties")) {
+            // LOG4J2-3281 PropertiesConfiguration.buildAppender not adding 
filters to appender
+            Configuration configuration = loggerContext.getConfiguration();
+            assertNotNull(configuration);
+            Appender appender = configuration.getAppender("CUSTOM");
+            assertNotNull(appender);
+            Filterable filterable = (Filterable) appender;
+            FilterAdapter filter = (FilterAdapter) filterable.getFilter();
+            assertNotNull(filter);
+            assertTrue(filter.getFilter() instanceof NeutralFilterFixture);
+        }
+    }
+
+    @Test
     public void testListAppender() throws Exception {
         LoggerContext loggerContext = 
configure("target/test-classes/log4j1-list.properties");
         Logger logger = LogManager.getLogger("test");
diff --git a/log4j-1.2-api/src/test/resources/LOG4J2-3247.properties 
b/log4j-1.2-api/src/test/resources/LOG4J2-3247.properties
new file mode 100644
index 0000000..b1d76e1
--- /dev/null
+++ b/log4j-1.2-api/src/test/resources/LOG4J2-3247.properties
@@ -0,0 +1,35 @@
+# 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.
+
+
+log4j.appender.CONSOLE=org.apache.log4j.ConsoleAppender
+log4j.appender.CONSOLE.filter.1=org.apache.log4j.config.NeutralFilterFixture
+log4j.appender.CONSOLE.filter.1.onMatch=neutral
+log4j.appender.CONSOLE.Target=System.out
+log4j.appender.CONSOLE.layout=org.apache.log4j.PatternLayout
+log4j.appender.CONSOLE.layout.ConversionPattern=%d{yyyy-MM-dd HH:mm:ss} %-5p 
%c{1}:%L - %m%n
+
+log4j.appender.A1=org.apache.log4j.FileAppender
+log4j.appender.A1.File=target/temp.A1
+log4j.appender.A1.Append=false
+log4j.appender.A1.layout=org.apache.log4j.PatternLayout
+log4j.appender.A1.layout.ConversionPattern=%-5p %c{2} - %m%n
+log4j.appender.A2=org.apache.log4j.FileAppender
+log4j.appender.A2.File=target/temp.A2
+log4j.appender.A2.Append=false
+log4j.appender.A2.layout=org.apache.log4j.TTCCLayout
+log4j.appender.A2.layout.DateFormat=ISO8601
+log4j.logger.org.apache.log4j.xml=trace, A1
+log4j.rootLogger=trace, CONSOLE, A1, A2
diff --git a/log4j-1.2-api/src/test/resources/LOG4J2-3281.properties 
b/log4j-1.2-api/src/test/resources/LOG4J2-3281.properties
new file mode 100644
index 0000000..4a45fd5
--- /dev/null
+++ b/log4j-1.2-api/src/test/resources/LOG4J2-3281.properties
@@ -0,0 +1,25 @@
+# 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.
+
+
+log4j.appender.CUSTOM=org.apache.log4j.CustomNoopAppender
+log4j.appender.CUSTOM.filter.1=org.apache.log4j.config.NeutralFilterFixture
+log4j.appender.CUSTOM.filter.1.onMatch=neutral
+log4j.appender.CUSTOM.Target=System.out
+log4j.appender.CUSTOM.layout=org.apache.log4j.PatternLayout
+log4j.appender.CUSTOM.layout.ConversionPattern=%d{yyyy-MM-dd HH:mm:ss} %-5p 
%c{1}:%L - %m%n
+
+log4j.logger.org.apache.log4j.xml=trace, CUSTOM
+log4j.rootLogger=trace, CUSTOM

Reply via email to