You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by rg...@apache.org on 2022/01/02 07:36:36 UTC
[logging-log4j2] branch release-2.x updated: Log4 j2 3306 (#677)
This is an automated email from the ASF dual-hosted git repository.
rgoers 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 6a21ada Log4 j2 3306 (#677)
6a21ada is described below
commit 6a21ada4da85445e7b701d1d106fe44607829910
Author: Ralph Goers <rg...@apache.org>
AuthorDate: Sun Jan 2 00:36:28 2022 -0700
Log4 j2 3306 (#677)
* LOG4J2-3301 - Prevent recursion
* LOG4J2-3301 - Prevent OptionConverter from recursing endlessly
* LOG4J2-3301 - Add more tests
---
.../org/apache/log4j/helpers/OptionConverter.java | 63 ++++++++++++++++++-
.../logging/log4j/core/util/OptionConverter.java | 73 ++++++++++++++++++++--
.../log4j/core/util/OptionConverterTest.java | 59 +++++++++++++++++
src/changes/changes.xml | 4 ++
4 files changed, 194 insertions(+), 5 deletions(-)
diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/helpers/OptionConverter.java b/log4j-1.2-api/src/main/java/org/apache/log4j/helpers/OptionConverter.java
index 4044668..62f1f69 100644
--- a/log4j-1.2-api/src/main/java/org/apache/log4j/helpers/OptionConverter.java
+++ b/log4j-1.2-api/src/main/java/org/apache/log4j/helpers/OptionConverter.java
@@ -25,11 +25,15 @@ import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.core.lookup.StrSubstitutor;
import org.apache.logging.log4j.util.LoaderUtil;
+import org.apache.logging.log4j.util.PropertiesUtil;
+import org.apache.logging.log4j.util.Strings;
import java.io.InputStream;
import java.io.InterruptedIOException;
import java.lang.reflect.InvocationTargetException;
import java.net.URL;
+import java.util.ArrayList;
+import java.util.List;
import java.util.Properties;
/**
@@ -296,7 +300,64 @@ public class OptionConverter {
* @throws IllegalArgumentException if <code>val</code> is malformed.
*/
public static String substVars(String val, Properties props) throws IllegalArgumentException {
- return StrSubstitutor.replace(val, props);
+ return substVars(val, props, new ArrayList<>());
+ }
+
+ private static String substVars(final String val, final Properties props, List<String> keys)
+ throws IllegalArgumentException {
+
+ final StringBuilder sbuf = new StringBuilder();
+
+ int i = 0;
+ int j;
+ int k;
+
+ while (true) {
+ j = val.indexOf(DELIM_START, i);
+ if (j == -1) {
+ // no more variables
+ if (i == 0) { // this is a simple string
+ return val;
+ }
+ // add the tail string which contails no variables and return the result.
+ sbuf.append(val.substring(i, val.length()));
+ return sbuf.toString();
+ }
+ sbuf.append(val.substring(i, j));
+ k = val.indexOf(DELIM_STOP, j);
+ if (k == -1) {
+ throw new IllegalArgumentException(Strings.dquote(val)
+ + " has no closing brace. Opening brace at position " + j
+ + '.');
+ }
+ j += DELIM_START_LEN;
+ final String key = val.substring(j, k);
+ // first try in System properties
+ String replacement = PropertiesUtil.getProperties().getStringProperty(key, null);
+ // then try props parameter
+ if (replacement == null && props != null) {
+ replacement = props.getProperty(key);
+ }
+
+ if (replacement != null) {
+
+ // Do variable substitution on the replacement string
+ // such that we can solve "Hello ${x2}" as "Hello p1"
+ // the where the properties are
+ // x1=p1
+ // x2=${x1}
+ if (!keys.contains(key)) {
+ List<String> usedKeys = new ArrayList<>(keys);
+ usedKeys.add(key);
+ final String recursiveReplacement = substVars(replacement, props, usedKeys);
+ sbuf.append(recursiveReplacement);
+ } else {
+ sbuf.append(replacement);
+ }
+
+ }
+ i = k + DELIM_STOP_LEN;
+ }
}
public static org.apache.logging.log4j.Level convertLevel(String level,
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/OptionConverter.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/OptionConverter.java
index c1f0b13..bd31fd3 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/OptionConverter.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/OptionConverter.java
@@ -17,13 +17,16 @@
package org.apache.logging.log4j.core.util;
import java.io.InterruptedIOException;
+import java.util.ArrayList;
+import java.util.List;
import java.util.Locale;
import java.util.Properties;
import org.apache.logging.log4j.Level;
import org.apache.logging.log4j.Logger;
-import org.apache.logging.log4j.core.lookup.StrSubstitutor;
import org.apache.logging.log4j.status.StatusLogger;
+import org.apache.logging.log4j.util.PropertiesUtil;
+import org.apache.logging.log4j.util.Strings;
/**
* A convenience class to convert property values to specific types.
@@ -32,6 +35,10 @@ public final class OptionConverter {
private static final Logger LOGGER = StatusLogger.getLogger();
+ private static final String DELIM_START = "${";
+ private static final char DELIM_STOP = '}';
+ private static final int DELIM_START_LEN = 2;
+ private static final int DELIM_STOP_LEN = 1;
private static final int ONE_K = 1024;
/**
@@ -162,9 +169,10 @@ public final class OptionConverter {
if (hashIndex == -1) {
if("NULL".equalsIgnoreCase(value)) {
return null;
+ } else {
+ // no class name specified : use standard Level class
+ return Level.toLevel(value, defaultValue);
}
- // no class name specified : use standard Level class
- return Level.toLevel(value, defaultValue);
}
Level result = defaultValue;
@@ -341,6 +349,63 @@ public final class OptionConverter {
*/
public static String substVars(final String val, final Properties props) throws
IllegalArgumentException {
- return StrSubstitutor.replace(val, props);
+ return substVars(val, props, new ArrayList<>());
+ }
+
+ private static String substVars(final String val, final Properties props, List<String> keys)
+ throws IllegalArgumentException {
+
+ final StringBuilder sbuf = new StringBuilder();
+
+ int i = 0;
+ int j;
+ int k;
+
+ while (true) {
+ j = val.indexOf(DELIM_START, i);
+ if (j == -1) {
+ // no more variables
+ if (i == 0) { // this is a simple string
+ return val;
+ }
+ // add the tail string which contails no variables and return the result.
+ sbuf.append(val.substring(i, val.length()));
+ return sbuf.toString();
+ }
+ sbuf.append(val.substring(i, j));
+ k = val.indexOf(DELIM_STOP, j);
+ if (k == -1) {
+ throw new IllegalArgumentException(Strings.dquote(val)
+ + " has no closing brace. Opening brace at position " + j
+ + '.');
+ }
+ j += DELIM_START_LEN;
+ final String key = val.substring(j, k);
+ // first try in System properties
+ String replacement = PropertiesUtil.getProperties().getStringProperty(key, null);
+ // then try props parameter
+ if (replacement == null && props != null) {
+ replacement = props.getProperty(key);
+ }
+
+ if (replacement != null) {
+
+ // Do variable substitution on the replacement string
+ // such that we can solve "Hello ${x2}" as "Hello p1"
+ // the where the properties are
+ // x1=p1
+ // x2=${x1}
+ if (!keys.contains(key)) {
+ List<String> usedKeys = new ArrayList<>(keys);
+ usedKeys.add(key);
+ final String recursiveReplacement = substVars(replacement, props, usedKeys);
+ sbuf.append(recursiveReplacement);
+ } else {
+ sbuf.append(replacement);
+ }
+
+ }
+ i = k + DELIM_STOP_LEN;
+ }
}
}
diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/util/OptionConverterTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/util/OptionConverterTest.java
index d0d7f90..65de5d7 100644
--- a/log4j-core/src/test/java/org/apache/logging/log4j/core/util/OptionConverterTest.java
+++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/util/OptionConverterTest.java
@@ -31,6 +31,65 @@ public class OptionConverterTest {
public void testSubstVars() {
Properties props = new Properties();
props.setProperty("key", "${key}");
+ props.setProperty("testKey", "Log4j");
assertEquals("Value of key is ${key}.", OptionConverter.substVars("Value of key is ${key}.", props));
+ assertEquals("Value of key is .", OptionConverter.substVars("Value of key is ${key2}.", props));
+ assertEquals("Value of testKey:testKey is Log4j:Log4j",
+ OptionConverter.substVars("Value of testKey:testKey is ${testKey}:${testKey}", props));
+ }
+
+ /**
+ * StrSubstitutor would resolve ${key} to Key, append the result to "test" and then resolve ${testKey}.
+ * Verify that substVars doesn't construct dynamic keys.
+ */
+ @Test
+ public void testAppend() {
+ Properties props = new Properties();
+ props.setProperty("key", "Key");
+ props.setProperty("testKey", "Hello");
+ assertEquals("Value of testKey is }",
+ OptionConverter.substVars("Value of testKey is ${test${key}}", props));
+ }
+
+ /**
+ * StrSubstitutor would resolve ${key}, append the result to "test" and then resolve ${testKey}.
+ * Verify that substVars will treat the second expression up to the first '}' as part of the key.
+ */
+ @Test
+ public void testAppend2() {
+ Properties props = new Properties();
+ props.setProperty("test${key", "Hello");
+ assertEquals("Value of testKey is Hello}",
+ OptionConverter.substVars("Value of testKey is ${test${key}}", props));
+ }
+
+ @Test
+ public void testRecursion() {
+ Properties props = new RecursiveProperties();
+ props.setProperty("name", "Neo");
+ props.setProperty("greeting", "Hello ${name}");
+
+ String s = props.getProperty("greeting");
+ System.out.println("greeting = '"+s+"'");
+ }
+
+ private static class RecursiveProperties extends Properties {
+ @Override
+ public String getProperty(String key)
+ {
+ System.out.println("getProperty for "+key);
+ try
+ {
+ String val = super.getProperty(key);
+ // The following call works for log4j 2.17.0 and causes StackOverflowError for 2.17.1
+ // This is because substVars change implementation in 2.17.1 to call StrSubstitutor.replace(val, props)
+ // which calls props.getProperty() for EVERY property making it recursive
+ return OptionConverter.substVars(val, this);
+ }
+ catch (Exception e)
+ {
+ return super.getProperty(key);
+ }
+ }
}
}
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 6989a12..191b8b7 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -30,10 +30,14 @@
- "remove" - Removed
-->
<release version="2.17.2" date="20YY-MM-DD" description="GA Release 2.17.2">
+
<action issue="LOG4J2-3267" dev="rpopma" type="update">
Change modifier of method org.apache.logging.log4j.core.tools.Generate#generate to public (was package private) to facilitate automated code generation.
</action>
<!-- FIXES -->
+ <action issue="LOG4J2-3301" dev="rgoers" type="fix">
+ OptionConverter could cause a StackOverflowError.
+ </action>
<action dev="ggregory" type="fix">
Log4j 1.2 bridge class ConsoleAppender should extend WriterAppender and provide better compatibility with custom appenders.
</action>