This is an automated email from the ASF dual-hosted git repository. ramanathan1504 pushed a commit to branch release/2.26.1 in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit 39517e0656f9dc12fc2a0965ea8708da13dc79d8 Author: Ramanathan <[email protected]> AuthorDate: Thu Jun 11 17:45:03 2026 +0530 Fix stack trace rendering for exceptions with identity malfunction (#4133) * Fix circular reference detection for exceptions with colliding equals/hashCode implementations * Add changelog entry for circular reference detection fix with colliding equals/hashCode exceptions * Add changelog entry for circular reference detection fix with colliding equals/hashCode exceptions * Add test for ThrowableProxy serialization with colliding equals/hashCode implementations * Refactor ThrowableProxy serialization test to use modern Java I/O classes * Use `TestFriendlyException` to exercise the malfunction * Update changelog * Improve comments * Remove redundant change --------- Co-authored-by: Volkan Yazıcı <[email protected]> --- .../src/test/java/foo/TestFriendlyException.java | 76 ++++++++++++++++++++-- .../logging/log4j/core/impl/ThrowableProxy.java | 14 ++-- .../core/pattern/ThrowableStackTraceRenderer.java | 16 +++-- ...nce-detection-for-exceptions-with-colliding.xml | 13 ++++ 4 files changed, 104 insertions(+), 15 deletions(-) diff --git a/log4j-core-test/src/test/java/foo/TestFriendlyException.java b/log4j-core-test/src/test/java/foo/TestFriendlyException.java index 7c791dc20d..c024db10ae 100644 --- a/log4j-core-test/src/test/java/foo/TestFriendlyException.java +++ b/log4j-core-test/src/test/java/foo/TestFriendlyException.java @@ -20,6 +20,12 @@ import static org.assertj.core.api.Assertions.assertThat; import java.net.Socket; import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.IdentityHashMap; +import java.util.LinkedList; +import java.util.Queue; +import java.util.Set; import java.util.stream.Stream; import org.apache.logging.log4j.util.Constants; @@ -33,6 +39,7 @@ import org.apache.logging.log4j.util.Constants; * <li>Suppressed exceptions</li> * <li>Clutter-free stack trace (i.e., elements from JUnit, JDK, etc.)</li> * <li>Stack trace elements from named modules<sup>3</sup></li> + * <li>Exceptions with malfunctioning (e.g., colliding) {@link Object#equals(Object) equals()} and {@link Object#hashCode() hashCode()} implementations in the causal chain</li> * </ul> * <p> * <sup>1</sup> Helps with observing stack trace manipulation effects of Log4j. @@ -80,20 +87,63 @@ public final class TestFriendlyException extends RuntimeException { "java.lang", "jdk.internal", "org.junit", "sun.reflect" }; - public static final TestFriendlyException INSTANCE = create("r", 0, 2, new boolean[] {false}, new boolean[] {true}); + public static final TestFriendlyException INSTANCE = + create("r", 0, 2, new boolean[] {false}, new boolean[] {true}, new int[] {5}); + + static { + ensureIdentityMalfunctionAtDifferentDepths(); + } + + /** + * Ensure we have identity malfunctioning exceptions that have different stack trace lengths. + * + * @see <a href="https://github.com/apache/logging-log4j2/issues/3933";>#3933</a> + */ + private static void ensureIdentityMalfunctionAtDifferentDepths() { + final Set<Throwable> visitedExceptions = Collections.newSetFromMap(new IdentityHashMap<>()); + final Set<Integer> identityMalfunctioningExceptionStackTraceDepths = new HashSet<>(); + final Queue<TestFriendlyException> exceptions = new LinkedList<>(); + exceptions.add(INSTANCE); + while (!exceptions.isEmpty()) { + + // Process the exception + final TestFriendlyException exception = exceptions.remove(); + if (!visitedExceptions.add(exception) || !exception.identityMalfunctioning) { + continue; + } + identityMalfunctioningExceptionStackTraceDepths.add(exception.getStackTrace().length); + + // Enqueue the cause + final TestFriendlyException cause = (TestFriendlyException) exception.getCause(); + if (cause != null) { + exceptions.add(cause); + } + + // Enqueue the suppressed + for (final Throwable suppressed : exception.getSuppressed()) { + exceptions.add((TestFriendlyException) suppressed); + } + } + assertThat(identityMalfunctioningExceptionStackTraceDepths) + .describedAs("# of visited exceptions = %s", visitedExceptions.size()) + .hasSizeGreaterThan(1); + } private static TestFriendlyException create( final String name, final int depth, final int maxDepth, final boolean[] circular, - final boolean[] namedModuleAllowed) { - final TestFriendlyException error = new TestFriendlyException(name, namedModuleAllowed); + final boolean[] namedModuleAllowed, + final int[] maxIdentityMalfunctionCount) { + final TestFriendlyException error = + new TestFriendlyException(name, namedModuleAllowed, maxIdentityMalfunctionCount); if (depth < maxDepth) { - final TestFriendlyException cause = create(name + "_c", depth + 1, maxDepth, circular, namedModuleAllowed); + final TestFriendlyException cause = + create(name + "_c", depth + 1, maxDepth, circular, namedModuleAllowed, maxIdentityMalfunctionCount); error.initCause(cause); final TestFriendlyException suppressed = - create(name + "_s", depth + 1, maxDepth, circular, namedModuleAllowed); + create(name + "_s", depth + 1, maxDepth, circular, namedModuleAllowed, maxIdentityMalfunctionCount); error.addSuppressed(suppressed); final boolean circularAllowed = depth + 1 == maxDepth && !circular[0]; if (circularAllowed) { @@ -105,8 +155,12 @@ public final class TestFriendlyException extends RuntimeException { return error; } - private TestFriendlyException(final String message, final boolean[] namedModuleAllowed) { + private final boolean identityMalfunctioning; + + private TestFriendlyException( + final String message, final boolean[] namedModuleAllowed, final int[] maxIdentityMalfunctionCount) { super(message); + this.identityMalfunctioning = --maxIdentityMalfunctionCount[0] > 0; removeExcludedStackTraceElements(namedModuleAllowed); } @@ -171,4 +225,14 @@ public final class TestFriendlyException extends RuntimeException { public String getLocalizedMessage() { return getMessage() + " [localized]"; } + + @Override + public int hashCode() { + return identityMalfunctioning ? 0 : super.hashCode(); + } + + @Override + public boolean equals(Object obj) { + return identityMalfunctioning || super.equals(obj); + } } 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 61d292dc48..d051a4fc20 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 @@ package org.apache.logging.log4j.core.impl; 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,16 @@ public class ThrowableProxy implements Serializable { this.extendedStackTrace = ThrowableProxyHelper.toExtendedStackTrace(this, stack, map, null, throwable.getStackTrace()); final Throwable throwableCause = throwable.getCause(); - final Set<Throwable> causeVisited = new HashSet<>(1); + // `IdentityHashMap` is needed for exceptions with identity malfunction. + // Consider `equals()` and `hashCode()` implementations causing collisions. + 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); } /** 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 4d21021321..cafb2a2a93 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,10 @@ class ThrowableStackTraceRenderer<C extends ThrowableStackTraceRenderer.Context> if (maxLineCount > 0) { try { C context = createContext(throwable); - renderThrowable(buffer, throwable, context, new HashSet<>(), lineSeparator); + // `IdentityHashMap` is needed for exceptions with identity malfunction. + // Consider `equals()` and `hashCode()` implementations causing collisions. + final Set<Throwable> visitedThrowables = Collections.newSetFromMap(new IdentityHashMap<>()); + renderThrowable(buffer, throwable, context, visitedThrowables, lineSeparator); } catch (final Exception error) { if (error != MAX_LINE_COUNT_EXCEEDED) { throw error; @@ -292,8 +295,11 @@ class ThrowableStackTraceRenderer<C extends ThrowableStackTraceRenderer.Context> } static Map<Throwable, Metadata> ofThrowable(final Throwable throwable) { - final Map<Throwable, Metadata> metadataByThrowable = new HashMap<>(); - populateMetadata(metadataByThrowable, new HashSet<>(), null, throwable); + // `IdentityHashMap` is needed for exceptions with identity malfunction. + // Consider `equals()` and `hashCode()` implementations causing collisions. + final Map<Throwable, Metadata> metadataByThrowable = new IdentityHashMap<>(); + final Set<Throwable> visitedThrowables = Collections.newSetFromMap(new IdentityHashMap<>()); + populateMetadata(metadataByThrowable, visitedThrowables, null, throwable); return metadataByThrowable; } 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 0000000000..1633dcdb8a --- /dev/null +++ b/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 stack trace rendering for exceptions with identity malfunction (e.g., colliding `equals()` and/or `hashCode()` implementations) + </description> +</entry>
