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