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/04 10:56:59 UTC

[logging-log4j2] branch release-2.x updated: LOG4J2-2948 Replace HashSet with IdentityHashMap in ParameterFormatter to detect cycles. (#471)

This is an automated email from the ASF dual-hosted git repository.

vy pushed a commit to branch release-2.x
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git


The following commit(s) were added to refs/heads/release-2.x by this push:
     new 8ae3eb8  LOG4J2-2948 Replace HashSet with IdentityHashMap in ParameterFormatter to detect cycles. (#471)
8ae3eb8 is described below

commit 8ae3eb838f0efaca86a641f7e4dc90d5e736710f
Author: Volkan Yazıcı <vo...@gmail.com>
AuthorDate: Thu Mar 4 11:56:47 2021 +0100

    LOG4J2-2948 Replace HashSet with IdentityHashMap in ParameterFormatter to detect cycles. (#471)
---
 .../apache/logging/log4j/message/MapMessage.java   |   6 +-
 .../logging/log4j/message/ParameterFormatter.java  | 172 +++++++++++++--------
 .../log4j/message/ParameterFormatterTest.java      |  30 +++-
 src/changes/changes.xml                            |   3 +
 4 files changed, 135 insertions(+), 76 deletions(-)

diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/message/MapMessage.java b/log4j-api/src/main/java/org/apache/logging/log4j/message/MapMessage.java
index 60c5d22..a54c7b8 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/message/MapMessage.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/message/MapMessage.java
@@ -357,7 +357,7 @@ public class MapMessage<M extends MapMessage<M, V>, V> implements MultiFormatStr
                     .append(data.getKeyAt(i))
                     .append("\">");
             final int size = sb.length();
-            ParameterFormatter.recursiveDeepToString(data.getValueAt(i), sb, null);
+            ParameterFormatter.recursiveDeepToString(data.getValueAt(i), sb);
             StringBuilders.escapeXml(sb, size);
             sb.append("</Entry>\n");
         }
@@ -407,7 +407,7 @@ public class MapMessage<M extends MapMessage<M, V>, V> implements MultiFormatStr
                 sb.append(' ');
             }
             sb.append(data.getKeyAt(i)).append(Chars.EQ).append(Chars.DQUOTE);
-            ParameterFormatter.recursiveDeepToString(data.getValueAt(i), sb, null);
+            ParameterFormatter.recursiveDeepToString(data.getValueAt(i), sb);
             sb.append(Chars.DQUOTE);
         }
     }
@@ -434,7 +434,7 @@ public class MapMessage<M extends MapMessage<M, V>, V> implements MultiFormatStr
             if (quoted) {
                 sb.append(Chars.DQUOTE);
             }
-            ParameterFormatter.recursiveDeepToString(data.getValueAt(i), sb, null);
+            ParameterFormatter.recursiveDeepToString(data.getValueAt(i), sb);
             if (quoted) {
                 sb.append(Chars.DQUOTE);
             }
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..1cc3b0c 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() {
     }
@@ -186,7 +188,7 @@ final class ParameterFormatter {
         for (int i = 0; i < argCount; i++) {
             buffer.append(messagePattern, previous, indices[i]);
             previous = indices[i] + 2;
-            recursiveDeepToString(arguments[i], buffer, null);
+            recursiveDeepToString(arguments[i], buffer);
         }
         buffer.append(messagePattern, previous, messagePattern.length());
     }
@@ -211,7 +213,7 @@ final class ParameterFormatter {
         for (int i = 0; i < argCount; i++) {
             buffer.append(messagePattern, previous, indices[i]);
             previous = indices[i] + 2;
-            recursiveDeepToString(arguments[i], buffer, null);
+            recursiveDeepToString(arguments[i], buffer);
         }
         buffer.append(messagePattern, previous, patternLength);
     }
@@ -363,7 +365,7 @@ final class ParameterFormatter {
     private static void writeArgOrDelimPair(final Object[] arguments, final int argCount, final int currentArgument,
             final StringBuilder buffer) {
         if (currentArgument < argCount) {
-            recursiveDeepToString(arguments[currentArgument], buffer, null);
+            recursiveDeepToString(arguments[currentArgument], buffer);
         } else {
             writeDelimPair(buffer);
         }
@@ -420,35 +422,54 @@ final class ParameterFormatter {
             return Byte.toString((Byte) o);
         }
         final StringBuilder str = new StringBuilder();
-        recursiveDeepToString(o, str, null);
+        recursiveDeepToString(o, str);
         return str.toString();
     }
 
     /**
-     * This method performs a deep toString of the given Object.
-     * Primitive arrays are converted using their respective Arrays.toString methods while
-     * special handling is implemented for "container types", i.e. Object[], Map and Collection because those could
-     * contain themselves.
+     * This method performs a deep {@code toString()} of the given {@code Object}.
      * <p>
-     * dejaVu is used in case of those container types to prevent an endless recursion.
-     * </p>
+     * Primitive arrays are converted using their respective {@code Arrays.toString()} methods, while
+     * special handling is implemented for <i>container types</i>, i.e. {@code Object[]}, {@code Map} and {@code Collection},
+     * because those could contain themselves.
      * <p>
-     * It should be noted that neither AbstractMap.toString() nor AbstractCollection.toString() implement such a
-     * behavior.
-     * They only check if the container is directly contained in itself, but not if a contained container contains the
-     * original one. Because of that, Arrays.toString(Object[]) isn't safe either.
-     * Confusing? Just read the last paragraph again and check the respective toString() implementation.
-     * </p>
+     * It should be noted that neither {@code AbstractMap.toString()} nor {@code AbstractCollection.toString()} implement such a behavior.
+     * They only check if the container is directly contained in itself, but not if a contained container contains the original one.
+     * Because of that, {@code Arrays.toString(Object[])} isn't safe either.
+     * Confusing? Just read the last paragraph again and check the respective {@code toString()} implementation.
      * <p>
-     * This means, in effect, that logging would produce a usable output even if an ordinary System.out.println(o)
-     * would produce a relatively hard-to-debug StackOverflowError.
-     * </p>
+     * This means, in effect, that logging would produce a usable output even if an ordinary {@code System.out.println(o)}
+     * would produce a relatively hard-to-debug {@code StackOverflowError}.
      *
-     * @param o      the Object to convert into a String
-     * @param str    the StringBuilder that o will be appended to
-     * @param dejaVu a list of container identities that were already used.
+     * @param o      the {@code Object} to convert into a {@code String}
+     * @param str    the {@code StringBuilder} that {@code o} will be appended to
      */
-    static void recursiveDeepToString(final Object o, final StringBuilder str, final Set<String> dejaVu) {
+    static void recursiveDeepToString(final Object o, final StringBuilder str) {
+        recursiveDeepToString(o, str, null);
+    }
+
+    /**
+     * This method performs a deep {@code toString()} of the given {@code Object}.
+     * <p>
+     * Primitive arrays are converted using their respective {@code Arrays.toString()} methods, while
+     * special handling is implemented for <i>container types</i>, i.e. {@code Object[]}, {@code Map} and {@code Collection},
+     * because those could contain themselves.
+     * <p>
+     * {@code dejaVu} is used in case of those container types to prevent an endless recursion.
+     * <p>
+     * It should be noted that neither {@code AbstractMap.toString()} nor {@code AbstractCollection.toString()} implement such a behavior.
+     * They only check if the container is directly contained in itself, but not if a contained container contains the original one.
+     * Because of that, {@code Arrays.toString(Object[])} isn't safe either.
+     * Confusing? Just read the last paragraph again and check the respective {@code toString()} implementation.
+     * <p>
+     * This means, in effect, that logging would produce a usable output even if an ordinary {@code System.out.println(o)}
+     * would produce a relatively hard-to-debug {@code StackOverflowError}.
+     *
+     * @param o      the {@code Object} to convert into a {@code String}
+     * @param str    the {@code StringBuilder} that {@code o} will be appended to
+     * @param dejaVu a set of container objects directly or transitively containing {@code o}
+     */
+    private static void recursiveDeepToString(final Object o, final StringBuilder str, final Set<Object> dejaVu) {
         if (appendSpecialTypes(o, str)) {
             return;
         }
@@ -468,20 +489,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 +501,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);
@@ -498,10 +512,15 @@ final class ParameterFormatter {
             appendMap(o, str, dejaVu);
         } else if (o instanceof Collection) {
             appendCollection(o, str, dejaVu);
+        } else {
+            throw new IllegalArgumentException("was expecting a container, found " + oClass);
         }
     }
 
-    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 +539,13 @@ 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 = getOrCreateDejaVu(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 +555,26 @@ final class ParameterFormatter {
                     } else {
                         str.append(", ");
                     }
-                    recursiveDeepToString(current, str, new HashSet<>(dejaVu));
+                    recursiveDeepToString(current, str, cloneDejaVu(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 = getOrCreateDejaVu(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 +587,27 @@ final class ParameterFormatter {
                 }
                 final Object key = current.getKey();
                 final Object value = current.getValue();
-                recursiveDeepToString(key, str, new HashSet<>(dejaVu));
+                recursiveDeepToString(key, str, cloneDejaVu(effectiveDejaVu));
                 str.append('=');
-                recursiveDeepToString(value, str, new HashSet<>(dejaVu));
+                recursiveDeepToString(value, str, cloneDejaVu(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 = getOrCreateDejaVu(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,12 +617,28 @@ final class ParameterFormatter {
                 } else {
                     str.append(", ");
                 }
-                recursiveDeepToString(anOCol, str, new HashSet<>(dejaVu));
+                recursiveDeepToString(anOCol, str, cloneDejaVu(effectiveDejaVu));
             }
             str.append(']');
         }
     }
 
+    private static Set<Object> getOrCreateDejaVu(Set<Object> dejaVu) {
+        return dejaVu == null
+                ? createDejaVu()
+                : dejaVu;
+    }
+
+    private static Set<Object> createDejaVu() {
+        return Collections.newSetFromMap(new IdentityHashMap<>());
+    }
+
+    private static Set<Object> cloneDejaVu(Set<Object> dejaVu) {
+        Set<Object> clonedDejaVu = createDejaVu();
+        clonedDejaVu.addAll(dejaVu);
+        return clonedDejaVu;
+    }
+
     private static void tryObjectToString(final Object o, final StringBuilder str) {
         // it's just some other Object, we can only use toString().
         try {
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..ba793e3 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
@@ -19,17 +19,18 @@ package org.apache.logging.log4j.message;
 import org.junit.jupiter.api.Test;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 
-import static org.junit.jupiter.api.Assertions.*;
+import static org.junit.jupiter.api.Assertions.assertEquals;
 
 /**
- * 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 +170,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 +182,29 @@ public class ParameterFormatterTest {
     }
 
     @Test
-    public void testIdentityToString() throws Exception {
+    public void testDeepToStringUsingNonRecursiveButConsequentObjects() {
         final List<Object> list = new ArrayList<>();
+        final Object item = Collections.singletonList(0);
         list.add(1);
+        list.add(item);
+        list.add(2);
+        list.add(item);
+        list.add(3);
+        final String actual = ParameterFormatter.deepToString(list);
+        final String expected = "[1, [0], 2, [0], 3]";
+        assertEquals(expected, actual);
+    }
+
+    @Test
+    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 e6b20fe..1b136d5 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>


Re: [logging-log4j2] branch release-2.x updated: LOG4J2-2948 Replace HashSet with IdentityHashMap in ParameterFormatter to detect cycles. (#471)

Posted by Ralph Goers <ra...@dslextreme.com>.
Ahh, ok. I guess I didn’t look closely at the diff. Just noticed it was still using SimpleDateFormat and you had changed the line.

Ralph

> On Mar 4, 2021, at 8:54 AM, Volkan Yazıcı <vo...@gmail.com> wrote:
> 
> I did neither replace the usage of SimpleDateFormat in release-2.x, nor
> implemented myself DateTimeFormatter in master. I have just touched to the
> indentation in release-2.x. I deliberately left it out since it wasn't
> related to the ticket and I could not invest time into figuring out the
> reason for the discrepancy.
> 
> On Thu, Mar 4, 2021 at 4:48 PM Ralph Goers <ra...@dslextreme.com>
> wrote:
> 
>> I am curious. I noticed in master you used DateTimeFormatter. The minimum
>> version for Log4j 2.x is now Java 8. Why didn’t you use it here too?
>> 
>> Ralph
>> 
>>> On Mar 4, 2021, at 3:56 AM, vy@apache.org wrote:
>>> 
>>> This is an automated email from the ASF dual-hosted git repository.
>>> 
>>> vy pushed a commit to branch release-2.x
>>> in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
>>> 
>>> 
>>> The following commit(s) were added to refs/heads/release-2.x by this
>> push:
>>>    new 8ae3eb8  LOG4J2-2948 Replace HashSet with IdentityHashMap in
>> ParameterFormatter to detect cycles. (#471)
>>> 8ae3eb8 is described below
>>> 
>>> 
>>> 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..1cc3b0c 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
>>> 
>>> 
>>> -    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"));
>>> 
>> 
>> 
>> 



Re: [logging-log4j2] branch release-2.x updated: LOG4J2-2948 Replace HashSet with IdentityHashMap in ParameterFormatter to detect cycles. (#471)

Posted by Volkan Yazıcı <vo...@gmail.com>.
I did neither replace the usage of SimpleDateFormat in release-2.x, nor
implemented myself DateTimeFormatter in master. I have just touched to the
indentation in release-2.x. I deliberately left it out since it wasn't
related to the ticket and I could not invest time into figuring out the
reason for the discrepancy.

On Thu, Mar 4, 2021 at 4:48 PM Ralph Goers <ra...@dslextreme.com>
wrote:

> I am curious. I noticed in master you used DateTimeFormatter. The minimum
> version for Log4j 2.x is now Java 8. Why didn’t you use it here too?
>
> Ralph
>
> > On Mar 4, 2021, at 3:56 AM, vy@apache.org wrote:
> >
> > This is an automated email from the ASF dual-hosted git repository.
> >
> > vy pushed a commit to branch release-2.x
> > in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
> >
> >
> > The following commit(s) were added to refs/heads/release-2.x by this
> push:
> >     new 8ae3eb8  LOG4J2-2948 Replace HashSet with IdentityHashMap in
> ParameterFormatter to detect cycles. (#471)
> > 8ae3eb8 is described below
> >
> >
> > 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..1cc3b0c 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
> >
> >
> > -    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"));
> >
>
>
>

Re: [logging-log4j2] branch release-2.x updated: LOG4J2-2948 Replace HashSet with IdentityHashMap in ParameterFormatter to detect cycles. (#471)

Posted by Ralph Goers <ra...@dslextreme.com>.
I am curious. I noticed in master you used DateTimeFormatter. The minimum version for Log4j 2.x is now Java 8. Why didn’t you use it here too?

Ralph

> On Mar 4, 2021, at 3:56 AM, vy@apache.org wrote:
> 
> This is an automated email from the ASF dual-hosted git repository.
> 
> vy pushed a commit to branch release-2.x
> in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
> 
> 
> The following commit(s) were added to refs/heads/release-2.x by this push:
>     new 8ae3eb8  LOG4J2-2948 Replace HashSet with IdentityHashMap in ParameterFormatter to detect cycles. (#471)
> 8ae3eb8 is described below
> 
> 
> 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..1cc3b0c 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
> 
> 
> -    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"));
>