This is an automated email from the ASF dual-hosted git repository. vy pushed a commit to branch LOG4J2-2948 in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit 85fbd2fb1a0b81da999d18b8d189bde3431a9b01 Author: Volkan Yazici <[email protected]> AuthorDate: Mon Mar 1 20:16:14 2021 +0100 LOG4J2-2948 Replace HashSet with IdentityHashMap in ParameterFormatter to detect cycles. --- .../logging/log4j/message/ParameterFormatter.java | 97 ++++++++++++---------- .../log4j/message/ParameterFormatterTest.java | 13 +-- src/changes/changes.xml | 3 + 3 files changed, 63 insertions(+), 50 deletions(-) diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterFormatter.java b/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterFormatter.java index 5c4cc73..d8b366d 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterFormatter.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterFormatter.java @@ -16,16 +16,17 @@ */ package org.apache.logging.log4j.message; +import org.apache.logging.log4j.util.StringBuilders; + import java.text.SimpleDateFormat; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.Date; -import java.util.HashSet; +import java.util.IdentityHashMap; import java.util.Map; import java.util.Set; -import org.apache.logging.log4j.util.StringBuilders; - /** * Supports parameter formatting as used in ParameterizedMessage and ReusableParameterizedMessage. */ @@ -60,7 +61,8 @@ final class ParameterFormatter { private static final char DELIM_STOP = '}'; private static final char ESCAPE_CHAR = '\\'; - private static ThreadLocal<SimpleDateFormat> threadLocalSimpleDateFormat = new ThreadLocal<>(); + private static final ThreadLocal<SimpleDateFormat> SIMPLE_DATE_FORMAT_REF = + ThreadLocal.withInitial(() -> new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSSZ")); private ParameterFormatter() { } @@ -448,7 +450,7 @@ final class ParameterFormatter { * @param str the StringBuilder that o will be appended to * @param dejaVu a list of container identities that were already used. */ - static void recursiveDeepToString(final Object o, final StringBuilder str, final Set<String> dejaVu) { + static void recursiveDeepToString(final Object o, final StringBuilder str, final Set<Object> dejaVu) { if (appendSpecialTypes(o, str)) { return; } @@ -468,20 +470,11 @@ final class ParameterFormatter { return false; } final Date date = (Date) o; - final SimpleDateFormat format = getSimpleDateFormat(); + final SimpleDateFormat format = SIMPLE_DATE_FORMAT_REF.get(); str.append(format.format(date)); return true; } - private static SimpleDateFormat getSimpleDateFormat() { - SimpleDateFormat result = threadLocalSimpleDateFormat.get(); - if (result == null) { - result = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSSZ"); - threadLocalSimpleDateFormat.set(result); - } - return result; - } - /** * Returns {@code true} if the specified object is an array, a Map or a Collection. */ @@ -489,8 +482,10 @@ final class ParameterFormatter { return o.getClass().isArray() || o instanceof Map || o instanceof Collection; } - private static void appendPotentiallyRecursiveValue(final Object o, final StringBuilder str, - final Set<String> dejaVu) { + private static void appendPotentiallyRecursiveValue( + final Object o, + final StringBuilder str, + final Set<Object> dejaVu) { final Class<?> oClass = o.getClass(); if (oClass.isArray()) { appendArray(o, str, dejaVu, oClass); @@ -501,7 +496,10 @@ final class ParameterFormatter { } } - private static void appendArray(final Object o, final StringBuilder str, Set<String> dejaVu, + private static void appendArray( + final Object o, + final StringBuilder str, + final Set<Object> dejaVu, final Class<?> oClass) { if (oClass == byte[].class) { str.append(Arrays.toString((byte[]) o)); @@ -520,15 +518,15 @@ final class ParameterFormatter { } else if (oClass == char[].class) { str.append(Arrays.toString((char[]) o)); } else { - if (dejaVu == null) { - dejaVu = new HashSet<>(); - } // special handling of container Object[] - final String id = identityToString(o); - if (dejaVu.contains(id)) { + final Set<Object> effectiveDejaVu = dejaVu == null + ? Collections.newSetFromMap(new IdentityHashMap<>()) + : dejaVu; + final boolean seen = !effectiveDejaVu.add(o); + if (seen) { + final String id = identityToString(o); str.append(RECURSION_PREFIX).append(id).append(RECURSION_SUFFIX); } else { - dejaVu.add(id); final Object[] oArray = (Object[]) o; str.append('['); boolean first = true; @@ -538,24 +536,28 @@ final class ParameterFormatter { } else { str.append(", "); } - recursiveDeepToString(current, str, new HashSet<>(dejaVu)); + recursiveDeepToString(current, str, effectiveDejaVu); } str.append(']'); } - //str.append(Arrays.deepToString((Object[]) o)); } } - private static void appendMap(final Object o, final StringBuilder str, Set<String> dejaVu) { - // special handling of container Map - if (dejaVu == null) { - dejaVu = new HashSet<>(); - } - final String id = identityToString(o); - if (dejaVu.contains(id)) { + /** + * Specialized handler for {@link Map}s. + */ + private static void appendMap( + final Object o, + final StringBuilder str, + final Set<Object> dejaVu) { + final Set<Object> effectiveDejaVu = dejaVu == null + ? Collections.newSetFromMap(new IdentityHashMap<>()) + : dejaVu; + final boolean seen = !effectiveDejaVu.add(o); + if (seen) { + final String id = identityToString(o); str.append(RECURSION_PREFIX).append(id).append(RECURSION_SUFFIX); } else { - dejaVu.add(id); final Map<?, ?> oMap = (Map<?, ?>) o; str.append('{'); boolean isFirst = true; @@ -568,24 +570,29 @@ final class ParameterFormatter { } final Object key = current.getKey(); final Object value = current.getValue(); - recursiveDeepToString(key, str, new HashSet<>(dejaVu)); + recursiveDeepToString(key, str, effectiveDejaVu); str.append('='); - recursiveDeepToString(value, str, new HashSet<>(dejaVu)); + recursiveDeepToString(value, str, effectiveDejaVu); } str.append('}'); } } - private static void appendCollection(final Object o, final StringBuilder str, Set<String> dejaVu) { - // special handling of container Collection - if (dejaVu == null) { - dejaVu = new HashSet<>(); - } - final String id = identityToString(o); - if (dejaVu.contains(id)) { + /** + * Specialized handler for {@link Collection}s. + */ + private static void appendCollection( + final Object o, + final StringBuilder str, + final Set<Object> dejaVu) { + final Set<Object> effectiveDejaVu = dejaVu == null + ? Collections.newSetFromMap(new IdentityHashMap<>()) + : dejaVu; + final boolean seen = !effectiveDejaVu.add(o); + if (seen) { + final String id = identityToString(o); str.append(RECURSION_PREFIX).append(id).append(RECURSION_SUFFIX); } else { - dejaVu.add(id); final Collection<?> oCol = (Collection<?>) o; str.append('['); boolean isFirst = true; @@ -595,7 +602,7 @@ final class ParameterFormatter { } else { str.append(", "); } - recursiveDeepToString(anOCol, str, new HashSet<>(dejaVu)); + recursiveDeepToString(anOCol, str, effectiveDejaVu); } str.append(']'); } diff --git a/log4j-api/src/test/java/org/apache/logging/log4j/message/ParameterFormatterTest.java b/log4j-api/src/test/java/org/apache/logging/log4j/message/ParameterFormatterTest.java index f1b1917..644f82d 100644 --- a/log4j-api/src/test/java/org/apache/logging/log4j/message/ParameterFormatterTest.java +++ b/log4j-api/src/test/java/org/apache/logging/log4j/message/ParameterFormatterTest.java @@ -24,12 +24,12 @@ import java.util.List; import static org.junit.jupiter.api.Assertions.*; /** - * Tests ParameterFormatter. + * Tests {@link ParameterFormatter}. */ public class ParameterFormatterTest { @Test - public void testCountArgumentPlaceholders() throws Exception { + public void testCountArgumentPlaceholders() { assertEquals(0, ParameterFormatter.countArgumentPlaceholders("")); assertEquals(0, ParameterFormatter.countArgumentPlaceholders("aaa")); assertEquals(0, ParameterFormatter.countArgumentPlaceholders("\\{}")); @@ -169,9 +169,10 @@ public class ParameterFormatterTest { } @Test - public void testDeepToString() throws Exception { + public void testDeepToString() { final List<Object> list = new ArrayList<>(); list.add(1); + // noinspection CollectionAddedToSelf list.add(list); list.add(2); final String actual = ParameterFormatter.deepToString(list); @@ -180,13 +181,15 @@ public class ParameterFormatterTest { } @Test - public void testIdentityToString() throws Exception { + public void testIdentityToString() { final List<Object> list = new ArrayList<>(); list.add(1); + // noinspection CollectionAddedToSelf list.add(list); list.add(2); final String actual = ParameterFormatter.identityToString(list); final String expected = list.getClass().getName() + "@" + Integer.toHexString(System.identityHashCode(list)); assertEquals(expected, actual); } -} \ No newline at end of file + +} diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 6808cf0..ca15835 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -31,6 +31,9 @@ --> <release version="2.14.1" date="2021-MM-DD" description="GA Release 2.14.1"> <!-- FIXES --> + <action issue="LOG4J2-2948" dev="vy" type="fix"> + Replace HashSet with IdentityHashMap in ParameterFormatter to detect cycles. + </action> <action issue="LOG4J2-2981" dev="rgoers" type="fix"> OnStartupTriggeringPolicy would fail to cause the file to roll over with DirectWriteTriggeringPolicy unless minSize was set to 0.
