This is an automated email from the ASF dual-hosted git repository. pkarwasz pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit 8321aa691dd83d161f0a32d2f0ae7a21cf510d7c Author: Piotr P. Karwasz <[email protected]> AuthorDate: Sat Jul 23 11:11:04 2022 +0200 [LOG4J2-3559] Correct normalization problem. Properties not starting with 'log4j' are normalized to 'LOG4J' for environment variables and 'log4j2.' for system properties. This PR normalizes them to `null`. --- .../log4j/util/EnvironmentPropertySourceTest.java | 1 + .../log4j/util/PropertiesPropertySourceTest.java | 1 + .../log4j/util/PropertiesUtilOrderTest.java | 18 +++++++----- .../logging/log4j/util/PropertiesUtilTest.java | 33 +++++++++++++++++----- .../log4j/util/EnvironmentPropertySource.java | 4 ++- .../log4j/util/PropertiesPropertySource.java | 3 +- .../apache/logging/log4j/util/PropertiesUtil.java | 32 ++++++++++----------- 7 files changed, 59 insertions(+), 33 deletions(-) diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/util/EnvironmentPropertySourceTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/util/EnvironmentPropertySourceTest.java index 46f3a10225..407915f517 100644 --- a/log4j-api-test/src/test/java/org/apache/logging/log4j/util/EnvironmentPropertySourceTest.java +++ b/log4j-api-test/src/test/java/org/apache/logging/log4j/util/EnvironmentPropertySourceTest.java @@ -35,6 +35,7 @@ public class EnvironmentPropertySourceTest { {"LOG4J_FOO_BAR_PROPERTY", Arrays.asList("foo", "bar", "property")}, {"LOG4J_EXACT", Collections.singletonList("EXACT")}, {"LOG4J_TEST_PROPERTY_NAME", PropertySource.Util.tokenize("Log4jTestPropertyName")}, + {null, Collections.emptyList()} }; } diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/util/PropertiesPropertySourceTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/util/PropertiesPropertySourceTest.java index 2fd487921a..5e741d20d5 100644 --- a/log4j-api-test/src/test/java/org/apache/logging/log4j/util/PropertiesPropertySourceTest.java +++ b/log4j-api-test/src/test/java/org/apache/logging/log4j/util/PropertiesPropertySourceTest.java @@ -36,6 +36,7 @@ public class PropertiesPropertySourceTest { {"log4j2.fooBarProperty", Arrays.asList("foo", "bar", "property")}, {"log4j2.EXACT", Collections.singletonList("EXACT")}, {"log4j2.testPropertyName", PropertySource.Util.tokenize("Log4jTestPropertyName")}, + {null, Collections.emptyList()} }; } diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/util/PropertiesUtilOrderTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/util/PropertiesUtilOrderTest.java index 6cb3831af6..41336688c5 100644 --- a/log4j-api-test/src/test/java/org/apache/logging/log4j/util/PropertiesUtilOrderTest.java +++ b/log4j-api-test/src/test/java/org/apache/logging/log4j/util/PropertiesUtilOrderTest.java @@ -23,6 +23,7 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; +import java.io.InputStream; import java.util.Properties; import org.junit.jupiter.api.BeforeEach; @@ -54,7 +55,8 @@ public class PropertiesUtilOrderTest { @Override public CharSequence getNormalForm(Iterable<? extends CharSequence> tokens) { - return "log4j2." + PropertySource.Util.joinAsCamelCase(tokens); + final CharSequence camelCase = PropertySource.Util.joinAsCamelCase(tokens); + return camelCase.length() > 0 ? "log4j2." + camelCase : null; } @Override @@ -82,11 +84,13 @@ public class PropertiesUtilOrderTest { @BeforeEach public void setUp() throws Exception { - properties.load(ClassLoader.getSystemResourceAsStream("PropertiesUtilOrderTest.properties")); + try (final InputStream is = ClassLoader.getSystemResourceAsStream("PropertiesUtilOrderTest.properties")) { + properties.load(is); + } } @Test - public void normalizedOverrideLegacy() { + public void testNormalizedOverrideLegacy() { final PropertiesUtil util = new PropertiesUtil(properties); final String legacy = "props.legacy"; final String normalized = "props.normalized"; @@ -105,7 +109,7 @@ public class PropertiesUtilOrderTest { } @Test - public void fallsBackToTokenMatching() { + public void testFallsBackToTokenMatching() { final PropertiesUtil util = new PropertiesUtil(properties); for (int i = 1; i <= 4; i++) { final String key = "log4j2.tokenBasedProperty" + i; @@ -118,7 +122,7 @@ public class PropertiesUtilOrderTest { } @Test - public void orderOfNormalizedProperties(EnvironmentVariables env, SystemProperties sysProps) { + public void testOrderOfNormalizedProperties(EnvironmentVariables env, SystemProperties sysProps) { properties.remove("log4j2.normalizedProperty"); properties.remove("LOG4J_normalized.property"); final PropertiesUtil util = new PropertiesUtil(properties); @@ -151,7 +155,7 @@ public class PropertiesUtilOrderTest { } @Test - public void highPriorityNonEnumerableSource(SystemProperties sysProps) { + public void testHighPriorityNonEnumerableSource(SystemProperties sysProps) { // In both datasources assertNotNull(properties.getProperty("log4j2.normalizedProperty")); assertNotNull(properties.getProperty("log4j.onlyLegacy")); @@ -188,7 +192,7 @@ public class PropertiesUtilOrderTest { * @param sysProps */ @Test - public void nullChecks(SystemProperties sysProps) { + public void testNullChecks(SystemProperties sysProps) { sysProps.set("log4j2.someProperty", "sysProps"); sysProps.set("Log4jLegacyProperty", "sysProps"); final PropertiesUtil util = new PropertiesUtil(new NullPropertySource()); diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/util/PropertiesUtilTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/util/PropertiesUtilTest.java index d417777095..05c19aaea8 100644 --- a/log4j-api-test/src/test/java/org/apache/logging/log4j/util/PropertiesUtilTest.java +++ b/log4j-api-test/src/test/java/org/apache/logging/log4j/util/PropertiesUtilTest.java @@ -17,19 +17,22 @@ package org.apache.logging.log4j.util; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.parallel.ResourceAccessMode; -import org.junit.jupiter.api.parallel.ResourceLock; -import org.junit.jupiter.api.parallel.Resources; +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.util.Map; import java.util.Properties; -import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.*; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.parallel.ResourceAccessMode; +import org.junit.jupiter.api.parallel.ResourceLock; +import org.junit.jupiter.api.parallel.Resources; +import org.junitpioneer.jupiter.ReadsSystemProperty; public class PropertiesUtilTest { @@ -148,4 +151,20 @@ public class PropertiesUtilTest { assertEquals(pair[0], util.getStringProperty(pair[1])); } } + + /** + * LOG4J2-3559: the fix for LOG4J2-3413 returns the value of 'log4j2.' for each + * property not starting with 'log4j'. + */ + @Test + @ReadsSystemProperty + public void testLog4jProperty() { + final Properties props = new Properties(); + final String incorrect = "log4j2."; + final String correct = "not.starting.with.log4j"; + props.setProperty(incorrect, incorrect); + props.setProperty(correct, correct); + final PropertiesUtil util = new PropertiesUtil(props); + assertEquals(correct, util.getStringProperty(correct)); + } } diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/util/EnvironmentPropertySource.java b/log4j-api/src/main/java/org/apache/logging/log4j/util/EnvironmentPropertySource.java index 2d93a4c6e2..65ca8b694f 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/util/EnvironmentPropertySource.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/util/EnvironmentPropertySource.java @@ -64,13 +64,15 @@ public class EnvironmentPropertySource implements PropertySource { @Override public CharSequence getNormalForm(final Iterable<? extends CharSequence> tokens) { final StringBuilder sb = new StringBuilder("LOG4J"); + boolean empty = true; for (final CharSequence token : tokens) { + empty = false; sb.append('_'); for (int i = 0; i < token.length(); i++) { sb.append(Character.toUpperCase(token.charAt(i))); } } - return sb.toString(); + return empty ? null : sb.toString(); } @Override diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/util/PropertiesPropertySource.java b/log4j-api/src/main/java/org/apache/logging/log4j/util/PropertiesPropertySource.java index 7d732a23fb..bb96fc498e 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/util/PropertiesPropertySource.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/util/PropertiesPropertySource.java @@ -58,7 +58,8 @@ public class PropertiesPropertySource implements PropertySource { @Override public CharSequence getNormalForm(final Iterable<? extends CharSequence> tokens) { - return PREFIX + Util.joinAsCamelCase(tokens); + final CharSequence camelCase = Util.joinAsCamelCase(tokens); + return camelCase.length() > 0 ? PREFIX + camelCase : null; } @Override diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/util/PropertiesUtil.java b/log4j-api/src/main/java/org/apache/logging/log4j/util/PropertiesUtil.java index 27f4486a92..f03cfaf551 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/util/PropertiesUtil.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/util/PropertiesUtil.java @@ -478,19 +478,19 @@ public final class PropertiesUtil { .filter(Objects::nonNull) .forEach(key -> { final List<CharSequence> tokens = PropertySource.Util.tokenize(key); + final boolean hasTokens = !tokens.isEmpty(); sources.forEach(source -> { - final String value = source.getProperty(key); - if (value != null) { + if (source.containsProperty(key)) { + final String value = source.getProperty(key); literal.putIfAbsent(key, value); - if (!tokens.isEmpty()) { + if (hasTokens) { tokenized.putIfAbsent(tokens, value); } } - final CharSequence normalKey = source.getNormalForm(tokens); - if (normalKey != null) { - final String normalValue = source.getProperty(normalKey.toString()); - if (normalValue != null) { - normalized.putIfAbsent(key, normalValue); + if (hasTokens) { + final String normalKey = Objects.toString(source.getNormalForm(tokens), null); + if (normalKey != null && source.containsProperty(normalKey)) { + normalized.putIfAbsent(key, source.getProperty(normalKey)); } } }); @@ -505,18 +505,16 @@ public final class PropertiesUtil { return literal.get(key); } final List<CharSequence> tokens = PropertySource.Util.tokenize(key); + final boolean hasTokens = !tokens.isEmpty(); for (final PropertySource source : sources) { - final String normalKey = Objects.toString(source.getNormalForm(tokens), null); - if (normalKey != null && source.containsProperty(normalKey)) { - final String normalValue = source.getProperty(normalKey); - // Caching previously unknown keys breaks many tests which set and unset system properties - // normalized.put(key, normalValue); - return normalValue; + if (hasTokens) { + final String normalKey = Objects.toString(source.getNormalForm(tokens), null); + if (normalKey != null && source.containsProperty(normalKey)) { + return source.getProperty(normalKey); + } } if (source.containsProperty(key)) { - final String value = source.getProperty(key); - // literal.put(key, value); - return value; + return source.getProperty(key); } } return tokenized.get(tokens);
