This is an automated email from the ASF dual-hosted git repository.
ckozak 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 17a77f5 LOG4J2-3317: Fix RoutingAppender backcompat and improve
lookup security
17a77f5 is described below
commit 17a77f5c27bda545de4b8eda46590401313e0be8
Author: Carter Kozak <[email protected]>
AuthorDate: Wed Jan 19 18:12:49 2022 -0500
LOG4J2-3317: Fix RoutingAppender backcompat and improve lookup security
This change unifies our configuration-time and runtime property
handling logic in order to make configuration-time properties
safer and more obvious, while allowing RoutingAppender configurations
that were broken by 2.17.1 to work as expected once again.
This is done using two high-level ideas:
1. Properties defined in the configuration are aware of lookups,
so the lookup-engine may resolve properties defined in the
top level logging configuraton recursively. This applies only
to the property literal, not the result of lookups used within
configuration properties.
2. No recursive evaluation of lookup results. While properties are
a special case, the data returned from other sources must not
itself be evaluated. This matches the restriction we introduced
in 2.17.1 for runtime lookups, however it applies everywhere
and shouldn't result in breaks due to property traversal being
allowed.
This introduces some level of risk, unlike the changes
in 2.17.1, this update the behavior of all lookups and is not
bounded to less-common runtime lookups.
We do not expect this to break users configurations, however
there are some cases in which we have intentionally changed
behavior:
Given environment `FOO=${lower:BAR}`
Using `<File name="${env:FOO}">` in a configuration will result
in a File appender with the literal name `${lower:BAR}` where
it would have previously evaluated to `bar`. This resolves an
ambiguity problem where those setting environment variables
don't need to be aware of log4j2 or our lookup syntax.
---
log4j-core/revapi.json | 27 ++++
.../core/appender/HttpURLConnectionManager.java | 2 +-
.../logging/log4j/core/async/AsyncLogger.java | 4 +-
.../logging/log4j/core/config/LoggerConfig.java | 6 +-
.../log4j/core/config/PropertiesPlugin.java | 67 +++++++--
.../apache/logging/log4j/core/config/Property.java | 43 +++++-
.../log4j/core/config/plugins/PluginValue.java | 3 +
.../plugins/visitors/PluginValueVisitor.java | 4 +-
.../logging/log4j/core/lookup/AbstractLookup.java | 12 +-
.../core/lookup/ConfigurationStrSubstitutor.java | 11 --
...bstractLookup.java => DefaultLookupResult.java} | 35 +++--
.../logging/log4j/core/lookup/Interpolator.java | 25 +++-
.../{AbstractLookup.java => LookupResult.java} | 23 ++-
.../log4j/core/lookup/PropertiesLookup.java | 126 +++++++++++++---
.../log4j/core/lookup/RuntimeStrSubstitutor.java | 11 --
.../logging/log4j/core/lookup/StrLookup.java | 22 +++
.../logging/log4j/core/lookup/StrSubstitutor.java | 20 +--
.../appender/routing/RoutingAppender3350Test.java | 65 ++++++++
.../PropertiesPluginTest.java} | 23 ++-
.../log4j/core/lookup/PropertiesLookupTest.java | 70 ++++++++-
.../log4j/core/lookup/StrSubstitutorTest.java | 166 ++++++++++++++++-----
.../src/test/resources/log4j-routing-2767.xml | 6 +-
.../src/test/resources/log4j-routing-purge.xml | 8 +-
log4j-core/src/test/resources/log4j-routing.json | 5 +-
.../src/test/resources/log4j-routing.properties | 4 +-
log4j-core/src/test/resources/log4j-routing.xml | 5 +-
log4j-core/src/test/resources/log4j-routing2.json | 5 +-
...og4j-routing-2767.xml => log4j-routing3350.xml} | 22 +--
src/changes/changes.xml | 3 +
29 files changed, 652 insertions(+), 171 deletions(-)
diff --git a/log4j-core/revapi.json b/log4j-core/revapi.json
index a4ff02a..10e586d 100644
--- a/log4j-core/revapi.json
+++ b/log4j-core/revapi.json
@@ -49,6 +49,33 @@
"old": "method void
org.apache.logging.log4j.core.script.ScriptManager::addScript(org.apache.logging.log4j.core.script.AbstractScript)",
"new": "method boolean
org.apache.logging.log4j.core.script.ScriptManager::addScript(org.apache.logging.log4j.core.script.AbstractScript)",
"justification": "LOG4J2-2486 - Require enabled script languages to be
specified in a system property"
+ },
+ {
+ "code": "java.method.returnTypeChanged",
+ "old": "method java.lang.String
org.apache.logging.log4j.core.lookup.StrSubstitutor::resolveVariable(org.apache.logging.log4j.core.LogEvent,
java.lang.String, java.lang.StringBuilder, int, int)",
+ "new": "method org.apache.logging.log4j.core.lookup.LookupResult
org.apache.logging.log4j.core.lookup.StrSubstitutor::resolveVariable(org.apache.logging.log4j.core.LogEvent,
java.lang.String, java.lang.StringBuilder, int, int)",
+ "justification": "LOG4J2-3317: Fix RoutingAppender backcompat while
improving lookup security"
+ },
+ {
+ "code": "java.annotation.removed",
+ "old": "parameter org.apache.logging.log4j.core.config.Property
org.apache.logging.log4j.core.config.Property::createProperty(===java.lang.String===,
java.lang.String)",
+ "new": "parameter org.apache.logging.log4j.core.config.Property
org.apache.logging.log4j.core.config.Property::createProperty(===java.lang.String===,
java.lang.String)",
+ "annotation":
"@org.apache.logging.log4j.core.config.plugins.PluginAttribute(\"name\")",
+ "justification": "LOG4J2-3317: Fix RoutingAppender backcompat while
improving lookup security"
+ },
+ {
+ "code": "java.annotation.removed",
+ "old": "parameter org.apache.logging.log4j.core.config.Property
org.apache.logging.log4j.core.config.Property::createProperty(java.lang.String,
===java.lang.String===)",
+ "new": "parameter org.apache.logging.log4j.core.config.Property
org.apache.logging.log4j.core.config.Property::createProperty(java.lang.String,
===java.lang.String===)",
+ "annotation":
"@org.apache.logging.log4j.core.config.plugins.PluginValue(\"value\")",
+ "justification": "LOG4J2-3317: Fix RoutingAppender backcompat while
improving lookup security"
+ },
+ {
+ "code": "java.annotation.removed",
+ "old": "method org.apache.logging.log4j.core.config.Property
org.apache.logging.log4j.core.config.Property::createProperty(java.lang.String,
java.lang.String)",
+ "new": "method org.apache.logging.log4j.core.config.Property
org.apache.logging.log4j.core.config.Property::createProperty(java.lang.String,
java.lang.String)",
+ "annotation":
"@org.apache.logging.log4j.core.config.plugins.PluginFactory",
+ "justification": "LOG4J2-3317: Fix RoutingAppender backcompat while
improving lookup security"
}
]
}
diff --git
a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/HttpURLConnectionManager.java
b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/HttpURLConnectionManager.java
index 55ccfdc..7d702f0 100644
---
a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/HttpURLConnectionManager.java
+++
b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/HttpURLConnectionManager.java
@@ -92,7 +92,7 @@ public class HttpURLConnectionManager extends HttpManager {
for (final Property header : headers) {
urlConnection.setRequestProperty(
header.getName(),
- header.isValueNeedsLookup() ?
getConfiguration().getStrSubstitutor().replace(event, header.getValue()) :
header.getValue());
+ header.evaluate(getConfiguration().getStrSubstitutor()));
}
if (sslConfiguration != null) {
((HttpsURLConnection)urlConnection).setSSLSocketFactory(sslConfiguration.getSslSocketFactory());
diff --git
a/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLogger.java
b/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLogger.java
index 9d48c8a..f7c4173 100644
---
a/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLogger.java
+++
b/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLogger.java
@@ -496,9 +496,7 @@ public class AsyncLogger extends Logger implements
EventTranslatorVararg<RingBuf
if (contextData.getValue(prop.getName()) != null) {
continue; // contextMap overrides config properties
}
- final String value = prop.isValueNeedsLookup() //
- ? privateConfig.config.getStrSubstitutor().replace(event,
prop.getValue()) //
- : prop.getValue();
+ final String value =
prop.evaluate(privateConfig.config.getStrSubstitutor());
contextData.putValue(prop.getName(), value);
}
event.setContextData(contextData);
diff --git
a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/LoggerConfig.java
b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/LoggerConfig.java
index a3bf60a..d7d7ce6 100644
---
a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/LoggerConfig.java
+++
b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/LoggerConfig.java
@@ -454,10 +454,8 @@ public class LoggerConfig extends AbstractFilterable
implements LocationAware {
.build();
for (int i = 0; i < props.size(); i++) {
final Property prop = props.get(i);
- final String value = prop.isValueNeedsLookup() // since LOG4J2-1575
- ? config.getStrSubstitutor().replace(event,
prop.getValue()) //
- : prop.getValue();
- results.add(Property.createProperty(prop.getName(), value));
+ final String value = prop.evaluate(config.getStrSubstitutor()); //
since LOG4J2-1575
+ results.add(Property.createProperty(prop.getName(),
prop.getRawValue(), value));
}
return results;
}
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 7c0d770..51fcd69 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
@@ -16,16 +16,16 @@
*/
package org.apache.logging.log4j.core.config;
-import java.util.HashMap;
-import java.util.Map;
-
+import org.apache.logging.log4j.core.LogEvent;
import org.apache.logging.log4j.core.config.plugins.Plugin;
import org.apache.logging.log4j.core.config.plugins.PluginConfiguration;
import org.apache.logging.log4j.core.config.plugins.PluginElement;
import org.apache.logging.log4j.core.config.plugins.PluginFactory;
import org.apache.logging.log4j.core.lookup.Interpolator;
+import org.apache.logging.log4j.core.lookup.LookupResult;
import org.apache.logging.log4j.core.lookup.PropertiesLookup;
import org.apache.logging.log4j.core.lookup.StrLookup;
+import org.apache.logging.log4j.core.lookup.StrSubstitutor;
/**
* Handles properties defined in the configuration.
@@ -33,6 +33,8 @@ import org.apache.logging.log4j.core.lookup.StrLookup;
@Plugin(name = "properties", category = Node.CATEGORY, printObject = true)
public final class PropertiesPlugin {
+ private static final StrSubstitutor UNESCAPING_SUBSTITUTOR =
createUnescapingSubstitutor();
+
private PropertiesPlugin() {
}
@@ -45,15 +47,62 @@ public final class PropertiesPlugin {
@PluginFactory
public static StrLookup configureSubstitutor(@PluginElement("Properties")
final Property[] properties,
@PluginConfiguration final
Configuration config) {
- if (properties == null) {
- return new Interpolator(config.getProperties());
+ // For backwards compatibility, we unescape all escaped lookups when
properties are parsed.
+ // This matches previous behavior for escaped components which were
meant to be executed later on.
+ Property[] unescapedProperties = new Property[properties == null ? 0 :
properties.length];
+ for (int i = 0; i < unescapedProperties.length; i++) {
+ unescapedProperties[i] = unescape(properties[i]);
}
- final Map<String, String> map = new HashMap<>(config.getProperties());
+ return new Interpolator(
+ new PropertiesLookup(unescapedProperties,
config.getProperties()),
+ config.getPluginPackages());
+ }
+
+ private static Property unescape(Property input) {
+ return Property.createProperty(
+ input.getName(),
+ unescape(input.getRawValue()),
+ input.getValue());
+ }
- for (final Property prop : properties) {
- map.put(prop.getName(), prop.getValue());
+ // Visible for testing
+ static String unescape(String input) {
+ return UNESCAPING_SUBSTITUTOR.replace(input);
+ }
+
+ /**
+ * Creates a new {@link StrSubstitutor} which is configured with no
lookups and does not handle
+ * defaults. This allows it to unescape one level of escaped lookups
without any further processing
+ * or removing replacing {@code ${ctx:foo:-default}} with {@code default}.
+ */
+ private static StrSubstitutor createUnescapingSubstitutor() {
+ StrSubstitutor substitutor = new StrSubstitutor(NullLookup.INSTANCE);
+ substitutor.setValueDelimiter(null);
+ substitutor.setValueDelimiterMatcher(null);
+ return substitutor;
+ }
+
+ private enum NullLookup implements StrLookup {
+ INSTANCE;
+
+ @Override
+ public String lookup(String key) {
+ return null;
+ }
+
+ @Override
+ public String lookup(LogEvent event, String key) {
+ return null;
+ }
+
+ @Override
+ public LookupResult evaluate(String key) {
+ return null;
}
- return new Interpolator(new PropertiesLookup(map),
config.getPluginPackages());
+ @Override
+ public LookupResult evaluate(LogEvent event, String key) {
+ return null;
+ }
}
}
diff --git
a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/Property.java
b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/Property.java
index 076ae44..fe320ef 100644
---
a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/Property.java
+++
b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/Property.java
@@ -23,6 +23,7 @@ import org.apache.logging.log4j.core.config.plugins.Plugin;
import org.apache.logging.log4j.core.config.plugins.PluginAttribute;
import org.apache.logging.log4j.core.config.plugins.PluginFactory;
import org.apache.logging.log4j.core.config.plugins.PluginValue;
+import org.apache.logging.log4j.core.lookup.StrSubstitutor;
import org.apache.logging.log4j.status.StatusLogger;
import org.apache.logging.log4j.util.Strings;
@@ -40,11 +41,13 @@ public final class Property {
private static final Logger LOGGER = StatusLogger.getLogger();
private final String name;
+ private final String rawValue;
private final String value;
private final boolean valueNeedsLookup;
- private Property(final String name, final String value) {
+ private Property(final String name, final String rawValue, final String
value) {
this.name = name;
+ this.rawValue = rawValue;
this.value = value;
this.valueNeedsLookup = value != null && value.contains("${");
}
@@ -58,6 +61,14 @@ public final class Property {
}
/**
+ * Returns the original raw property value without substitution.
+ * @return the raw value of the property, or empty string if it is not set.
+ */
+ public String getRawValue() {
+ return Objects.toString(rawValue, Strings.EMPTY);
+ }
+
+ /**
* Returns the property value.
* @return the value of the property.
*/
@@ -74,20 +85,48 @@ public final class Property {
}
/**
+ * Evaluate this property with the provided substitutor. If {@link
#isValueNeedsLookup()} is {@code false},
+ * the {@link #getValue() value} is returned, otherwise the {@link
#getRawValue() raw value} is evaluated
+ * with the given substitutor.
+ */
+ public String evaluate(StrSubstitutor substitutor) {
+ return valueNeedsLookup
+ // Unescape the raw value first, handling '$${ctx:foo}' ->
'${ctx:foo}'
+ ? substitutor.replace(PropertiesPlugin.unescape(getRawValue()))
+ : getValue();
+ }
+
+ /**
+ * Creates a Property.
+ *
+ * @param name The key.
+ * @param value The value.
+ * @return A Property.
+ */
+ public static Property createProperty(final String name, final String
value) {
+ if (name == null) {
+ LOGGER.error("Property name cannot be null");
+ }
+ return new Property(name, value, value);
+ }
+
+ /**
* Creates a Property.
*
* @param name The key.
+ * @param rawValue The value without any substitution applied.
* @param value The value.
* @return A Property.
*/
@PluginFactory
public static Property createProperty(
@PluginAttribute("name") final String name,
+ @PluginValue(value = "value", substitute = false) final String
rawValue,
@PluginValue("value") final String value) {
if (name == null) {
LOGGER.error("Property name cannot be null");
}
- return new Property(name, value);
+ return new Property(name, rawValue, value);
}
@Override
diff --git
a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/PluginValue.java
b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/PluginValue.java
index 9c20cc2..56d664f 100644
---
a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/PluginValue.java
+++
b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/PluginValue.java
@@ -37,4 +37,7 @@ import
org.apache.logging.log4j.core.config.plugins.visitors.PluginValueVisitor;
public @interface PluginValue {
String value();
+
+ /** If false, standard configuration value substitution is not done on the
referenced value. */
+ boolean substitute() default true;
}
diff --git
a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginValueVisitor.java
b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginValueVisitor.java
index d2a49d5..fe848d8 100644
---
a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginValueVisitor.java
+++
b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginValueVisitor.java
@@ -49,7 +49,9 @@ public class PluginValueVisitor extends
AbstractPluginVisitor<PluginValue> {
} else {
rawValue = removeAttributeValue(node.getAttributes(), name);
}
- final String value = this.substitutor.replace(event, rawValue);
+ final String value = this.annotation.substitute()
+ ? this.substitutor.replace(event, rawValue)
+ : rawValue;
StringBuilders.appendKeyDqValue(log, name, value);
return value;
}
diff --git
a/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/AbstractLookup.java
b/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/AbstractLookup.java
index c76561c..500116b 100644
---
a/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/AbstractLookup.java
+++
b/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/AbstractLookup.java
@@ -24,7 +24,7 @@ package org.apache.logging.log4j.core.lookup;
public abstract class AbstractLookup implements StrLookup {
/**
- * Calls {@code lookup(null, key)} in the super class.
+ * Calls {@code lookup(null, key)} in the implementation.
*
* @see StrLookup#lookup(LogEvent, String)
*/
@@ -33,4 +33,14 @@ public abstract class AbstractLookup implements StrLookup {
return lookup(null, key);
}
+ /**
+ * Calls {@code evaluate(null, key)} in the implementation.
+ *
+ * @see StrLookup#evaluate(LogEvent, String)
+ */
+ @Override
+ public LookupResult evaluate(final String key) {
+ return evaluate(null, key);
+ }
+
}
diff --git
a/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/ConfigurationStrSubstitutor.java
b/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/ConfigurationStrSubstitutor.java
index 1287721..6c4d20a 100644
---
a/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/ConfigurationStrSubstitutor.java
+++
b/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/ConfigurationStrSubstitutor.java
@@ -46,17 +46,6 @@ public final class ConfigurationStrSubstitutor extends
StrSubstitutor {
}
@Override
- boolean isRecursiveEvaluationAllowed() {
- return true;
- }
-
- @Override
- void setRecursiveEvaluationAllowed(final boolean
recursiveEvaluationAllowed) {
- throw new UnsupportedOperationException(
- "recursiveEvaluationAllowed cannot be modified within
ConfigurationStrSubstitutor");
- }
-
- @Override
public String toString() {
return "ConfigurationStrSubstitutor{" + super.toString() + "}";
}
diff --git
a/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/AbstractLookup.java
b/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/DefaultLookupResult.java
similarity index 60%
copy from
log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/AbstractLookup.java
copy to
log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/DefaultLookupResult.java
index c76561c..580c538 100644
---
a/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/AbstractLookup.java
+++
b/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/DefaultLookupResult.java
@@ -14,23 +14,32 @@
* See the license for the specific language governing permissions and
* limitations under the license.
*/
+
package org.apache.logging.log4j.core.lookup;
-/**
- * A default lookup for others to extend.
- *
- * @since 2.1
- */
-public abstract class AbstractLookup implements StrLookup {
+import java.util.Objects;
+
+/** Default internal implementation of {@link LookupResult}. */
+final class DefaultLookupResult implements LookupResult {
+
+ private final String value;
+
+ DefaultLookupResult(String value) {
+ this.value = Objects.requireNonNull(value, "value is required");
+ }
+
+ @Override
+ public String value() {
+ return value;
+ }
- /**
- * Calls {@code lookup(null, key)} in the super class.
- *
- * @see StrLookup#lookup(LogEvent, String)
- */
@Override
- public String lookup(final String key) {
- return lookup(null, key);
+ public boolean isLookupEvaluationAllowedInValue() {
+ return false;
}
+ @Override
+ public String toString() {
+ return "DefaultLookupResult{value='" + value + "'}";
+ }
}
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 86a742e..44c72b7 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
@@ -154,6 +154,25 @@ public class Interpolator extends
AbstractConfigurationAwareLookup {
*/
@Override
public String lookup(final LogEvent event, String var) {
+ LookupResult result = evaluate(event, var);
+ return result == null ? null : result.value();
+ }
+
+ /**
+ * Resolves the specified variable. This implementation will try to extract
+ * a variable prefix from the given variable name (the first colon (':') is
+ * used as prefix separator). It then passes the name of the variable with
+ * the prefix stripped to the lookup object registered for this prefix. If
+ * no prefix can be found or if the associated lookup object cannot resolve
+ * this variable, the default lookup object will be used.
+ *
+ * @param event The current LogEvent or null.
+ * @param var the name of the variable whose value is to be looked up
+ * @return the value of this variable or <b>null</b> if it cannot be
+ * resolved
+ */
+ @Override
+ public LookupResult evaluate(final LogEvent event, String var) {
if (var == null) {
return null;
}
@@ -166,9 +185,9 @@ public class Interpolator extends
AbstractConfigurationAwareLookup {
if (lookup instanceof ConfigurationAware) {
((ConfigurationAware) lookup).setConfiguration(configuration);
}
- String value = null;
+ LookupResult value = null;
if (lookup != null) {
- value = event == null ? lookup.lookup(name) :
lookup.lookup(event, name);
+ value = event == null ? lookup.evaluate(name) :
lookup.evaluate(event, name);
}
if (value != null) {
@@ -177,7 +196,7 @@ public class Interpolator extends
AbstractConfigurationAwareLookup {
var = var.substring(prefixPos + 1);
}
if (defaultLookup != null) {
- return event == null ? defaultLookup.lookup(var) :
defaultLookup.lookup(event, var);
+ return event == null ? defaultLookup.evaluate(var) :
defaultLookup.evaluate(event, var);
}
return null;
}
diff --git
a/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/AbstractLookup.java
b/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/LookupResult.java
similarity index 59%
copy from
log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/AbstractLookup.java
copy to
log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/LookupResult.java
index c76561c..caa82e4 100644
---
a/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/AbstractLookup.java
+++
b/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/LookupResult.java
@@ -14,23 +14,22 @@
* See the license for the specific language governing permissions and
* limitations under the license.
*/
+
package org.apache.logging.log4j.core.lookup;
-/**
- * A default lookup for others to extend.
- *
- * @since 2.1
- */
-public abstract class AbstractLookup implements StrLookup {
+public interface LookupResult {
+
+ /** Value of the lookup result. Never null. */
+ String value();
/**
- * Calls {@code lookup(null, key)} in the super class.
- *
- * @see StrLookup#lookup(LogEvent, String)
+ * True if the {@link #value()} should be re-evaluated for other lookups.
+ * This is used by {@link PropertiesLookup} to allow properties to be
evaluated against other properties,
+ * because the configuration properties are completely trusted and
designed with lookups in mind. It is
+ * unsafe to return true in most cases because it may allow unintended
lookups to evaluate other lookups.
*/
- @Override
- public String lookup(final String key) {
- return lookup(null, key);
+ default boolean isLookupEvaluationAllowedInValue() {
+ return false;
}
}
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
index 3066792..a17846b 100644
---
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
@@ -17,9 +17,12 @@
package org.apache.logging.log4j.core.lookup;
import org.apache.logging.log4j.core.LogEvent;
+import org.apache.logging.log4j.core.config.Property;
import java.util.Collections;
+import java.util.HashMap;
import java.util.Map;
+import java.util.Objects;
/**
* A lookup designed for {@code Properties} defined in the configuration. This
is similar
@@ -29,29 +32,28 @@ import java.util.Map;
*/
public final class PropertiesLookup implements StrLookup {
- /**
- * Configuration from which to read properties.
- */
- private final Map<String, String> properties;
+ /** Logger context properties. */
+ private final Map<String, String> contextProperties;
- /**
- * Constructs a new instance for the given map.
- *
- * @param properties map these.
- */
- public PropertiesLookup(final Map<String, String> properties) {
- this.properties = properties == null
+ /** Configuration properties. */
+ private final Map<String, ConfigurationPropertyResult>
configurationProperties;
+
+ public PropertiesLookup(final Property[] configProperties, final
Map<String, String> contextProperties) {
+ this.contextProperties = contextProperties == null
? Collections.emptyMap()
- : properties;
+ : contextProperties;
+ this.configurationProperties = configProperties == null
+ ? Collections.emptyMap()
+ : createConfigurationPropertyMap(configProperties);
}
/**
- * Gets the property map.
+ * Constructs a new instance for the given map.
*
- * @return the property map.
+ * @param properties map these.
*/
- public Map<String, String> getProperties() {
- return properties;
+ public PropertiesLookup(final Map<String, String> properties) {
+ this(Property.EMPTY_ARRAY, properties);
}
@Override
@@ -70,12 +72,100 @@ public final class PropertiesLookup implements StrLookup {
*/
@Override
public String lookup(final String key) {
- return key == null ? null : properties.get(key);
+ LookupResult result = evaluate(key);
+ return result == null ? null : result.value();
+ }
+
+ @Override
+ public LookupResult evaluate(String key) {
+ if (key == null) {
+ return null;
+ }
+ LookupResult configResult = configurationProperties.get(key);
+ if (configResult != null) {
+ return configResult;
+ }
+ // Allow the context map to be mutated after this lookup has been
initialized.
+ String contextResult = contextProperties.get(key);
+ return contextResult == null ? null : new
ContextPropertyResult(contextResult);
+ }
+
+ @Override
+ public LookupResult evaluate(@SuppressWarnings("ignored") final LogEvent
event, final String key) {
+ return evaluate(key);
}
@Override
public String toString() {
- return "PropertiesLookup{properties=" + properties + '}';
+ return "PropertiesLookup{" +
+ "contextProperties=" + contextProperties +
+ ", configurationProperties=" + configurationProperties +
+ '}';
}
+ private static Map<String, ConfigurationPropertyResult>
createConfigurationPropertyMap(Property[] props) {
+ // The raw property values must be used without the substitution
handled by the plugin framework
+ // which calls this method, otherwise we risk re-interpolating through
unexpected data.
+ // The PropertiesLookup is unique in that results from this lookup
support recursive evaluation.
+ Map<String, ConfigurationPropertyResult> result = new
HashMap<>(props.length);
+ for (Property property : props) {
+ result.put(property.getName(), new
ConfigurationPropertyResult(property.getRawValue()));
+ }
+ return result;
+ }
+
+ private static final class ConfigurationPropertyResult implements
LookupResult {
+
+ private final String value;
+
+ ConfigurationPropertyResult(String value) {
+ this.value = Objects.requireNonNull(value, "value is required");
+ }
+
+ @Override
+ public String value() {
+ return value;
+ }
+
+ /**
+ * Properties are a special case in which lookups contained
+ * within the properties map are allowed for recursive evaluation.
+ */
+ @Override
+ public boolean isLookupEvaluationAllowedInValue() {
+ return true;
+ }
+
+ @Override
+ public String toString() {
+ return "ConfigurationPropertyResult{'" + value + "'}";
+ }
+ }
+
+ private static final class ContextPropertyResult implements LookupResult {
+
+ private final String value;
+
+ ContextPropertyResult(String value) {
+ this.value = Objects.requireNonNull(value, "value is required");
+ }
+
+ @Override
+ public String value() {
+ return value;
+ }
+
+ /**
+ * Unlike configuration properties, context properties are not built
around lookup syntax.
+ */
+ @Override
+ public boolean isLookupEvaluationAllowedInValue() {
+ return false;
+ }
+
+ @Override
+ public String toString() {
+ return "ContextPropertyResult{'" + value + "'}";
+ }
+ }
}
diff --git
a/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/RuntimeStrSubstitutor.java
b/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/RuntimeStrSubstitutor.java
index f002c6e..6000113 100644
---
a/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/RuntimeStrSubstitutor.java
+++
b/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/RuntimeStrSubstitutor.java
@@ -44,17 +44,6 @@ public final class RuntimeStrSubstitutor extends
StrSubstitutor {
}
@Override
- boolean isRecursiveEvaluationAllowed() {
- return false;
- }
-
- @Override
- void setRecursiveEvaluationAllowed(final boolean
recursiveEvaluationAllowed) {
- throw new UnsupportedOperationException(
- "recursiveEvaluationAllowed cannot be modified within
RuntimeStrSubstitutor");
- }
-
- @Override
public String toString() {
return "RuntimeStrSubstitutor{" + super.toString() + "}";
}
diff --git
a/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/StrLookup.java
b/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/StrLookup.java
index e29d280..86daa46 100644
---
a/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/StrLookup.java
+++
b/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/StrLookup.java
@@ -99,4 +99,26 @@ public interface StrLookup {
* @return the matching value, null if no match
*/
String lookup(LogEvent event, String key);
+
+ /**
+ * Same as {@link #lookup(String)}, but provides additional metadata
describing the result.
+ * Returns null if the key cannot be evaluated, otherwise a {@link
LookupResult} wrapping the non-null string value.
+ */
+ default LookupResult evaluate(String key) {
+ String value = lookup(key);
+ return value == null
+ ? null
+ : new DefaultLookupResult(value);
+ }
+
+ /**
+ * Same as {@link #lookup(LogEvent, String)}, but provides additional
metadata describing the result.
+ * Returns null if the key cannot be evaluated, otherwise a {@link
LookupResult} wrapping the non-null string value.
+ */
+ default LookupResult evaluate(LogEvent event, String key) {
+ String value = lookup(event, key);
+ return value == null
+ ? null
+ : new DefaultLookupResult(value);
+ }
}
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 2f7c95c..7e78d04 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
@@ -209,8 +209,6 @@ public class StrSubstitutor implements ConfigurationAware {
*/
private Configuration configuration;
- private boolean recursiveEvaluationAllowed;
-
//-----------------------------------------------------------------------
/**
* Creates a new instance with defaults for variable prefix and suffix
@@ -392,7 +390,6 @@ public class StrSubstitutor implements ConfigurationAware {
this.setValueDelimiterMatcher(other.valueDelimiterMatcher);
this.valueEscapeDelimiterMatcher = other.valueEscapeDelimiterMatcher;
this.configuration = other.configuration;
- this.recursiveEvaluationAllowed = other.isRecursiveEvaluationAllowed();
this.enableSubstitutionInVariables =
other.isEnableSubstitutionInVariables();
this.valueDelimiterString = other.valueDelimiterString;
}
@@ -1098,7 +1095,8 @@ public class StrSubstitutor implements ConfigurationAware
{
boolean isCyclic = isCyclicSubstitution(varName,
priorVariables);
// resolve the variable
- String varValue = isCyclic ? null :
resolveVariable(event, varName, buf, startPos, endPos);
+ LookupResult resolvedResult = isCyclic ? null :
resolveVariable(event, varName, buf, startPos, endPos);
+ String varValue = resolvedResult == null ? null :
resolvedResult.value();
if (varValue == null) {
varValue = varDefaultValue;
}
@@ -1107,7 +1105,7 @@ public class StrSubstitutor implements ConfigurationAware
{
final int varLen = varValue.length();
buf.replace(startPos, endPos, varValue);
altered = true;
- int change = isRecursiveEvaluationAllowed()
+ int change = resolvedResult != null &&
resolvedResult.isLookupEvaluationAllowedInValue()
? substitute(event, buf, startPos,
varLen, priorVariables)
: 0;
change = change + (varLen - (endPos -
startPos));
@@ -1175,14 +1173,14 @@ public class StrSubstitutor implements
ConfigurationAware {
* @param endPos the end position of the variable including the suffix,
valid
* @return the variable's value or <b>null</b> if the variable is unknown
*/
- protected String resolveVariable(final LogEvent event, final String
variableName, final StringBuilder buf,
+ protected LookupResult resolveVariable(final LogEvent event, final String
variableName, final StringBuilder buf,
final int startPos, final int endPos) {
final StrLookup resolver = getVariableResolver();
if (resolver == null) {
return null;
}
try {
- return resolver.lookup(event, variableName);
+ return resolver.evaluate(event, variableName);
} catch (Throwable t) {
StatusLogger.getLogger().error("Resolver failed to lookup {}",
variableName, t);
return null;
@@ -1475,14 +1473,6 @@ public class StrSubstitutor implements
ConfigurationAware {
this.enableSubstitutionInVariables = enableSubstitutionInVariables;
}
- boolean isRecursiveEvaluationAllowed() {
- return recursiveEvaluationAllowed;
- }
-
- void setRecursiveEvaluationAllowed(final boolean
recursiveEvaluationAllowed) {
- this.recursiveEvaluationAllowed = recursiveEvaluationAllowed;
- }
-
private char[] getChars(final StringBuilder sb) {
final char[] chars = new char[sb.length()];
sb.getChars(0, sb.length(), chars, 0);
diff --git
a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/routing/RoutingAppender3350Test.java
b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/routing/RoutingAppender3350Test.java
new file mode 100644
index 0000000..55f382b
--- /dev/null
+++
b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/routing/RoutingAppender3350Test.java
@@ -0,0 +1,65 @@
+/*
+ * 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.appender.routing;
+
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.junit.LoggerContextRule;
+import org.apache.logging.log4j.message.StringMapMessage;
+import org.junit.After;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.RuleChain;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.nio.charset.StandardCharsets;
+
+import static org.junit.Assert.assertEquals;
+
+public class RoutingAppender3350Test {
+ private static final String CONFIG = "log4j-routing3350.xml";
+ private static final String LOG_FILE = "target/tmp/test.log";
+
+ private final LoggerContextRule loggerContextRule = new
LoggerContextRule(CONFIG);
+
+ @Rule
+ public RuleChain rules = loggerContextRule.withCleanFilesRule(LOG_FILE);
+
+ @After
+ public void tearDown() throws Exception {
+ this.loggerContextRule.getLoggerContext().stop();
+ }
+
+ @Test
+ public void routingTest() throws IOException {
+ String expected = "expectedValue";
+ StringMapMessage message = new StringMapMessage().with("data",
expected);
+ Logger logger =
loggerContextRule.getLoggerContext().getLogger(getClass());
+ logger.error(message);
+ File file = new File(LOG_FILE);
+ try (InputStream inputStream = new FileInputStream(file);
+ InputStreamReader streamReader = new
InputStreamReader(inputStream, StandardCharsets.UTF_8);
+ BufferedReader reader = new BufferedReader(streamReader)) {
+ String actual = reader.readLine();
+ assertEquals(expected, actual);
+ }
+ }
+}
diff --git
a/log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/PropertiesLookupTest.java
b/log4j-core/src/test/java/org/apache/logging/log4j/core/config/PropertiesPluginTest.java
similarity index 67%
copy from
log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/PropertiesLookupTest.java
copy to
log4j-core/src/test/java/org/apache/logging/log4j/core/config/PropertiesPluginTest.java
index 2c708f0..7f35ef8 100644
---
a/log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/PropertiesLookupTest.java
+++
b/log4j-core/src/test/java/org/apache/logging/log4j/core/config/PropertiesPluginTest.java
@@ -15,23 +15,22 @@
* limitations under the license.
*/
-package org.apache.logging.log4j.core.lookup;
+package org.apache.logging.log4j.core.config;
-import static org.junit.Assert.assertEquals;
+import org.junit.jupiter.api.Test;
-import java.util.HashMap;
+import static org.junit.jupiter.api.Assertions.assertEquals;
-import org.junit.jupiter.api.Test;
+public class PropertiesPluginTest {
-/**
- * Tests {@link PropertiesLookup}.
- */
-public class PropertiesLookupTest {
+ @Test
+ public void testUnescape() {
+ assertEquals("${foo}", PropertiesPlugin.unescape("$${foo}"));
+ }
@Test
- public void testGetProperties() {
- final HashMap<String, String> properties = new HashMap<>();
- properties.put("A", "1");
- assertEquals(properties, new
PropertiesLookup(properties).getProperties());
+ public void testUnescapeNotEscapedWithDefault() {
+ String value = "${foo:-bar}";
+ assertEquals(value, PropertiesPlugin.unescape(value));
}
}
diff --git
a/log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/PropertiesLookupTest.java
b/log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/PropertiesLookupTest.java
index 2c708f0..1e34c5a 100644
---
a/log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/PropertiesLookupTest.java
+++
b/log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/PropertiesLookupTest.java
@@ -17,10 +17,16 @@
package org.apache.logging.log4j.core.lookup;
-import static org.junit.Assert.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import java.util.Collections;
import java.util.HashMap;
+import java.util.Map;
+import org.apache.logging.log4j.core.config.Property;
import org.junit.jupiter.api.Test;
/**
@@ -29,9 +35,63 @@ import org.junit.jupiter.api.Test;
public class PropertiesLookupTest {
@Test
- public void testGetProperties() {
- final HashMap<String, String> properties = new HashMap<>();
- properties.put("A", "1");
- assertEquals(properties, new
PropertiesLookup(properties).getProperties());
+ public void testLookupContextProperty() {
+ final StrLookup propertiesLookup = new PropertiesLookup(
+ Property.EMPTY_ARRAY, Collections.singletonMap("A", "1"));
+ assertEquals("1", propertiesLookup.lookup("A"));
+ final LookupResult lookupResult = propertiesLookup.evaluate("A");
+ assertEquals("1", lookupResult.value());
+ assertFalse(lookupResult.isLookupEvaluationAllowedInValue());
+ }
+
+ @Test
+ public void testLookupConfigProperty() {
+ final StrLookup propertiesLookup = new PropertiesLookup(
+ new Property[] {Property.createProperty("A", "1")},
+ Collections.emptyMap());
+ assertEquals("1", propertiesLookup.lookup("A"));
+ final LookupResult lookupResult = propertiesLookup.evaluate("A");
+ assertEquals("1", lookupResult.value());
+ assertTrue(lookupResult.isLookupEvaluationAllowedInValue());
+ }
+
+ @Test
+ public void testConfigPropertiesPreferredOverContextProperties() {
+ final StrLookup propertiesLookup = new PropertiesLookup(
+ new Property[] {Property.createProperty("A", "1")},
+ Collections.singletonMap("A", "2"));
+ assertEquals("1", propertiesLookup.lookup("A"));
+ final LookupResult lookupResult = propertiesLookup.evaluate("A");
+ assertEquals("1", lookupResult.value());
+ assertTrue(lookupResult.isLookupEvaluationAllowedInValue());
+ }
+
+ @Test
+ public void testEvaluateResultsSupportRecursiveEvaluation() {
+ PropertiesLookup lookup = new
PropertiesLookup(Collections.singletonMap("key", "value"));
+ assertFalse(lookup.evaluate("key").isLookupEvaluationAllowedInValue());
+ }
+
+ @Test
+ public void testEvaluateReturnsNullWhenKeyIsNotFound() {
+ PropertiesLookup lookup = new PropertiesLookup(Collections.emptyMap());
+ assertNull(lookup.evaluate("key"));
+ }
+
+ @Test
+ public void testEvaluateReturnsNullWhenKeyIsNull() {
+ PropertiesLookup lookup = new PropertiesLookup(Collections.emptyMap());
+ assertNull(lookup.evaluate(null));
+ }
+
+ @Test
+ public void testContextPropertiesAreMutable() {
+ Map<String, String> contextProperties = new HashMap<>();
+ PropertiesLookup lookup = new PropertiesLookup(Property.EMPTY_ARRAY,
contextProperties);
+ assertNull(lookup.evaluate("key"));
+ contextProperties.put("key", "value");
+ LookupResult result = lookup.evaluate("key");
+ assertEquals("value", result.value());
+ assertFalse(result.isLookupEvaluationAllowedInValue());
}
}
diff --git
a/log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/StrSubstitutorTest.java
b/log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/StrSubstitutorTest.java
index efb492c..9397fb8 100644
---
a/log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/StrSubstitutorTest.java
+++
b/log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/StrSubstitutorTest.java
@@ -116,7 +116,7 @@ public class StrSubstitutorTest {
valuesMap.put("main:--file", "path/file.txt");
// Create substitutor, initially without support for escaping :-
final StrSubstitutor sub = new StrSubstitutor(
- new PropertiesLookup(valuesMap),
+ new RecursiveLookup(valuesMap),
StrSubstitutor.DEFAULT_PREFIX,
StrSubstitutor.DEFAULT_SUFFIX,
StrSubstitutor.DEFAULT_ESCAPE,
@@ -144,7 +144,7 @@ public class StrSubstitutorTest {
public void testDefault() {
final Map<String, String> map = new HashMap<>();
map.put(TESTKEY, TESTVAL);
- final StrLookup lookup = new Interpolator(new MapLookup(map));
+ final StrLookup lookup = new Interpolator(new NonRecursiveLookup(map));
final StrSubstitutor subst = new StrSubstitutor(lookup);
ThreadContext.put(TESTKEY, TESTVAL);
//String value = subst.replace("${sys:TestKey1:-${ctx:TestKey}}");
@@ -156,36 +156,33 @@ public class StrSubstitutorTest {
public void testDefaultReferencesLookupValue() {
final Map<String, String> map = new HashMap<>();
map.put(TESTKEY, "${java:version}");
- final StrLookup lookup = new Interpolator(new MapLookup(map));
+ final StrLookup lookup = new Interpolator(new NonRecursiveLookup(map));
final StrSubstitutor subst = new StrSubstitutor(lookup);
- subst.setRecursiveEvaluationAllowed(false);
final String value = subst.replace("${sys:TestKey1:-${ctx:TestKey}}");
assertEquals("${java:version}", value);
}
@Test
public void testInfiniteSubstitutionOnString() {
- final StrLookup lookup = new Interpolator(new MapLookup(new
HashMap<>()));
+ final StrLookup lookup = new Interpolator(new NonRecursiveLookup(new
HashMap<>()));
final StrSubstitutor subst = new StrSubstitutor(lookup);
- subst.setRecursiveEvaluationAllowed(true);
String infiniteSubstitution = "${${::-${::-$${::-j}}}}";
- assertEquals(infiniteSubstitution,
subst.replace(infiniteSubstitution));
+ assertEquals("j}", subst.replace(infiniteSubstitution));
}
@Test
public void testInfiniteSubstitutionOnStringBuilder() {
- final StrLookup lookup = new Interpolator(new MapLookup(new
HashMap<>()));
+ final StrLookup lookup = new Interpolator(new NonRecursiveLookup(new
HashMap<>()));
final StrSubstitutor subst = new StrSubstitutor(lookup);
- subst.setRecursiveEvaluationAllowed(true);
String infiniteSubstitution = "${${::-${::-$${::-j}}}}";
- assertEquals(infiniteSubstitution, subst.replace(null, new
StringBuilder(infiniteSubstitution)));
+ assertEquals("j}", subst.replace(null, new
StringBuilder(infiniteSubstitution)));
}
@Test
public void testLookup() {
final Map<String, String> map = new HashMap<>();
map.put(TESTKEY, TESTVAL);
- final StrLookup lookup = new Interpolator(new MapLookup(map));
+ final StrLookup lookup = new Interpolator(new NonRecursiveLookup(map));
final StrSubstitutor subst = new StrSubstitutor(lookup);
ThreadContext.put(TESTKEY, TESTVAL);
String value =
subst.replace("${TestKey}-${ctx:TestKey}-${sys:TestKey}");
@@ -205,9 +202,8 @@ public class StrSubstitutorTest {
public void testLookupsNestedWithoutRecursiveEvaluation() {
final Map<String, String> map = new HashMap<>();
map.put("first", "${java:version}");
- final StrLookup lookup = new Interpolator(new MapLookup(map));
+ final StrLookup lookup = new Interpolator(new NonRecursiveLookup(map));
final StrSubstitutor subst = new StrSubstitutor(lookup);
- subst.setRecursiveEvaluationAllowed(false);
assertEquals("${java:version}",
subst.replace("${${lower:C}t${lower:X}:first}"));
}
@@ -228,7 +224,6 @@ public class StrSubstitutorTest {
return "success";
}
}));
- subst.setRecursiveEvaluationAllowed(false);
assertEquals("success ${foo:throw} success", subst.replace("${foo:a}
${foo:throw} ${foo:c}"));
}
@@ -236,9 +231,8 @@ public class StrSubstitutorTest {
public void testNestedSelfReferenceWithRecursiveEvaluation() {
final Map<String, String> map = new HashMap<>();
map.put("first", "${${ctx:first}}");
- final StrLookup lookup = new Interpolator(new MapLookup(map));
+ final StrLookup lookup = new Interpolator(new RecursiveLookup(map));
final StrSubstitutor subst = new StrSubstitutor(lookup);
- subst.setRecursiveEvaluationAllowed(true);
assertEquals("${${ctx:first}}", subst.replace("${ctx:first}"));
}
@@ -247,9 +241,8 @@ public class StrSubstitutorTest {
final Map<String, String> map = new HashMap<>();
map.put("first", "${java:version}");
map.put("second", "${java:runtime}");
- final StrLookup lookup = new Interpolator(new MapLookup(map));
+ final StrLookup lookup = new Interpolator(new NonRecursiveLookup(map));
final StrSubstitutor subst = new StrSubstitutor(lookup);
- subst.setRecursiveEvaluationAllowed(false);
assertEquals("${java:version}",
subst.replace("${ctx:first:-${ctx:second}}"));
}
@@ -257,9 +250,8 @@ public class StrSubstitutorTest {
public void testNoRecursiveEvaluationWithDepthOne() {
final Map<String, String> map = new HashMap<>();
map.put("first", "${java:version}");
- final StrLookup lookup = new Interpolator(new MapLookup(map));
+ final StrLookup lookup = new Interpolator(new NonRecursiveLookup(map));
final StrSubstitutor subst = new StrSubstitutor(lookup);
- subst.setRecursiveEvaluationAllowed(false);
assertEquals("${java:version}", subst.replace("${ctx:first}"));
}
@@ -267,9 +259,8 @@ public class StrSubstitutorTest {
public void testRandomWithRecursiveDefault() {
final Map<String, String> map = new HashMap<>();
map.put("first", "${env:RANDOM:-${ctx:first}}");
- final StrLookup lookup = new Interpolator(new MapLookup(map));
+ final StrLookup lookup = new Interpolator(new RecursiveLookup(map));
final StrSubstitutor subst = new StrSubstitutor(lookup);
- subst.setRecursiveEvaluationAllowed(true);
assertEquals("${ctx:first}", subst.replace("${ctx:first}"));
}
@@ -278,9 +269,8 @@ public class StrSubstitutorTest {
final Map<String, String> map = new HashMap<>();
map.put("first", "${ctx:first}");
map.put("second", "secondValue");
- final StrLookup lookup = new Interpolator(new MapLookup(map));
+ final StrLookup lookup = new Interpolator(new RecursiveLookup(map));
final StrSubstitutor subst = new StrSubstitutor(lookup);
- subst.setRecursiveEvaluationAllowed(true);
assertEquals("${ctx:first} and secondValue",
subst.replace("${ctx:first} and ${ctx:second}"));
}
@@ -288,9 +278,8 @@ public class StrSubstitutorTest {
public void testRecursiveWithDefault() {
final Map<String, String> map = new HashMap<>();
map.put("first", "${ctx:first:-default}");
- final StrLookup lookup = new Interpolator(new MapLookup(map));
+ final StrLookup lookup = new Interpolator(new RecursiveLookup(map));
final StrSubstitutor subst = new StrSubstitutor(lookup);
- subst.setRecursiveEvaluationAllowed(true);
assertEquals("default", subst.replace("${ctx:first}"));
}
@@ -298,9 +287,8 @@ public class StrSubstitutorTest {
public void testRecursiveWithRecursiveDefault() {
final Map<String, String> map = new HashMap<>();
map.put("first", "${ctx:first:-${ctx:first}}");
- final StrLookup lookup = new Interpolator(new MapLookup(map));
+ final StrLookup lookup = new Interpolator(new RecursiveLookup(map));
final StrSubstitutor subst = new StrSubstitutor(lookup);
- subst.setRecursiveEvaluationAllowed(true);
assertEquals("${ctx:first}", subst.replace("${ctx:first}"));
}
@@ -318,9 +306,8 @@ public class StrSubstitutorTest {
public void testTopLevelLookupsWithoutRecursiveEvaluation() {
final Map<String, String> map = new HashMap<>();
map.put("key", "VaLuE");
- final StrLookup lookup = new Interpolator(new MapLookup(map));
+ final StrLookup lookup = new Interpolator(new NonRecursiveLookup(map));
final StrSubstitutor subst = new StrSubstitutor(lookup);
- subst.setRecursiveEvaluationAllowed(false);
assertEquals("value", subst.replace("${lower:${ctx:key}}"));
}
@@ -328,9 +315,8 @@ public class StrSubstitutorTest {
public void testTopLevelLookupsWithoutRecursiveEvaluation_doubleLower() {
final Map<String, String> map = new HashMap<>();
map.put("key", "VaLuE");
- final StrLookup lookup = new Interpolator(new MapLookup(map));
+ final StrLookup lookup = new Interpolator(new NonRecursiveLookup(map));
final StrSubstitutor subst = new StrSubstitutor(lookup);
- subst.setRecursiveEvaluationAllowed(false);
assertEquals("value", subst.replace("${lower:${lower:${ctx:key}}}"));
}
@@ -338,9 +324,121 @@ public class StrSubstitutorTest {
public void
testTopLevelLookupsWithoutRecursiveEvaluationAndDefaultValueLookup() {
final Map<String, String> map = new HashMap<>();
map.put("key2", "TWO");
- final StrLookup lookup = new Interpolator(new MapLookup(map));
+ final StrLookup lookup = new Interpolator(new NonRecursiveLookup(map));
final StrSubstitutor subst = new StrSubstitutor(lookup);
- subst.setRecursiveEvaluationAllowed(false);
assertEquals("two",
subst.replace("${lower:${ctx:key1:-${ctx:key2}}}"));
}
+
+ @Test
+ public void testNonRecursiveReferencesRecursive() {
+ final StrLookup lookup = new StrLookup() {
+ @Override
+ public String lookup(final String key) {
+ return "unexpected";
+ }
+
+ @Override
+ public String lookup(final LogEvent event, final String key) {
+ return "unexpected";
+ }
+
+ @Override
+ public LookupResult evaluate(final String key) {
+ return evaluate(null, key);
+ }
+
+ @Override
+ public LookupResult evaluate(final LogEvent event, final String
key) {
+ switch (key) {
+ case "first":
+ return new RecursiveLookupResult("${second}");
+ case "second":
+ return new NonRecursiveLookupResult("${third}");
+ default:
+ return new RecursiveLookupResult("should not be used:
" + key);
+ }
+ }
+ };
+ final StrSubstitutor subst = new StrSubstitutor(lookup);
+ // First (recursive) expands to second, which is not recursive, so the
literal '${third}' is used.
+ assertEquals("${third}", subst.replace("${first}"));
+ }
+
+ private static final class RecursiveLookup extends AbstractLookup {
+
+ private final Map<String, String> properties;
+
+ RecursiveLookup(Map<String, String> properties) {
+ this.properties = properties;
+ }
+
+ @Override
+ public String lookup(LogEvent event, String key) {
+ LookupResult result = evaluate(event, key);
+ return result == null ? null : result.value();
+ }
+
+ @Override
+ public LookupResult evaluate(LogEvent event, String key) {
+ String result = key == null ? null : properties.get(key);
+ return result == null ? null : new RecursiveLookupResult(result);
+ }
+ }
+
+ private static final class RecursiveLookupResult implements LookupResult {
+
+ private final String value;
+
+ RecursiveLookupResult(String value) {
+ this.value = value;
+ }
+ @Override
+ public String value() {
+ return value;
+ }
+
+ @Override
+ public boolean isLookupEvaluationAllowedInValue() {
+ return true;
+ }
+ }
+
+ private static final class NonRecursiveLookup extends AbstractLookup {
+
+ private final Map<String, String> properties;
+
+ NonRecursiveLookup(Map<String, String> properties) {
+ this.properties = properties;
+ }
+
+ @Override
+ public String lookup(LogEvent event, String key) {
+ LookupResult result = evaluate(event, key);
+ return result == null ? null : result.value();
+ }
+
+ @Override
+ public LookupResult evaluate(LogEvent event, String key) {
+ String result = key == null ? null : properties.get(key);
+ return result == null ? null : new
NonRecursiveLookupResult(result);
+ }
+ }
+
+ private static final class NonRecursiveLookupResult implements
LookupResult {
+
+ private final String value;
+
+ NonRecursiveLookupResult(String value) {
+ this.value = value;
+ }
+ @Override
+ public String value() {
+ return value;
+ }
+
+ @Override
+ public boolean isLookupEvaluationAllowedInValue() {
+ return false;
+ }
+ }
}
diff --git a/log4j-core/src/test/resources/log4j-routing-2767.xml
b/log4j-core/src/test/resources/log4j-routing-2767.xml
index 4002659..34b9c30 100644
--- a/log4j-core/src/test/resources/log4j-routing-2767.xml
+++ b/log4j-core/src/test/resources/log4j-routing-2767.xml
@@ -17,6 +17,10 @@
-->
<Configuration status="WARN" name="RoutingTest">
+ <Properties>
+ <Property
name="filename">target/routing1/routingtest-$${sd:type}.log</Property>
+ </Properties>
+
<Appenders>
<Console name="STDOUT">
<PatternLayout pattern="%m%n"/>
@@ -24,7 +28,7 @@
<Routing name="Routing">
<Routes>
<Route>
- <RollingFile name="Routing-${sd:type}"
fileName="target/routing1/routingtest-${sd:type}.log"
+ <RollingFile name="Routing-${sd:type}" fileName="${filename}"
filePattern="target/routing1/test1-${sd:type}.%i.log.gz">
<PatternLayout>
<Pattern>%d %p %C{1.} [%t] %m%n</Pattern>
diff --git a/log4j-core/src/test/resources/log4j-routing-purge.xml
b/log4j-core/src/test/resources/log4j-routing-purge.xml
index 910ba80..d1de288 100644
--- a/log4j-core/src/test/resources/log4j-routing-purge.xml
+++ b/log4j-core/src/test/resources/log4j-routing-purge.xml
@@ -17,6 +17,10 @@
-->
<Configuration status="OFF" name="RoutingTest">
+ <Properties>
+ <Property
name="filename-idle">target/routing-purge-idle/routingtest-$${sd:id}.log</Property>
+ <Property
name="filename-manual">target/routing-purge-manual/routingtest-$${sd:id}.log</Property>
+ </Properties>
<ThresholdFilter level="debug"/>
<Appenders>
@@ -30,7 +34,7 @@
<Routing name="RoutingPurgeIdle">
<Routes pattern="$${sd:id}">
<Route>
- <File name="Routing-${sd:id}"
fileName="target/routing-purge-idle/routingtest-${sd:id}.log">
+ <File name="Routing-${sd:id}" fileName="${filename-idle}">
<PatternLayout>
<Pattern>%d %p %C{1.} [%t] %m%n</Pattern>
</PatternLayout>
@@ -54,7 +58,7 @@
<Routing name="RoutingPurgeManual">
<Routes pattern="$${sd:id}">
<Route>
- <File name="Routing-${sd:id}"
fileName="target/routing-purge-manual/routingtest-${sd:id}.log">
+ <File name="Routing-${sd:id}" fileName="${filename-manual}">
<PatternLayout>
<Pattern>%d %p %C{1.} [%t] %m%n</Pattern>
</PatternLayout>
diff --git a/log4j-core/src/test/resources/log4j-routing.json
b/log4j-core/src/test/resources/log4j-routing.json
index 4322d8d..f6f97d4 100644
--- a/log4j-core/src/test/resources/log4j-routing.json
+++ b/log4j-core/src/test/resources/log4j-routing.json
@@ -15,6 +15,9 @@
* limitations under the license.
*/
{ "configuration": { "status": "error", "name": "RoutingTest",
+ "properties": {
+ "property": { "name": "filename", "value" :
"target/rolling1/rollingtest-$${sd:type}.log" }
+ },
"ThresholdFilter": { "level": "debug" },
"appenders": {
"Console": { "name": "STDOUT",
@@ -28,7 +31,7 @@
"Route": [
{
"RollingFile": {
- "name": "Rolling-${sd:type}", "fileName":
"target/rolling1/rollingtest-${sd:type}.log",
+ "name": "Rolling-${sd:type}", "fileName": "${filename}",
"filePattern": "target/rolling1/test1-${sd:type}.%i.log.gz",
"PatternLayout": {"pattern": "%d %p %C{1.} [%t] %m%n"},
"SizeBasedTriggeringPolicy": { "size": "500" }
diff --git a/log4j-core/src/test/resources/log4j-routing.properties
b/log4j-core/src/test/resources/log4j-routing.properties
index 0dc7547..c365e35 100644
--- a/log4j-core/src/test/resources/log4j-routing.properties
+++ b/log4j-core/src/test/resources/log4j-routing.properties
@@ -16,6 +16,8 @@
status = error
name = RoutingTest
+property.filename = target/routing1/routingtestProps-$${sd:type}.log
+
filter.threshold.type = ThresholdFilter
filter.threshold.level = debug
@@ -31,7 +33,7 @@ appender.routing.routes.pattern = $${sd:type}
appender.routing.routes.route1.type = Route
appender.routing.routes.route1.rolling.type = RollingFile
appender.routing.routes.route1.rolling.name = Routing-${sd:type}
-appender.routing.routes.route1.rolling.fileName =
target/routing1/routingtestProps-${sd:type}.log
+appender.routing.routes.route1.rolling.fileName = ${filename}
appender.routing.routes.route1.rolling.filePattern =
target/routing1/test1-${sd:type}.%i.log.gz
appender.routing.routes.route1.rolling.layout.type = PatternLayout
appender.routing.routes.route1.rolling.layout.pattern = %d %p %C{1.} [%t] %m%n
diff --git a/log4j-core/src/test/resources/log4j-routing.xml
b/log4j-core/src/test/resources/log4j-routing.xml
index 003f7d7..4d83886 100644
--- a/log4j-core/src/test/resources/log4j-routing.xml
+++ b/log4j-core/src/test/resources/log4j-routing.xml
@@ -17,6 +17,9 @@
-->
<Configuration status="OFF" name="RoutingTest">
+ <Properties>
+ <Property
name="filename">target/routing1/routingtest-$${sd:type}.log</Property>
+ </Properties>
<ThresholdFilter level="debug"/>
<Appenders>
@@ -29,7 +32,7 @@
<Routing name="Routing">
<Routes pattern="$${sd:type}">
<Route>
- <RollingFile name="Routing-${sd:type}"
fileName="target/routing1/routingtest-${sd:type}.log"
+ <RollingFile name="Routing-${sd:type}" fileName="${filename}"
filePattern="target/routing1/test1-${sd:type}.%i.log.gz">
<PatternLayout>
<Pattern>%d %p %C{1.} [%t] %m%n</Pattern>
diff --git a/log4j-core/src/test/resources/log4j-routing2.json
b/log4j-core/src/test/resources/log4j-routing2.json
index af67454..baf475a 100644
--- a/log4j-core/src/test/resources/log4j-routing2.json
+++ b/log4j-core/src/test/resources/log4j-routing2.json
@@ -15,6 +15,9 @@
* limitations under the license.
*/
{ "configuration": { "status": "error", "name": "RoutingTest",
+ "properties": {
+ "property": { "name": "filename", "value" :
"target/rolling1/rollingtest-$${sd:type}.log" }
+ },
"ThresholdFilter": { "level": "debug" },
"appenders": {
"appender": [
@@ -25,7 +28,7 @@
"Route": [
{
"RollingFile": {
- "name": "Rolling-${sd:type}", "fileName":
"target/rolling1/rollingtest-${sd:type}.log",
+ "name": "Rolling-${sd:type}", "fileName": "${filename}",
"filePattern": "target/rolling1/test1-${sd:type}.%i.log.gz",
"PatternLayout": {"pattern": "%d %p %C{1.} [%t] %m%n"},
"SizeBasedTriggeringPolicy": { "size": "500" }
diff --git a/log4j-core/src/test/resources/log4j-routing-2767.xml
b/log4j-core/src/test/resources/log4j-routing3350.xml
similarity index 66%
copy from log4j-core/src/test/resources/log4j-routing-2767.xml
copy to log4j-core/src/test/resources/log4j-routing3350.xml
index 4002659..9b17066 100644
--- a/log4j-core/src/test/resources/log4j-routing-2767.xml
+++ b/log4j-core/src/test/resources/log4j-routing3350.xml
@@ -16,20 +16,24 @@
limitations under the License.
-->
-<Configuration status="WARN" name="RoutingTest">
+<Configuration status="OFF" name="Routing3350">
+ <Properties>
+ <Property name="pcode">def</Property>
+ <Property name="drive">target</Property>
+ <Property name="path">/tmp/</Property>
+ <Property name="name">test.log</Property>
+ <Property name="filename">${drive}${path}${name}</Property>
+ <Property name="filepattern">${filename}.%i.backup</Property>
+ </Properties>
<Appenders>
- <Console name="STDOUT">
- <PatternLayout pattern="%m%n"/>
- </Console>
<Routing name="Routing">
- <Routes>
+ <Routes pattern="$${map:pcode}">
<Route>
- <RollingFile name="Routing-${sd:type}"
fileName="target/routing1/routingtest-${sd:type}.log"
-
filePattern="target/routing1/test1-${sd:type}.%i.log.gz">
+ <RollingFile name="Rolling" fileName="${filename}"
filePattern="${filepattern}">
<PatternLayout>
- <Pattern>%d %p %C{1.} [%t] %m%n</Pattern>
+ <pattern>%map{data}%n</pattern>
</PatternLayout>
- <SizeBasedTriggeringPolicy size="500" />
+ <SizeBasedTriggeringPolicy size="500"/>
</RollingFile>
</Route>
</Routes>
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index f0b3f3e..8300101 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -31,6 +31,9 @@
-->
<release version="2.17.2" date="20YY-MM-DD" description="GA Release
2.17.2">
<!-- FIXES -->
+ <action issue="LOG4J2-3317" dev="ckozak" type="fix">
+ Fix RoutingAppender backcompat and disallow recursive evaluation of
lookup results outside of configuration properties.
+ </action>
<action issue="LOG4J2-3333" dev="ckozak" type="fix">
Fix ThreadContextDataInjector initialization deadlock
</action>
