Fix stack trace rendering for exceptions with identity malfunction

log4j-core-test/src/main/java/org/apache/logging/log4j/core/test/impl/ThrowableCollisionTest.java

@@ -0,0 +1,95 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.logging.log4j.core.test.impl;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.Objects;
+import org.apache.logging.log4j.core.impl.ThrowableProxy;
+import org.junit.jupiter.api.Test;
+
+public class ThrowableCollisionTest {
+
+    static class CollidingException extends RuntimeException {
+        public CollidingException(String message, Throwable cause) {
+            super(message, cause);
+        }
+
+        @Override
+        public boolean equals(Object obj) {
+            return obj instanceof CollidingException
+                    && Objects.equals(getMessage(), ((CollidingException) obj).getMessage());
+        }
+
+        @Override
+        public int hashCode() {
+            return Objects.hashCode(getMessage());
+        }
+    }
+
+    static class CyclicException extends RuntimeException {
+        private Throwable customCause;
+
+        public CyclicException(String message) {
+            super(message);
+        }
+
+        public void setCustomCause(Throwable cause) {
+            this.customCause = cause;
+        }
+
+        @Override
+        public Throwable getCause() {
+            return customCause;
+        }
+    }
+
+    @Test
+    public void testCollisionDoesNotTriggerCircularReference() {
+        Throwable inner = new CollidingException("collision", null);
+        Throwable outer = new CollidingException("collision", inner);
+
+        assertDoesNotThrow(() -> {
+            ThrowableProxy proxy = new ThrowableProxy(outer);
+            String trace = proxy.getExtendedStackTraceAsString();
+
+            assertFalse(
+                    trace.contains("CIRCULAR REFERENCE"),
+                    "Should not mark a non-cyclic colliding exception chain as circular!");
+        });
+    }
+
+    @Test
+    public void testTrueCircularReferenceIsStillHandledSafely() {
+        CyclicException ex1 = new CyclicException("Cycle Exception 1");
+        CyclicException ex2 = new CyclicException("Cycle Exception 2");
+
+        ex1.setCustomCause(ex2);
+        ex2.setCustomCause(ex1);
+
+        assertDoesNotThrow(() -> {
+            ThrowableProxy proxy = new ThrowableProxy(ex1);
+            String trace = proxy.getExtendedStackTraceAsString();
+
+            assertTrue(
+                    trace.contains("CIRCULAR REFERENCE"),
+                    "Should successfully detect and flag a genuine cyclic reference!");
+        });
+    }
+}

log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ThrowableProxy.java

@@ -18,9 +18,10 @@
 
 import java.io.Serializable;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.Deque;
 import java.util.HashMap;
-import java.util.HashSet;
+import java.util.IdentityHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -113,11 +114,14 @@ public ThrowableProxy(final Throwable throwable) {
         this.extendedStackTrace =
                 ThrowableProxyHelper.toExtendedStackTrace(this, stack, map, null, throwable.getStackTrace());
         final Throwable throwableCause = throwable.getCause();
-        final Set<Throwable> causeVisited = new HashSet<>(1);
+        final Set<Throwable> causeVisited = Collections.newSetFromMap(new IdentityHashMap<>(1));
+        final Set<Throwable> suppressedVisited =
+                visited == null ? Collections.newSetFromMap(new IdentityHashMap<>()) : visited;
+
         this.causeProxy = throwableCause == null
                 ? null
-                : new ThrowableProxy(throwable, stack, map, throwableCause, visited, causeVisited);
-        this.suppressedProxies = ThrowableProxyHelper.toSuppressedProxies(throwable, visited);
+                : new ThrowableProxy(throwable, stack, map, throwableCause, suppressedVisited, causeVisited);
+        this.suppressedProxies = ThrowableProxyHelper.toSuppressedProxies(throwable, suppressedVisited);
     }
 
     /**

log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/ThrowableStackTraceRenderer.java

@@ -16,8 +16,8 @@
  */
 package org.apache.logging.log4j.core.pattern;
 
-import java.util.HashMap;
-import java.util.HashSet;
+import java.util.Collections;
+import java.util.IdentityHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -53,7 +53,8 @@ public final void renderThrowable(
         if (maxLineCount > 0) {
             try {
                 C context = createContext(throwable);
-                renderThrowable(buffer, throwable, context, new HashSet<>(), lineSeparator);
+                renderThrowable(
+                        buffer, throwable, context, Collections.newSetFromMap(new IdentityHashMap<>()), lineSeparator);
             } catch (final Exception error) {
                 if (error != MAX_LINE_COUNT_EXCEEDED) {
                     throw error;
@@ -64,7 +65,9 @@ public final void renderThrowable(
 
     @SuppressWarnings("unchecked")
     C createContext(final Throwable throwable) {
-        final Map<Throwable, Context.Metadata> metadataByThrowable = Context.Metadata.ofThrowable(throwable);
+        final Map<Throwable, Context.Metadata> metadataByThrowable = new IdentityHashMap<>();
+        Context.Metadata.populateMetadata(
+                metadataByThrowable, Collections.newSetFromMap(new IdentityHashMap<>()), null, throwable);
         return (C) new Context(0, metadataByThrowable);
     }
 
@@ -292,8 +295,9 @@ private Metadata(
             }
 
             static Map<Throwable, Metadata> ofThrowable(final Throwable throwable) {
-                final Map<Throwable, Metadata> metadataByThrowable = new HashMap<>();
-                populateMetadata(metadataByThrowable, new HashSet<>(), null, throwable);
+                final Map<Throwable, Metadata> metadataByThrowable = new IdentityHashMap<>();
+                populateMetadata(
+                        metadataByThrowable, Collections.newSetFromMap(new IdentityHashMap<>()), null, throwable);
                 return metadataByThrowable;
             }
 

src/changelog/.2.x.x/fix-circular-reference-detection-for-exceptions-with-colliding.xml

@@ -0,0 +1,13 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<entry xmlns="https://logging.apache.org/xml/ns"
+       xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+       xsi:schemaLocation="
+           https://logging.apache.org/xml/ns
+           https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
+       type="changed">
+    <issue id="3933" link="https://github.com/apache/logging-log4j2/issues/3933"/>
+    <issue id="4133" link="https://github.com/apache/logging-log4j2/pull/4133"/>
+    <description format="asciidoc">
+        Fix `circular reference` detection for exceptions with `colliding` equals/hashCode implementations
+    </description>
+</entry>