From 53e2b7690972f1943ef802252a137cb3d5ec82c1 Mon Sep 17 00:00:00 2001 From: Ramanathan Date: Sun, 31 May 2026 15:55:22 +0530 Subject: [PATCH 1/5] Fix circular reference detection for exceptions with colliding equals/hashCode implementations --- .../test/impl/ThrowableCollisionTest.java | 95 +++++++++++++++++++ .../log4j/core/impl/ThrowableProxy.java | 12 ++- .../pattern/ThrowableStackTraceRenderer.java | 16 ++-- 3 files changed, 113 insertions(+), 10 deletions(-) create mode 100644 log4j-core-test/src/main/java/org/apache/logging/log4j/core/test/impl/ThrowableCollisionTest.java diff --git a/log4j-core-test/src/main/java/org/apache/logging/log4j/core/test/impl/ThrowableCollisionTest.java b/log4j-core-test/src/main/java/org/apache/logging/log4j/core/test/impl/ThrowableCollisionTest.java new file mode 100644 index 00000000000..b14237732d6 --- /dev/null +++ b/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!"); + }); + } +} diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ThrowableProxy.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ThrowableProxy.java index 61d292dc487..eb05610f5bd 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ThrowableProxy.java +++ b/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 causeVisited = new HashSet<>(1); + final Set causeVisited = Collections.newSetFromMap(new IdentityHashMap<>(1)); + final Set 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); } /** diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/ThrowableStackTraceRenderer.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/ThrowableStackTraceRenderer.java index 4d210213213..c6ab8e22e05 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/ThrowableStackTraceRenderer.java +++ b/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 metadataByThrowable = Context.Metadata.ofThrowable(throwable); + final Map 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 ofThrowable(final Throwable throwable) { - final Map metadataByThrowable = new HashMap<>(); - populateMetadata(metadataByThrowable, new HashSet<>(), null, throwable); + final Map metadataByThrowable = new IdentityHashMap<>(); + populateMetadata( + metadataByThrowable, Collections.newSetFromMap(new IdentityHashMap<>()), null, throwable); return metadataByThrowable; } From c9c4173e1e62c86de1279d8a5b85bac667ff0629 Mon Sep 17 00:00:00 2001 From: Ramanathan Date: Sun, 31 May 2026 15:59:54 +0530 Subject: [PATCH 2/5] Add changelog entry for circular reference detection fix with colliding equals/hashCode exceptions --- ...ence-detection-for-exceptions-with-colliding.xml | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 src/changelog/.2.x.x/fix-circular-reference-detection-for-exceptions-with-colliding.xml diff --git a/src/changelog/.2.x.x/fix-circular-reference-detection-for-exceptions-with-colliding.xml b/src/changelog/.2.x.x/fix-circular-reference-detection-for-exceptions-with-colliding.xml new file mode 100644 index 00000000000..b5a4415d214 --- /dev/null +++ b/src/changelog/.2.x.x/fix-circular-reference-detection-for-exceptions-with-colliding.xml @@ -0,0 +1,13 @@ + + + + + + Fix `circular reference` detection for exceptions with `colliding` equals/hashCode implementations + + From c65c142789363334a27230df4efe7476f20b8f86 Mon Sep 17 00:00:00 2001 From: Ramanathan Date: Sun, 31 May 2026 16:00:56 +0530 Subject: [PATCH 3/5] Add changelog entry for circular reference detection fix with colliding equals/hashCode exceptions --- ...ular-reference-detection-for-exceptions-with-colliding.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/changelog/.2.x.x/fix-circular-reference-detection-for-exceptions-with-colliding.xml b/src/changelog/.2.x.x/fix-circular-reference-detection-for-exceptions-with-colliding.xml index b5a4415d214..f3ac980e313 100644 --- a/src/changelog/.2.x.x/fix-circular-reference-detection-for-exceptions-with-colliding.xml +++ b/src/changelog/.2.x.x/fix-circular-reference-detection-for-exceptions-with-colliding.xml @@ -5,8 +5,8 @@ https://logging.apache.org/xml/ns https://logging.apache.org/xml/ns/log4j-changelog-0.xsd" type="changed"> - - + + Fix `circular reference` detection for exceptions with `colliding` equals/hashCode implementations From 377a429183bb2e9cd1317bd86a16520d32c0cad4 Mon Sep 17 00:00:00 2001 From: Ramanathan Date: Tue, 2 Jun 2026 03:13:37 +0530 Subject: [PATCH 4/5] Add test for ThrowableProxy serialization with colliding equals/hashCode implementations --- .../test/impl/ThrowableCollisionTest.java | 95 ------------------- .../ThrowablePatternConverterTest.java | 46 +++++++++ .../log4j/core/impl/ThrowableProxy.java | 2 + .../pattern/ThrowableStackTraceRenderer.java | 6 ++ 4 files changed, 54 insertions(+), 95 deletions(-) delete mode 100644 log4j-core-test/src/main/java/org/apache/logging/log4j/core/test/impl/ThrowableCollisionTest.java diff --git a/log4j-core-test/src/main/java/org/apache/logging/log4j/core/test/impl/ThrowableCollisionTest.java b/log4j-core-test/src/main/java/org/apache/logging/log4j/core/test/impl/ThrowableCollisionTest.java deleted file mode 100644 index b14237732d6..00000000000 --- a/log4j-core-test/src/main/java/org/apache/logging/log4j/core/test/impl/ThrowableCollisionTest.java +++ /dev/null @@ -1,95 +0,0 @@ -/* - * 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!"); - }); - } -} diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/pattern/ThrowablePatternConverterTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/pattern/ThrowablePatternConverterTest.java index 0ce38d03237..c3a82a8b9e6 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/pattern/ThrowablePatternConverterTest.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/pattern/ThrowablePatternConverterTest.java @@ -587,4 +587,50 @@ private static String convert(final String pattern, final Throwable throwable) { } return buffer.toString(); } + + @Test + void testThrowableProxySerializationCollision() throws Exception { + class CollidingException extends RuntimeException { + public CollidingException(String message, Throwable cause) { + super(message, cause); + } + + @Override + public boolean equals(Object obj) { + return obj instanceof CollidingException + && java.util.Objects.equals(getMessage(), ((CollidingException) obj).getMessage()); + } + + @Override + public int hashCode() { + return java.util.Objects.hashCode(getMessage()); + } + } + + Throwable inner = new CollidingException("collision", null); + Throwable middle = new CollidingException("collision", inner); + Throwable outer = new CollidingException("collision", middle); + + org.apache.logging.log4j.core.impl.ThrowableProxy proxy = + new org.apache.logging.log4j.core.impl.ThrowableProxy(outer); + + java.io.ByteArrayOutputStream baos = new java.io.ByteArrayOutputStream(); + try (java.io.ObjectOutputStream oos = new java.io.ObjectOutputStream(baos)) { + oos.writeObject(proxy); + } + + org.apache.logging.log4j.core.impl.ThrowableProxy deserializedProxy; + try (java.io.ObjectInputStream ois = + new java.io.ObjectInputStream(new java.io.ByteArrayInputStream(baos.toByteArray()))) { + deserializedProxy = (org.apache.logging.log4j.core.impl.ThrowableProxy) ois.readObject(); + } + + String actualStackTrace = deserializedProxy.getExtendedStackTraceAsString(); + + long count = java.util.Arrays.stream(actualStackTrace.split("\n")) + .filter(line -> line.contains("Caused by:")) + .count(); + + assertThat(count).isEqualTo(2); + } } diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ThrowableProxy.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ThrowableProxy.java index eb05610f5bd..6be228e8edb 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ThrowableProxy.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ThrowableProxy.java @@ -114,6 +114,8 @@ public ThrowableProxy(final Throwable throwable) { this.extendedStackTrace = ThrowableProxyHelper.toExtendedStackTrace(this, stack, map, null, throwable.getStackTrace()); final Throwable throwableCause = throwable.getCause(); + // `IdentityHashMap` is needed to deal with custom `equals()` and `hashCode()` implementations causing + // collisions final Set causeVisited = Collections.newSetFromMap(new IdentityHashMap<>(1)); final Set suppressedVisited = visited == null ? Collections.newSetFromMap(new IdentityHashMap<>()) : visited; diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/ThrowableStackTraceRenderer.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/ThrowableStackTraceRenderer.java index c6ab8e22e05..b23c7c30f85 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/ThrowableStackTraceRenderer.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/ThrowableStackTraceRenderer.java @@ -53,6 +53,8 @@ public final void renderThrowable( if (maxLineCount > 0) { try { C context = createContext(throwable); + // `IdentityHashMap` is needed to deal with custom `equals()` and `hashCode()` implementations causing + // collisions renderThrowable( buffer, throwable, context, Collections.newSetFromMap(new IdentityHashMap<>()), lineSeparator); } catch (final Exception error) { @@ -65,6 +67,8 @@ public final void renderThrowable( @SuppressWarnings("unchecked") C createContext(final Throwable throwable) { + // `IdentityHashMap` is needed to deal with custom `equals()` and `hashCode()` implementations causing + // collisions final Map metadataByThrowable = new IdentityHashMap<>(); Context.Metadata.populateMetadata( metadataByThrowable, Collections.newSetFromMap(new IdentityHashMap<>()), null, throwable); @@ -295,6 +299,8 @@ private Metadata( } static Map ofThrowable(final Throwable throwable) { + // `IdentityHashMap` is needed to deal with custom `equals()` and `hashCode()` implementations causing + // collisions final Map metadataByThrowable = new IdentityHashMap<>(); populateMetadata( metadataByThrowable, Collections.newSetFromMap(new IdentityHashMap<>()), null, throwable); From 8ddbc1b6dbe4709eea2f4e67806caa6dffa94853 Mon Sep 17 00:00:00 2001 From: Ramanathan Date: Tue, 2 Jun 2026 10:46:31 +0530 Subject: [PATCH 5/5] Refactor ThrowableProxy serialization test to use modern Java I/O classes --- .../ThrowablePatternConverterTest.java | 95 ++++++++++--------- 1 file changed, 49 insertions(+), 46 deletions(-) diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/pattern/ThrowablePatternConverterTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/pattern/ThrowablePatternConverterTest.java index c3a82a8b9e6..5dfcaa2f261 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/pattern/ThrowablePatternConverterTest.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/pattern/ThrowablePatternConverterTest.java @@ -21,12 +21,16 @@ import static org.assertj.core.api.Assertions.assertThat; import foo.TestFriendlyException; +import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; +import java.io.ObjectInputStream; +import java.io.ObjectOutputStream; import java.io.PrintStream; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.List; +import java.util.Objects; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -40,6 +44,7 @@ import org.apache.logging.log4j.Level; import org.apache.logging.log4j.core.LogEvent; import org.apache.logging.log4j.core.impl.Log4jLogEvent; +import org.apache.logging.log4j.core.impl.ThrowableProxy; import org.apache.logging.log4j.core.layout.PatternLayout; import org.jspecify.annotations.Nullable; import org.junit.jupiter.api.Nested; @@ -571,6 +576,50 @@ private static List createExceptionsOfDifferentDepths() { }) .collect(Collectors.toList()); } + + @Test + void testThrowableProxySerializationCollision() throws Exception { + 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()); + } + } + + Throwable inner = new CollidingException("collision", null); + Throwable middle = new CollidingException("collision", inner); + Throwable outer = new CollidingException("collision", middle); + + ThrowableProxy proxy = new ThrowableProxy(outer); + + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + try (ObjectOutputStream oos = new ObjectOutputStream(baos)) { + oos.writeObject(proxy); + } + + ThrowableProxy deserializedProxy; + try (ObjectInputStream ois = new ObjectInputStream(new ByteArrayInputStream(baos.toByteArray()))) { + deserializedProxy = (ThrowableProxy) ois.readObject(); + } + + String actualStackTrace = deserializedProxy.getExtendedStackTraceAsString(); + + long count = Arrays.stream(actualStackTrace.split("\n")) + .filter(line -> line.contains("Caused by:")) + .count(); + + assertThat(count).isEqualTo(2); + } } static String convert(final String pattern) { @@ -587,50 +636,4 @@ private static String convert(final String pattern, final Throwable throwable) { } return buffer.toString(); } - - @Test - void testThrowableProxySerializationCollision() throws Exception { - class CollidingException extends RuntimeException { - public CollidingException(String message, Throwable cause) { - super(message, cause); - } - - @Override - public boolean equals(Object obj) { - return obj instanceof CollidingException - && java.util.Objects.equals(getMessage(), ((CollidingException) obj).getMessage()); - } - - @Override - public int hashCode() { - return java.util.Objects.hashCode(getMessage()); - } - } - - Throwable inner = new CollidingException("collision", null); - Throwable middle = new CollidingException("collision", inner); - Throwable outer = new CollidingException("collision", middle); - - org.apache.logging.log4j.core.impl.ThrowableProxy proxy = - new org.apache.logging.log4j.core.impl.ThrowableProxy(outer); - - java.io.ByteArrayOutputStream baos = new java.io.ByteArrayOutputStream(); - try (java.io.ObjectOutputStream oos = new java.io.ObjectOutputStream(baos)) { - oos.writeObject(proxy); - } - - org.apache.logging.log4j.core.impl.ThrowableProxy deserializedProxy; - try (java.io.ObjectInputStream ois = - new java.io.ObjectInputStream(new java.io.ByteArrayInputStream(baos.toByteArray()))) { - deserializedProxy = (org.apache.logging.log4j.core.impl.ThrowableProxy) ois.readObject(); - } - - String actualStackTrace = deserializedProxy.getExtendedStackTraceAsString(); - - long count = java.util.Arrays.stream(actualStackTrace.split("\n")) - .filter(line -> line.contains("Caused by:")) - .count(); - - assertThat(count).isEqualTo(2); - } }