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:22:19 UTC

[logging-log4j2] branch LOG4J2-2948 updated (85fbd2f -> 17ceed8)

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.


 discard 85fbd2f  LOG4J2-2948 Replace HashSet with IdentityHashMap in ParameterFormatter to detect cycles.
     add 9ed470f  LOG4J2-3028: Always clear the OutputStreamManager buffer on flush
     new 17ceed8  LOG4J2-2948 Replace HashSet with IdentityHashMap in ParameterFormatter to detect cycles.

This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version.  This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:

 * -- * -- B -- O -- O -- O   (85fbd2f)
            \
             N -- N -- N   refs/heads/LOG4J2-2948 (17ceed8)

You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.

Any revisions marked "omit" are not gone; other references still
refer to them.  Any revisions marked "discard" are gone forever.

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.


Summary of changes:
 .../log4j/core/appender/OutputStreamManager.java   |  9 +++++---
 .../core/appender/OutputStreamManagerTest.java     | 27 ++++++++++++++++++++++
 src/changes/changes.xml                            |  3 +++
 3 files changed, 36 insertions(+), 3 deletions(-)


[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 17ceed8fec1564c39384955ba51ff30070f945fa
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 0ad7b0f..0ac7fdc 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-3028" dev="ckozak" type="fix" due-to="Jakub Kozlowski">
         OutputStreamManager.flushBuffer always resets the buffer, previously the buffer was not reset after an exception.
       </action>