You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by ck...@apache.org on 2022/01/07 19:58:56 UTC
[logging-log4j2] 01/05: LOG4J2-3270 Provide separation between MapMessage and properties lookups
This is an automated email from the ASF dual-hosted git repository.
ckozak pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit 36511ac01d899f0ae76abe6176b4237d57885cc4
Author: Carter Kozak <ck...@apache.org>
AuthorDate: Tue Dec 21 14:07:06 2021 -0500
LOG4J2-3270 Provide separation between MapMessage and properties lookups
MapLookup includes special handling for MapMessages in addition
to the configuration properties map. This change aims to prevent
unwanted interactions between these two concerns.
---
.../RollingFileAppenderReconfigureTest.java | 2 +-
.../log4j/core/lookup/InterpolatorTest.java | 70 ++++++++++++++++++++--
.../core/lookup/MainInputArgumentsMapLookup.java | 16 +++++
.../log4j/core/lookup/StrSubstitutorTest.java | 33 +++++++++-
.../log4j/core/config/AbstractConfiguration.java | 4 +-
.../log4j/core/config/PropertiesPlugin.java | 4 +-
.../logging/log4j/core/lookup/Interpolator.java | 5 +-
.../lookup/JmxRuntimeInputArgumentsLookup.java | 14 +++++
.../logging/log4j/core/lookup/MapLookup.java | 4 +-
.../log4j/core/lookup/PropertiesLookup.java | 68 +++++++++++++++++++++
.../logging/log4j/core/lookup/StrSubstitutor.java | 8 +--
src/changes/changes.xml | 3 +
12 files changed, 210 insertions(+), 21 deletions(-)
diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/rolling/RollingFileAppenderReconfigureTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/rolling/RollingFileAppenderReconfigureTest.java
index 8f9a12a..55190e1 100644
--- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/rolling/RollingFileAppenderReconfigureTest.java
+++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/rolling/RollingFileAppenderReconfigureTest.java
@@ -27,7 +27,7 @@ import org.junit.Test;
public class RollingFileAppenderReconfigureTest {
@Rule
- public final LoggerContextRule loggerContextRule = new LoggerContextRule("src/test/rolling-file-appender-reconfigure.xml");
+ public final LoggerContextRule loggerContextRule = new LoggerContextRule("rolling-file-appender-reconfigure.xml");
@Test
public void testReconfigure() {
diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/lookup/InterpolatorTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/lookup/InterpolatorTest.java
index 234c91f..d366d7a 100644
--- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/lookup/InterpolatorTest.java
+++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/lookup/InterpolatorTest.java
@@ -17,16 +17,25 @@
package org.apache.logging.log4j.core.lookup;
import java.text.SimpleDateFormat;
+import java.util.Collections;
import java.util.Date;
import java.util.HashMap;
import java.util.Map;
+import org.apache.logging.log4j.Level;
import org.apache.logging.log4j.ThreadContext;
-import org.junit.AfterClass;
-import org.junit.BeforeClass;
-import org.junit.Test;
+import org.apache.logging.log4j.core.LogEvent;
+import org.apache.logging.log4j.core.Logger;
+import org.apache.logging.log4j.core.impl.Log4jLogEvent;
+import org.apache.logging.log4j.message.StringMapMessage;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
-import static org.junit.Assert.*;
+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.assertNull;
/**
*
@@ -40,13 +49,13 @@ public class InterpolatorTest {
private static final String TEST_CONTEXT_RESOURCE_NAME = "logging/context-name";
private static final String TEST_CONTEXT_NAME = "app-1";
- @BeforeClass
+ @BeforeAll
public static void beforeClass() {
System.setProperty(TESTKEY, TESTVAL);
System.setProperty(TESTKEY2, TESTVAL);
}
- @AfterClass
+ @AfterAll
public static void afterClass() {
System.clearProperty(TESTKEY);
System.clearProperty(TESTKEY2);
@@ -99,4 +108,53 @@ public class InterpolatorTest {
assertLookupNotEmpty(lookup, "java:locale");
assertLookupNotEmpty(lookup, "java:hw");
}
+
+
+ @Test
+ public void testInterpolatorMapMessageWithNoPrefix() {
+ final HashMap<String, String> configProperties = new HashMap<>();
+ configProperties.put("key", "configProperties");
+ Interpolator interpolator = new Interpolator(configProperties);
+ final HashMap<String, String> map = new HashMap<>();
+ map.put("key", "mapMessage");
+ LogEvent event = Log4jLogEvent.newBuilder()
+ .setLoggerName(getClass().getName())
+ .setLoggerFqcn(Logger.class.getName())
+ .setLevel(Level.INFO)
+ .setMessage(new StringMapMessage(map))
+ .build();
+ assertEquals("configProperties", interpolator.lookup(event, "key"));
+ }
+
+ @Test
+ public void testInterpolatorMapMessageWithNoPrefixConfigDoesntMatch() {
+ Interpolator interpolator = new Interpolator(Collections.emptyMap());
+ final HashMap<String, String> map = new HashMap<>();
+ map.put("key", "mapMessage");
+ LogEvent event = Log4jLogEvent.newBuilder()
+ .setLoggerName(getClass().getName())
+ .setLoggerFqcn(Logger.class.getName())
+ .setLevel(Level.INFO)
+ .setMessage(new StringMapMessage(map))
+ .build();
+ assertNull(
+ interpolator.lookup(event, "key"),
+ "Values without a map prefix should not match MapMessages");
+ }
+
+ @Test
+ public void testInterpolatorMapMessageWithMapPrefix() {
+ final HashMap<String, String> configProperties = new HashMap<>();
+ configProperties.put("key", "configProperties");
+ Interpolator interpolator = new Interpolator(configProperties);
+ final HashMap<String, String> map = new HashMap<>();
+ map.put("key", "mapMessage");
+ LogEvent event = Log4jLogEvent.newBuilder()
+ .setLoggerName(getClass().getName())
+ .setLoggerFqcn(Logger.class.getName())
+ .setLevel(Level.INFO)
+ .setMessage(new StringMapMessage(map))
+ .build();
+ assertEquals("configProperties", interpolator.lookup(event, "map:key"));
+ }
}
diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/lookup/MainInputArgumentsMapLookup.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/lookup/MainInputArgumentsMapLookup.java
index 5ce3efb..9245621 100644
--- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/lookup/MainInputArgumentsMapLookup.java
+++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/lookup/MainInputArgumentsMapLookup.java
@@ -16,6 +16,8 @@
*/
package org.apache.logging.log4j.core.lookup;
+import org.apache.logging.log4j.core.LogEvent;
+
import java.util.Map;
/**
@@ -54,4 +56,18 @@ public class MainInputArgumentsMapLookup extends MapLookup {
public MainInputArgumentsMapLookup(final Map<String, String> map) {
super(map);
}
+
+ @Override
+ public String lookup(final LogEvent event, final String key) {
+ return lookup(key);
+ }
+
+ @Override
+ public String lookup(final String key) {
+ if (key == null) {
+ return null;
+ }
+ Map<String, String> map = getMap();
+ return map == null ? null : map.get(key);
+ }
}
diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/lookup/StrSubstitutorTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/lookup/StrSubstitutorTest.java
index 375ff30..53f0fda 100644
--- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/lookup/StrSubstitutorTest.java
+++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/lookup/StrSubstitutorTest.java
@@ -25,7 +25,7 @@ import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
-import static org.junit.jupiter.api.Assertions.*;
+import static org.junit.jupiter.api.Assertions.assertEquals;
public class StrSubstitutorTest {
@@ -43,7 +43,6 @@ public class StrSubstitutorTest {
System.clearProperty(TESTKEY);
}
-
@Test
public void testLookup() {
final Map<String, String> map = new HashMap<>();
@@ -207,4 +206,34 @@ public class StrSubstitutorTest {
subst.setRecursiveEvaluationAllowed(false);
assertEquals("success ${foo:throw} success", subst.replace("${foo:a} ${foo:throw} ${foo:c}"));
}
+
+ @Test
+ public void testTopLevelLookupsWithoutRecursiveEvaluation() {
+ final Map<String, String> map = new HashMap<>();
+ map.put("key", "VaLuE");
+ final StrLookup lookup = new Interpolator(new MapLookup(map));
+ final StrSubstitutor subst = new StrSubstitutor(lookup);
+ subst.setRecursiveEvaluationAllowed(false);
+ assertEquals("value", subst.replace("${lower:${ctx:key}}"));
+ }
+
+ @Test
+ public void testTopLevelLookupsWithoutRecursiveEvaluation_doubleLower() {
+ final Map<String, String> map = new HashMap<>();
+ map.put("key", "VaLuE");
+ final StrLookup lookup = new Interpolator(new MapLookup(map));
+ final StrSubstitutor subst = new StrSubstitutor(lookup);
+ subst.setRecursiveEvaluationAllowed(false);
+ assertEquals("value", subst.replace("${lower:${lower:${ctx:key}}}"));
+ }
+
+ @Test
+ public void testTopLevelLookupsWithoutRecursiveEvaluationAndDefaultValueLookup() {
+ final Map<String, String> map = new HashMap<>();
+ map.put("key2", "TWO");
+ final StrLookup lookup = new Interpolator(new MapLookup(map));
+ final StrSubstitutor subst = new StrSubstitutor(lookup);
+ subst.setRecursiveEvaluationAllowed(false);
+ assertEquals("two", subst.replace("${lower:${ctx:key1:-${ctx:key2}}}"));
+ }
}
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java
index 9b9b7b5..126e142 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java
@@ -57,7 +57,7 @@ import org.apache.logging.log4j.core.filter.AbstractFilterable;
import org.apache.logging.log4j.core.layout.PatternLayout;
import org.apache.logging.log4j.core.lookup.ConfigurationStrSubstitutor;
import org.apache.logging.log4j.core.lookup.Interpolator;
-import org.apache.logging.log4j.core.lookup.MapLookup;
+import org.apache.logging.log4j.core.lookup.PropertiesLookup;
import org.apache.logging.log4j.core.lookup.RuntimeStrSubstitutor;
import org.apache.logging.log4j.core.lookup.StrLookup;
import org.apache.logging.log4j.core.lookup.StrSubstitutor;
@@ -633,7 +633,7 @@ public abstract class AbstractConfiguration extends AbstractFilterable implement
}
} else {
final Map<String, String> map = this.getComponent(CONTEXT_PROPERTIES);
- final StrLookup lookup = map == null ? null : new MapLookup(map);
+ final StrLookup lookup = map == null ? null : new PropertiesLookup(map);
Interpolator interpolator = new Interpolator(lookup, pluginPackages);
subst.setVariableResolver(interpolator);
configurationStrSubstitutor.setVariableResolver(interpolator);
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/PropertiesPlugin.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/PropertiesPlugin.java
index 9ead135..d027831 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/PropertiesPlugin.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/PropertiesPlugin.java
@@ -22,7 +22,7 @@ import java.util.Map;
import org.apache.logging.log4j.core.config.plugins.PluginConfiguration;
import org.apache.logging.log4j.core.lookup.Interpolator;
-import org.apache.logging.log4j.core.lookup.MapLookup;
+import org.apache.logging.log4j.core.lookup.PropertiesLookup;
import org.apache.logging.log4j.core.lookup.StrLookup;
import org.apache.logging.log4j.plugins.Node;
import org.apache.logging.log4j.plugins.Plugin;
@@ -56,6 +56,6 @@ public final class PropertiesPlugin {
map.put(prop.getName(), prop.getValue());
}
- return new Interpolator(new MapLookup(map), config.getPluginPackages());
+ return new Interpolator(new PropertiesLookup(map), config.getPluginPackages());
}
}
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
index 7def2d9..980f6a6 100644
--- 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
@@ -76,7 +76,7 @@ public class Interpolator extends AbstractConfigurationAwareLookup {
* @since 2.1
*/
public Interpolator(final StrLookup defaultLookup, final List<String> pluginPackages) {
- this.defaultLookup = defaultLookup == null ? new MapLookup(new HashMap<String, String>()) : defaultLookup;
+ this.defaultLookup = defaultLookup == null ? new PropertiesLookup(new HashMap<String, String>()) : defaultLookup;
final PluginManager manager = new PluginManager(CATEGORY);
manager.collectPlugins(pluginPackages);
final Map<String, PluginType<?>> plugins = manager.getPlugins();
@@ -104,12 +104,13 @@ public class Interpolator extends AbstractConfigurationAwareLookup {
* Creates the Interpolator using only Lookups that work without an event and initial properties.
*/
public Interpolator(final Map<String, String> properties) {
- this.defaultLookup = new MapLookup(properties == null ? new HashMap<String, String>() : properties);
+ this.defaultLookup = new PropertiesLookup(properties);
// TODO: this ought to use the PluginManager
strLookupMap.put("log4j", new Log4jLookup());
strLookupMap.put("sys", new SystemPropertiesLookup());
strLookupMap.put("env", new EnvironmentLookup());
strLookupMap.put("main", MainMapLookup.MAIN_SINGLETON);
+ strLookupMap.put("map", new MapLookup(properties));
strLookupMap.put("marker", new MarkerLookup());
strLookupMap.put("java", new JavaLookup());
strLookupMap.put("base64", new Base64StrLookup());
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/JmxRuntimeInputArgumentsLookup.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/JmxRuntimeInputArgumentsLookup.java
index e1cec53..2e3c5b0 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/JmxRuntimeInputArgumentsLookup.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/JmxRuntimeInputArgumentsLookup.java
@@ -20,6 +20,7 @@ import java.lang.management.ManagementFactory;
import java.util.List;
import java.util.Map;
+import org.apache.logging.log4j.core.LogEvent;
import org.apache.logging.log4j.plugins.Plugin;
/**
@@ -49,4 +50,17 @@ public class JmxRuntimeInputArgumentsLookup extends MapLookup {
super(map);
}
+ @Override
+ public String lookup(final LogEvent event, final String key) {
+ return lookup(key);
+ }
+
+ @Override
+ public String lookup(final String key) {
+ if (key == null) {
+ return null;
+ }
+ Map<String, String> map = getMap();
+ return map == null ? null : map.get(key);
+ }
}
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/MapLookup.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/MapLookup.java
index 7d60979..dba88c7 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/MapLookup.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/MapLookup.java
@@ -43,7 +43,7 @@ public class MapLookup implements StrLookup {
}
/**
- * Creates a new instance backed by a Map. Used by the default lookup.
+ * Creates a new instance backed by a Map.
*
* @param map
* the map of keys to values, may be null
@@ -115,7 +115,7 @@ public class MapLookup implements StrLookup {
*/
@Override
public String lookup(final String key) {
- if (map == null) {
+ if (key == null || map == null) {
return null;
}
return map.get(key);
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/PropertiesLookup.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/PropertiesLookup.java
new file mode 100644
index 0000000..995a71b
--- /dev/null
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/PropertiesLookup.java
@@ -0,0 +1,68 @@
+/*
+ * 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.logging.log4j.core.lookup;
+
+import org.apache.logging.log4j.core.LogEvent;
+
+import java.util.Collections;
+import java.util.Map;
+
+/**
+ * A lookup designed for {@code Properties} defined in the configuration. This is similar
+ * to {@link MapLookup} without special handling for structured messages.
+ *
+ * Note that this lookup is not a plugin, but wired as a default lookup in the configuration.
+ */
+public final class PropertiesLookup implements StrLookup {
+
+ /**
+ * Configuration from which to read properties.
+ */
+ private final Map<String, String> properties;
+
+ public PropertiesLookup(final Map<String, String> properties) {
+ this.properties = properties == null
+ ? Collections.emptyMap()
+ : properties;
+ }
+
+ @Override
+ public String lookup(
+ @SuppressWarnings("ignored") final LogEvent event,
+ final String key) {
+ return lookup(key);
+ }
+
+ /**
+ * Looks a value from configuration properties.
+ * <p>
+ * If the property is not defined, then null is returned.
+ * </p>
+ *
+ * @param key the key to be looked up, may be null
+ * @return the matching value, null if no match
+ */
+ @Override
+ public String lookup(final String key) {
+ return key == null ? null : properties.get(key);
+ }
+
+ @Override
+ public String toString() {
+ return "PropertiesLookup{properties=" + properties + '}';
+ }
+}
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/StrSubstitutor.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/StrSubstitutor.java
index 4818ab0..3c068fe 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/StrSubstitutor.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/StrSubstitutor.java
@@ -227,7 +227,7 @@ public class StrSubstitutor implements ConfigurationAware {
* @param valueMap the map with the variables' values, may be null
*/
public StrSubstitutor(final Map<String, String> valueMap) {
- this(new MapLookup(valueMap), DEFAULT_PREFIX, DEFAULT_SUFFIX, DEFAULT_ESCAPE);
+ this(new PropertiesLookup(valueMap), DEFAULT_PREFIX, DEFAULT_SUFFIX, DEFAULT_ESCAPE);
}
/**
@@ -239,7 +239,7 @@ public class StrSubstitutor implements ConfigurationAware {
* @throws IllegalArgumentException if the prefix or suffix is null
*/
public StrSubstitutor(final Map<String, String> valueMap, final String prefix, final String suffix) {
- this(new MapLookup(valueMap), prefix, suffix, DEFAULT_ESCAPE);
+ this(new PropertiesLookup(valueMap), prefix, suffix, DEFAULT_ESCAPE);
}
/**
@@ -253,7 +253,7 @@ public class StrSubstitutor implements ConfigurationAware {
*/
public StrSubstitutor(final Map<String, String> valueMap, final String prefix, final String suffix,
final char escape) {
- this(new MapLookup(valueMap), prefix, suffix, escape);
+ this(new PropertiesLookup(valueMap), prefix, suffix, escape);
}
/**
@@ -268,7 +268,7 @@ public class StrSubstitutor implements ConfigurationAware {
*/
public StrSubstitutor(final Map<String, String> valueMap, final String prefix, final String suffix,
final char escape, final String valueDelimiter) {
- this(new MapLookup(valueMap), prefix, suffix, escape, valueDelimiter);
+ this(new PropertiesLookup(valueMap), prefix, suffix, escape, valueDelimiter);
}
/**
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 9558ae1..5b91ba6 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -265,6 +265,9 @@
<action issue="LOG4J2-3270" dev="ggregory" type="fix" due-to="Lee Dongjin">
Reduce ignored package scope of KafkaAppender.
</action>
+ <action issue="LOG4J2-3270" dev="ckozak" type="fix">
+ Lookups with no prefix only read values from the configuration properties as expected.
+ </action>
</release>
<release version="2.17.0" date="2021-12-17" description="GA Release 2.17.0">
<action issue="LOG4J2-3230" dev="ckozak" type="fix">