sharanggupta commented on code in PR #4393:
URL: https://github.com/apache/logging-log4j2/pull/4393#discussion_r4336530525
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/StrSubstitutor.java:
##########
@@ -1049,6 +1068,11 @@ private int substitute(
int endMatchLen = 0;
int nestedVarCount = 0;
while (pos < bufEnd) {
+ if (isEscapeSequence(chars, pos, bufEnd, escape)) {
Review Comment:
Yes, it should. In 064c183 `isEscapeSequence` no longer treats an escape
character in front of the variable prefix as an escape sequence, so `$${...}`
inside an expression counts its `}` again, and `${x:-$${k}}` gives `set` with
`x=set`, as on `2.x`. `testEscapedVariableInDefaultValue` covers it. It gave
`set}` before.
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/StrSubstitutor.java:
##########
@@ -1123,6 +1151,10 @@ private int substitute(
}
}
}
+ varName = unescape(varName, escape);
Review Comment:
Done in 064c183. The nested variables of an expression are now resolved one
at a time, and the characters they return are marked, so only the expression's
own text is unescaped. `${undefined:-${p}}` returns `a$$b` and `a$}b` unchanged
again, and the default value delimiter is still found in the resolved text, as
on `2.x`. `testValuesOfNestedVariablesAreNotUnescaped` covers both.
`testValueEndingWithEscapeCharacterEscapesFollowingVariable` checks that a
nested value ending in `$` still escapes a following `${...}`, as it does on
`2.x`.
##########
log4j-core-test/src/test/java/org/apache/logging/log4j/core/lookup/StrSubstitutorTest.java:
##########
@@ -138,6 +138,79 @@ void testValueEscapeDelimiter() {
assertEquals("", sub.replace("${:\\-:-}"));
}
+ @Test
+ void testEscapedClosingBraceInDefaultValue() {
+ final Map<String, String> valuesMap = new HashMap<>();
+ final StrSubstitutor sub = new StrSubstitutor(valuesMap);
+ // Example from https://github.com/apache/logging-log4j2/issues/2679
+ assertEquals("%d{yyyy-MM-dd HH:mm:ss}{GMT+00}",
sub.replace("${undefined:-%d{yyyy-MM-dd HH:mm:ss$}{GMT+00$}}"));
+ assertEquals("}", sub.replace("${undefined:-$}}"));
+ // The same expression through the Interpolator, as in a configuration
file
Review Comment:
Removed in 064c183.
##########
log4j-core-test/src/test/java/org/apache/logging/log4j/core/lookup/StrSubstitutorTest.java:
##########
@@ -138,6 +138,79 @@ void testValueEscapeDelimiter() {
assertEquals("", sub.replace("${:\\-:-}"));
}
+ @Test
+ void testEscapedClosingBraceInDefaultValue() {
+ final Map<String, String> valuesMap = new HashMap<>();
+ final StrSubstitutor sub = new StrSubstitutor(valuesMap);
+ // Example from https://github.com/apache/logging-log4j2/issues/2679
+ assertEquals("%d{yyyy-MM-dd HH:mm:ss}{GMT+00}",
sub.replace("${undefined:-%d{yyyy-MM-dd HH:mm:ss$}{GMT+00$}}"));
+ assertEquals("}", sub.replace("${undefined:-$}}"));
+ // The same expression through the Interpolator, as in a configuration
file
+ final StrSubstitutor interpolating = new StrSubstitutor(new
Interpolator(new PropertiesLookup(valuesMap)));
+ assertEquals(
+ "%d{yyyy-MM-dd HH:mm:ss}{GMT+00}",
+ interpolating.replace("${env:LOG4J_UNDEFINED_TZ:-%d{yyyy-MM-dd
HH:mm:ss$}{GMT+00$}}"));
+ }
+
+ @Test
+ void testEscapedDelimiterInDefaultValue() {
+ final StrSubstitutor sub = new StrSubstitutor(new HashMap<>());
+ assertEquals("a:-b", sub.replace("${undefined:-a$:-b}"));
+ assertEquals("a:-b", sub.replace("${undefined:-a:$-b}"));
+ }
+
+ @Test
+ void testEscapedEscapeCharacter() {
+ final StrSubstitutor sub = new StrSubstitutor(new HashMap<>());
+ assertEquals("$", sub.replace("${undefined:-$$}"));
+ assertEquals("cost $5", sub.replace("${undefined:-cost $$5}"));
+ }
+
+ @Test
+ void testEscapedDelimiterInVariableName() {
+ final Map<String, String> valuesMap = new HashMap<>();
+ valuesMap.put("name:-x", "value");
+ final StrSubstitutor sub = new StrSubstitutor(valuesMap);
+ assertEquals("value", sub.replace("${name$:-x}"));
+ assertEquals("value", sub.replace("${name:$-x}"));
+ // Example from https://github.com/apache/logging-log4j2/issues/2679:
a system property named `-foo`
+ final String property = "-" + TESTKEY;
+ System.setProperty(property, TESTVAL);
+ try {
+ final StrSubstitutor interpolating = new StrSubstitutor(new
Interpolator(new PropertiesLookup(valuesMap)));
+ assertEquals(TESTVAL, interpolating.replace("${sys$:-" + TESTKEY +
"}"));
+ assertEquals(TESTVAL, interpolating.replace("${sys:$-" + TESTKEY +
"}"));
+ } finally {
+ System.clearProperty(property);
+ }
+ }
+
+ @Test
+ void testNestedLookupsWithEscapedCharacters() {
+ final Map<String, String> valuesMap = new HashMap<>();
+ valuesMap.put("animal", "quick brown fox");
+ valuesMap.put("name", "animal");
+ final StrSubstitutor sub = new StrSubstitutor(valuesMap);
+ assertEquals("quick brown fox",
sub.replace("${undefined:-${animal}}"));
+ assertEquals("quick brown fox", sub.replace("${${name}}"));
+ assertEquals("quick brown fox }", sub.replace("${undefined:-${animal}
$}}"));
+ assertEquals("quick brown fox:-}",
sub.replace("${undefined:-${${name}}$:-$}}"));
+ }
+
+ @Test
+ void testEscapeCharacterBeforeOtherCharactersIsLiteral() {
+ final Map<String, String> valuesMap = new HashMap<>();
+ valuesMap.put("animal", "quick brown fox");
+ final StrSubstitutor sub = new StrSubstitutor(valuesMap);
+ // `$` keeps its literal meaning before any other character
Review Comment:
Removed in 064c183.
##########
log4j-core-test/src/test/java/org/apache/logging/log4j/core/lookup/StrSubstitutorTest.java:
##########
@@ -138,6 +138,79 @@ void testValueEscapeDelimiter() {
assertEquals("", sub.replace("${:\\-:-}"));
}
+ @Test
+ void testEscapedClosingBraceInDefaultValue() {
+ final Map<String, String> valuesMap = new HashMap<>();
+ final StrSubstitutor sub = new StrSubstitutor(valuesMap);
+ // Example from https://github.com/apache/logging-log4j2/issues/2679
+ assertEquals("%d{yyyy-MM-dd HH:mm:ss}{GMT+00}",
sub.replace("${undefined:-%d{yyyy-MM-dd HH:mm:ss$}{GMT+00$}}"));
+ assertEquals("}", sub.replace("${undefined:-$}}"));
+ // The same expression through the Interpolator, as in a configuration
file
+ final StrSubstitutor interpolating = new StrSubstitutor(new
Interpolator(new PropertiesLookup(valuesMap)));
+ assertEquals(
+ "%d{yyyy-MM-dd HH:mm:ss}{GMT+00}",
+ interpolating.replace("${env:LOG4J_UNDEFINED_TZ:-%d{yyyy-MM-dd
HH:mm:ss$}{GMT+00$}}"));
+ }
+
+ @Test
+ void testEscapedDelimiterInDefaultValue() {
+ final StrSubstitutor sub = new StrSubstitutor(new HashMap<>());
+ assertEquals("a:-b", sub.replace("${undefined:-a$:-b}"));
+ assertEquals("a:-b", sub.replace("${undefined:-a:$-b}"));
+ }
+
+ @Test
+ void testEscapedEscapeCharacter() {
+ final StrSubstitutor sub = new StrSubstitutor(new HashMap<>());
+ assertEquals("$", sub.replace("${undefined:-$$}"));
+ assertEquals("cost $5", sub.replace("${undefined:-cost $$5}"));
+ }
+
+ @Test
+ void testEscapedDelimiterInVariableName() {
+ final Map<String, String> valuesMap = new HashMap<>();
+ valuesMap.put("name:-x", "value");
+ final StrSubstitutor sub = new StrSubstitutor(valuesMap);
+ assertEquals("value", sub.replace("${name$:-x}"));
+ assertEquals("value", sub.replace("${name:$-x}"));
+ // Example from https://github.com/apache/logging-log4j2/issues/2679:
a system property named `-foo`
+ final String property = "-" + TESTKEY;
+ System.setProperty(property, TESTVAL);
+ try {
+ final StrSubstitutor interpolating = new StrSubstitutor(new
Interpolator(new PropertiesLookup(valuesMap)));
+ assertEquals(TESTVAL, interpolating.replace("${sys$:-" + TESTKEY +
"}"));
+ assertEquals(TESTVAL, interpolating.replace("${sys:$-" + TESTKEY +
"}"));
+ } finally {
+ System.clearProperty(property);
+ }
+ }
+
+ @Test
+ void testNestedLookupsWithEscapedCharacters() {
+ final Map<String, String> valuesMap = new HashMap<>();
+ valuesMap.put("animal", "quick brown fox");
+ valuesMap.put("name", "animal");
+ final StrSubstitutor sub = new StrSubstitutor(valuesMap);
+ assertEquals("quick brown fox",
sub.replace("${undefined:-${animal}}"));
+ assertEquals("quick brown fox", sub.replace("${${name}}"));
+ assertEquals("quick brown fox }", sub.replace("${undefined:-${animal}
$}}"));
+ assertEquals("quick brown fox:-}",
sub.replace("${undefined:-${${name}}$:-$}}"));
+ }
+
+ @Test
+ void testEscapeCharacterBeforeOtherCharactersIsLiteral() {
+ final Map<String, String> valuesMap = new HashMap<>();
+ valuesMap.put("animal", "quick brown fox");
+ final StrSubstitutor sub = new StrSubstitutor(valuesMap);
+ // `$` keeps its literal meaning before any other character
+ assertEquals("$x", sub.replace("${undefined:-$x}"));
+ assertEquals("a$b c$ d", sub.replace("${undefined:-a$b c$ d}"));
+ // Outside of a variable reference only `$${` is an escape
Review Comment:
Removed in 064c183.
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/StrSubstitutor.java:
##########
@@ -1049,6 +1068,11 @@ private int substitute(
int endMatchLen = 0;
int nestedVarCount = 0;
while (pos < bufEnd) {
+ if (isEscapeSequence(chars, pos, bufEnd, escape)) {
+ // an escaped character has no special meaning
Review Comment:
Removed in 064c183.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
