You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by vy...@apache.org on 2021/03/01 19:16:58 UTC
[logging-log4j2] 01/01: LOG4J2-2948 Replace HashSet with
IdentityHashMap in ParameterFormatter to detect cycles.
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 <vo...@gmail.com>
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.