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:57 UTC
[logging-log4j2] branch LOG4J2-2948 created (now 85fbd2f)
This is an automated email from the ASF dual-hosted git repository.
vy pushed a change to branch LOG4J2-2948
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git.
at 85fbd2f LOG4J2-2948 Replace HashSet with IdentityHashMap in ParameterFormatter to detect cycles.
This branch includes the following new commits:
new 85fbd2f LOG4J2-2948 Replace HashSet with IdentityHashMap in ParameterFormatter to detect cycles.
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[logging-log4j2] 01/01: LOG4J2-2948 Replace HashSet with
IdentityHashMap in ParameterFormatter to detect cycles.
Posted by vy...@apache.org.
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.