You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by pk...@apache.org on 2022/03/30 21:37:53 UTC
[logging-log4j2] 03/15: Allow for whitespace in property files.
This is an automated email from the ASF dual-hosted git repository.
pkarwasz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit 38d4392df0039e0dec730227ebbe7a36a73417fa
Author: Gary Gregory <ga...@gmail.com>
AuthorDate: Sat Feb 19 18:12:56 2022 -0500
Allow for whitespace in property files.
Tests are from PR #762 by ppkarwasz.
Conflicts:
log4j-1.2-api/src/main/java/org/apache/log4j/builders/BuilderManager.java
log4j-1.2-api/src/test/java/org/apache/log4j/config/Log4j1ConfigurationFactoryTest.java
log4j-1.2-api/src/test/java/org/apache/log4j/config/PropertiesConfigurationTest.java
---
.../org/apache/log4j/builders/BuilderManager.java | 105 ++++++++++++---------
.../log4j/config/Log4j1ConfigurationParser.java | 11 ++-
.../config/Log4j1ConfigurationFactoryTest.java | 23 +++++
.../log4j/config/PropertiesConfigurationTest.java | 51 ++++++++--
.../config-1.2/log4j-untrimmed.properties | 28 ++++++
5 files changed, 156 insertions(+), 62 deletions(-)
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 6672c8e..d0a8531 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,7 +36,7 @@ import org.apache.logging.log4j.status.StatusLogger;
import org.apache.logging.log4j.util.LoaderUtil;
import org.w3c.dom.Element;
-import java.lang.reflect.Constructor;
+import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.Properties;
@@ -58,21 +58,48 @@ public class BuilderManager {
plugins = injector.getInstance(PLUGIN_MANAGER_KEY).getPlugins();
}
+ @SuppressWarnings("unchecked")
+ private <T extends Builder> T createBuilder(PluginType<T> plugin, String prefix, Properties props) {
+ try {
+ Class<T> clazz = plugin.getPluginClass();
+ if (AbstractBuilder.class.isAssignableFrom(clazz)) {
+ return clazz.getConstructor(CONSTRUCTOR_PARAMS).newInstance(prefix, props);
+ }
+ Object builder = LoaderUtil.newInstanceOf(clazz);
+ // Reasonable message instead of `ClassCastException`
+ if (!Builder.class.isAssignableFrom(clazz)) {
+ LOGGER.warn("Unable to load plugin: builder {} does not implement {}", clazz, Builder.class);
+ return null;
+ }
+ return (T) builder;
+ } catch (ReflectiveOperationException ex) {
+ LOGGER.warn("Unable to load plugin: {} due to: {}", plugin.getKey(), ex.getMessage());
+ return null;
+ }
+ }
+
+ @SuppressWarnings("unchecked")
+ private <T> PluginType<T> getPlugin(String className) {
+ Objects.requireNonNull(plugins, "plugins");
+ Objects.requireNonNull(className, "className");
+ return (PluginType<T>) plugins.get(className.toLowerCase(Locale.ROOT).trim());
+ }
+
public Appender parseAppender(String className, Element appenderElement, XmlConfiguration config) {
- PluginType<?> plugin = plugins.get(className.toLowerCase());
+ PluginType<AppenderBuilder> plugin = getPlugin(className);
if (plugin != null) {
- final Class<? extends AppenderBuilder> pluginClass = plugin.getPluginClass().asSubclass(AppenderBuilder.class);
- final AppenderBuilder builder = injector.getInstance(pluginClass);
- return builder.parseAppender(appenderElement, config);
+ try {
+ return LoaderUtil.newInstanceOf(plugin.getPluginClass()).parseAppender(appenderElement, config);
+ } catch (ReflectiveOperationException ex) {
+ LOGGER.warn("Unable to load plugin: {} due to: {}", plugin.getKey(), ex.getMessage());
+ }
}
return null;
}
- 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());
+ public Appender parseAppender(String name, String className, String prefix, String layoutPrefix, String filterPrefix, Properties props,
+ PropertiesConfiguration config) {
+ PluginType<AppenderBuilder> plugin = getPlugin(className);
if (plugin != null) {
AppenderBuilder builder = createBuilder(plugin, prefix, props);
if (builder != null) {
@@ -83,17 +110,19 @@ public class BuilderManager {
}
public Filter parseFilter(String className, Element filterElement, XmlConfiguration config) {
- PluginType<?> plugin = plugins.get(className.toLowerCase());
+ PluginType<FilterBuilder> plugin = getPlugin(className);
if (plugin != null) {
- final Class<? extends FilterBuilder> pluginClass = plugin.getPluginClass().asSubclass(FilterBuilder.class);
- final FilterBuilder builder = injector.getInstance(pluginClass);
- return builder.parseFilter(filterElement, config);
+ try {
+ return LoaderUtil.newInstanceOf(plugin.getPluginClass()).parseFilter(filterElement, config);
+ } catch (ReflectiveOperationException ex) {
+ LOGGER.warn("Unable to load plugin: {} due to: {}", plugin.getKey(), ex.getMessage());
+ }
}
return null;
}
public Filter parseFilter(String className, String filterPrefix, Properties props, PropertiesConfiguration config) {
- PluginType<?> plugin = plugins.get(className.toLowerCase());
+ PluginType<FilterBuilder> plugin = getPlugin(className);
if (plugin != null) {
FilterBuilder builder = createBuilder(plugin, filterPrefix, props);
if (builder != null) {
@@ -104,17 +133,19 @@ public class BuilderManager {
}
public Layout parseLayout(String className, Element layoutElement, XmlConfiguration config) {
- PluginType<?> plugin = plugins.get(className.toLowerCase());
+ PluginType<LayoutBuilder> plugin = getPlugin(className);
if (plugin != null) {
- final Class<? extends LayoutBuilder> pluginClass = plugin.getPluginClass().asSubclass(LayoutBuilder.class);
- final LayoutBuilder builder = injector.getInstance(pluginClass);
- return builder.parseLayout(layoutElement, config);
+ try {
+ return LoaderUtil.newInstanceOf(plugin.getPluginClass()).parseLayout(layoutElement, config);
+ } catch (ReflectiveOperationException ex) {
+ LOGGER.warn("Unable to load plugin: {} due to: {}", plugin.getKey(), ex.getMessage());
+ }
}
return null;
}
public Layout parseLayout(String className, String layoutPrefix, Properties props, PropertiesConfiguration config) {
- PluginType<?> plugin = plugins.get(className.toLowerCase());
+ PluginType<LayoutBuilder> plugin = getPlugin(className);
if (plugin != null) {
LayoutBuilder builder = createBuilder(plugin, layoutPrefix, props);
if (builder != null) {
@@ -125,17 +156,19 @@ public class BuilderManager {
}
public RewritePolicy parseRewritePolicy(String className, Element rewriteElement, XmlConfiguration config) {
- PluginType<?> plugin = plugins.get(className.toLowerCase());
+ PluginType<RewritePolicyBuilder> plugin = getPlugin(className);
if (plugin != null) {
- final Class<? extends RewritePolicyBuilder> pluginClass = plugin.getPluginClass().asSubclass(RewritePolicyBuilder.class);
- final RewritePolicyBuilder builder = injector.getInstance(pluginClass);
- return builder.parseRewritePolicy(rewriteElement, config);
+ try {
+ return LoaderUtil.newInstanceOf(plugin.getPluginClass()).parseRewritePolicy(rewriteElement, config);
+ } catch (ReflectiveOperationException ex) {
+ LOGGER.warn("Unable to load plugin: {} due to: {}", plugin.getKey(), ex.getMessage());
+ }
}
return null;
}
public RewritePolicy parseRewritePolicy(String className, String policyPrefix, Properties props, PropertiesConfiguration config) {
- PluginType<?> plugin = plugins.get(className.toLowerCase());
+ PluginType<RewritePolicyBuilder> plugin = getPlugin(className);
if (plugin != null) {
RewritePolicyBuilder builder = createBuilder(plugin, policyPrefix, props);
if (builder != null) {
@@ -145,26 +178,4 @@ public class BuilderManager {
return null;
}
- @SuppressWarnings("unchecked")
- private <T extends Builder> T createBuilder(PluginType<?> plugin, String prefix, Properties props) {
- try {
- Class<?> clazz = plugin.getPluginClass();
- if (AbstractBuilder.class.isAssignableFrom(clazz)) {
- Constructor<T> constructor =
- (Constructor<T>) clazz.getConstructor(CONSTRUCTOR_PARAMS);
- return constructor.newInstance(prefix, props);
- }
- Object builder = LoaderUtil.newInstanceOf(clazz);
- // Reasonable message instead of `ClassCastException`
- if (!Builder.class.isAssignableFrom(clazz)) {
- LOGGER.warn("Unable to load plugin: builder {} does not implement {}", clazz, Builder.class);
- return null;
- }
- return (T) 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/Log4j1ConfigurationParser.java b/log4j-1.2-api/src/main/java/org/apache/log4j/config/Log4j1ConfigurationParser.java
index 5425244..d345ba0 100644
--- a/log4j-1.2-api/src/main/java/org/apache/log4j/config/Log4j1ConfigurationParser.java
+++ b/log4j-1.2-api/src/main/java/org/apache/log4j/config/Log4j1ConfigurationParser.java
@@ -143,13 +143,13 @@ public class Log4j1ConfigurationParser {
for (final Map.Entry<Object, Object> entry : properties.entrySet()) {
final Object keyObj = entry.getKey();
if (keyObj != null) {
- final String key = keyObj.toString();
+ final String key = keyObj.toString().trim();
if (key.startsWith(prefix)) {
if (key.indexOf('.', preLength) < 0) {
final String name = key.substring(preLength);
final Object value = entry.getValue();
if (value != null) {
- map.put(name, value.toString());
+ map.put(name, value.toString().trim());
}
}
}
@@ -394,13 +394,13 @@ public class Log4j1ConfigurationParser {
for (final Map.Entry<Object, Object> entry : properties.entrySet()) {
final Object keyObj = entry.getKey();
if (keyObj != null) {
- final String key = keyObj.toString();
+ final String key = keyObj.toString().trim();
if (key.startsWith(prefix)) {
final String name = key.substring(preLength);
final Object value = entry.getValue();
if (value != null) {
// a Level may be followed by a list of Appender refs.
- final String valueStr = value.toString();
+ final String valueStr = value.toString().trim();
final String[] split = valueStr.split(COMMA_DELIMITED_RE);
final String level = getLevelString(split, null);
if (level == null) {
@@ -429,7 +429,8 @@ public class Log4j1ConfigurationParser {
private String getProperty(final String key) {
final String value = properties.getProperty(key);
- return OptionConverter.substVars(value, properties);
+ final String substVars = OptionConverter.substVars(value, properties);
+ return substVars == null ? null : substVars.trim();
}
private String getProperty(final String key, final String defaultValue) {
diff --git a/log4j-1.2-api/src/test/java/org/apache/log4j/config/Log4j1ConfigurationFactoryTest.java b/log4j-1.2-api/src/test/java/org/apache/log4j/config/Log4j1ConfigurationFactoryTest.java
index e2638ec..26c000a 100644
--- a/log4j-1.2-api/src/test/java/org/apache/log4j/config/Log4j1ConfigurationFactoryTest.java
+++ b/log4j-1.2-api/src/test/java/org/apache/log4j/config/Log4j1ConfigurationFactoryTest.java
@@ -16,21 +16,25 @@
*/
package org.apache.log4j.config;
+import static org.junit.Assert.fail;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import java.io.Serializable;
import java.net.URISyntaxException;
import java.net.URL;
import org.apache.log4j.layout.Log4j1XmlLayout;
import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.core.Appender;
import org.apache.logging.log4j.core.Layout;
import org.apache.logging.log4j.core.appender.ConsoleAppender;
import org.apache.logging.log4j.core.appender.ConsoleAppender.Target;
import org.apache.logging.log4j.core.config.Configuration;
import org.apache.logging.log4j.core.config.LoggerConfig;
+import org.apache.logging.log4j.core.layout.PatternLayout;
import org.junit.Test;
public class Log4j1ConfigurationFactoryTest extends AbstractLog4j1ConfigurationTest {
@@ -150,4 +154,23 @@ public class Log4j1ConfigurationFactoryTest extends AbstractLog4j1ConfigurationT
public void testDefaultValues() throws Exception {
super.testDefaultValues();
}
+
+ @Test
+ public void testUntrimmedValues() throws Exception {
+ try {
+ final Configuration config = getConfiguration("config-1.2/log4j-untrimmed");
+ final LoggerConfig rootLogger = config.getRootLogger();
+ assertEquals(Level.DEBUG, rootLogger.getLevel());
+ final Appender appender = config.getAppender("Console");
+ assertTrue(appender instanceof ConsoleAppender);
+ final Layout<? extends Serializable> layout = appender.getLayout();
+ assertTrue(layout instanceof PatternLayout);
+ assertEquals("%level - %m%n", ((PatternLayout)layout).getConversionPattern());
+ // No filter support
+ config.start();
+ config.stop();
+ } catch (NoClassDefFoundError e) {
+ fail(e.getMessage());
+ }
+ }
}
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 09208cd..0c3ddc0 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,20 @@
*/
package org.apache.log4j.config;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.Serializable;
+import java.net.URISyntaxException;
+import java.nio.file.Path;
+import java.util.List;
+import java.util.Map;
+
import org.apache.log4j.ListAppender;
import org.apache.log4j.LogManager;
import org.apache.log4j.Logger;
@@ -24,26 +38,22 @@ import org.apache.log4j.bridge.FilterAdapter;
import org.apache.log4j.spi.LoggingEvent;
import org.apache.logging.log4j.Level;
import org.apache.logging.log4j.core.Appender;
+import org.apache.logging.log4j.core.Filter;
+import org.apache.logging.log4j.core.Layout;
import org.apache.logging.log4j.core.LoggerContext;
+import org.apache.logging.log4j.core.appender.ConsoleAppender;
import org.apache.logging.log4j.core.appender.FileAppender;
import org.apache.logging.log4j.core.config.Configuration;
import org.apache.logging.log4j.core.config.ConfigurationSource;
+import org.apache.logging.log4j.core.config.LoggerConfig;
import org.apache.logging.log4j.core.filter.CompositeFilter;
+import org.apache.logging.log4j.core.filter.DenyAllFilter;
import org.apache.logging.log4j.core.filter.Filterable;
import org.apache.logging.log4j.core.filter.LevelRangeFilter;
+import org.apache.logging.log4j.core.layout.PatternLayout;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
-import java.io.File;
-import java.io.IOException;
-import java.io.InputStream;
-import java.net.URISyntaxException;
-import java.nio.file.Path;
-import java.util.List;
-import java.util.Map;
-
-import static org.junit.jupiter.api.Assertions.*;
-
/**
* Test configuration from Properties.
*/
@@ -271,4 +281,25 @@ public class PropertiesConfigurationTest extends AbstractLog4j1ConfigurationTest
public void testMultipleFilters(final @TempDir Path tmpFolder) throws Exception {
super.testMultipleFilters(tmpFolder);
}
+
+ @Test
+ public void testUntrimmedValues() throws Exception {
+ try {
+ final Configuration config = getConfiguration("config-1.2/log4j-untrimmed");
+ final LoggerConfig rootLogger = config.getRootLogger();
+ assertEquals(Level.DEBUG, rootLogger.getLevel());
+ final Appender appender = config.getAppender("Console");
+ assertTrue(appender instanceof ConsoleAppender);
+ final Layout<? extends Serializable> layout = appender.getLayout();
+ assertTrue(layout instanceof PatternLayout);
+ assertEquals("%level - %m%n", ((PatternLayout)layout).getConversionPattern());
+ final Filter filter = ((Filterable) appender).getFilter();
+ assertTrue(filter instanceof DenyAllFilter);
+ config.start();
+ config.stop();
+ } catch (NoClassDefFoundError e) {
+ fail(e.getMessage());
+ }
+ }
+
}
diff --git a/log4j-1.2-api/src/test/resources/config-1.2/log4j-untrimmed.properties b/log4j-1.2-api/src/test/resources/config-1.2/log4j-untrimmed.properties
new file mode 100644
index 0000000..66b2fba
--- /dev/null
+++ b/log4j-1.2-api/src/test/resources/config-1.2/log4j-untrimmed.properties
@@ -0,0 +1,28 @@
+#
+# 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.
+#
+
+###
+# Warning: this file contains INTENTIONAL trailing spaces on all properties
+#
+
+log4j.threshold = INFO
+
+log4j.appender.Console = org.apache.log4j.ConsoleAppender
+log4j.appender.Console.layout = org.apache.log4j.SimpleLayout
+log4j.appender.Console.filter.1 = org.apache.log4j.varia.DenyAllFilter
+
+log4j.rootLogger = DEBUG , Console
\ No newline at end of file