This is an automated email from the ASF dual-hosted git repository.
garydgregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-lang.git
The following commit(s) were added to refs/heads/master by this push:
new ac026ed6e Use structured stack frames for compact root cause traces
ac026ed6e is described below
commit ac026ed6e8892ae7ad93514cdbb47b8c2e3ad799
Author: Gary Gregory <[email protected]>
AuthorDate: Sun Oct 25 09:35:31 2026 -0400
Use structured stack frames for compact root cause traces
Use StackTraceElement objects to build compact root cause stack traces,
avoiding text parsing and preventing exception message text from being
mistaken for stack frames.
Rewrite ExceptionUtils.getRootCauseStackTraceList(Throwable) to read
StackTraceElement arrays directly from Throwable.getStackTrace().
Compare
elements to remove each cause's shared trailing frames, then format only
the retained frames.
This avoids repeatedly printing and parsing complete exception traces.
Frame-shaped exception message text can no longer be mistaken for frames
or suppress real frames. Structured comparisons also preserve distinct
frames whose string representations happen to match.
Preserve root-cause-first ordering, wrapped exception markers, and all
public and protected signatures. Exception messages remain in headers,
but custom printStackTrace() output is no longer used.
Remove the unused getStackFrameList(Throwable) parser and its tests.
Update the related Javadoc to describe structured frame extraction.
Main methods removed (private):
- ExceptionUtils.getStackFrameList(Throwable)
- ExceptionUtils.isStackFrame(String)
Test methods removed:
- ExceptionUtilsTest.testGetStackFrameListAcceptsAllRealFrameShapes()
- ExceptionUtilsTest.testGetStackFrameListRejectsForgedMessageLines()
---
src/changes/changes.xml | 1 +
.../commons/lang3/exception/ExceptionUtils.java | 129 ++++-----------------
.../lang3/exception/ExceptionUtilsTest.java | 128 ++++++++++----------
3 files changed, 87 insertions(+), 171 deletions(-)
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 84a4f428a..f56b09564 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -74,6 +74,7 @@ The <action> type attribute can be add,update,fix,remove.
<action type="fix" dev="ggregory" due-to="alhuda, Gary
Gregory">Fix FastDateParser normalization of out-of-range ISO day-of-week
values (#1839).</action>
<action type="fix" dev="ggregory" due-to="Aminark, Gary
Gregory">[javadoc] Correct StringUtils example methods and overloads
(#1836).</action>
<action type="fix" dev="ggregory" due-to="alhuda, Gary
Gregory">Stop ExceptionUtils.removeCommonFrames at the first differing frame
(#1831).</action>
+ <action type="fix" dev="ggregory" due-to="Gary
Gregory">Use StackTraceElement objects to build compact root cause stack
traces, avoiding text parsing and preventing exception message text from being
mistaken for stack frames.</action>
<!-- ADD -->
<!-- UPDATE -->
<action type="update" dev="ggregory" due-to="Gary Gregory">Bump
org.apache.commons:commons-parent from 105 to 106.</action>
diff --git
a/src/main/java/org/apache/commons/lang3/exception/ExceptionUtils.java
b/src/main/java/org/apache/commons/lang3/exception/ExceptionUtils.java
index 5eedb916a..76554439e 100644
--- a/src/main/java/org/apache/commons/lang3/exception/ExceptionUtils.java
+++ b/src/main/java/org/apache/commons/lang3/exception/ExceptionUtils.java
@@ -27,7 +27,6 @@
import java.util.List;
import java.util.Objects;
import java.util.Set;
-import java.util.StringTokenizer;
import java.util.function.Consumer;
import java.util.stream.Stream;
@@ -357,11 +356,8 @@ public static String getRootCauseMessage(final Throwable
throwable) {
* </p>
*
* <p>
- * <strong>Note:</strong> the frames are recovered by re-parsing the text
produced by {@link Throwable#printStackTrace()}, they are not read from
- * {@link Throwable#getStackTrace()}. A line inside an exception
<em>message</em> that mimics a stack frame (leading whitespace, then {@code "at
"},
- * then a class/method reference with {@code '('}) is indistinguishable
from a real frame: untrusted message content can therefore inject fabricated
- * frames into this output and cause the real frames that follow to be
dropped. Do not treat this output as forensic evidence when exception messages
- * may contain untrusted input; read {@link Throwable#getStackTrace()} for
structured frames that cannot be forged by message content.
+ * Frames are read from {@link Throwable#getStackTrace()} and compared as
{@link StackTraceElement} objects before formatting. Exception messages
+ * remain part of the exception headers, but are not parsed as stack
frames. Custom {@code printStackTrace} output is not used.
* </p>
*
* @param throwable The throwable to examine, may be null.
@@ -381,11 +377,8 @@ public static String[] getRootCauseStackTrace(final
Throwable throwable) {
* </p>
*
* <p>
- * <strong>Note:</strong> the frames are recovered by re-parsing the text
produced by {@link Throwable#printStackTrace()}, they are not read from
- * {@link Throwable#getStackTrace()}. A line inside an exception
<em>message</em> that mimics a stack frame (leading whitespace, then {@code "at
"},
- * then a class/method reference with {@code '('}) is indistinguishable
from a real frame: untrusted message content can therefore inject fabricated
- * frames into this output and cause the real frames that follow to be
dropped. Do not treat this output as forensic evidence when exception messages
- * may contain untrusted input; read {@link Throwable#getStackTrace()} for
structured frames that cannot be forged by message content.
+ * Frames are read from {@link Throwable#getStackTrace()} and compared as
{@link StackTraceElement} objects before formatting. Exception messages
+ * remain part of the exception headers, but are not parsed as stack
frames. Custom {@code printStackTrace} output is not used.
* </p>
*
* @param throwable The throwable to examine, may be null.
@@ -399,51 +392,25 @@ public static List<String>
getRootCauseStackTraceList(final Throwable throwable)
final Throwable[] throwables = getThrowables(throwable);
final int count = throwables.length;
final List<String> frames = new ArrayList<>();
- List<String> nextTrace = getStackFrameList(throwables[count - 1]);
+ StackTraceElement[] nextTrace = throwables[count - 1].getStackTrace();
for (int i = count; --i >= 0;) {
- final List<String> trace = nextTrace;
+ final StackTraceElement[] trace = nextTrace;
+ int frameCount = trace.length;
if (i != 0) {
- nextTrace = getStackFrameList(throwables[i - 1]);
- removeCommonFrames(trace, nextTrace);
+ nextTrace = throwables[i - 1].getStackTrace();
+ int wrapperFrameIndex = nextTrace.length - 1;
+ while (frameCount > 0 && wrapperFrameIndex >= 0 &&
trace[frameCount - 1].equals(nextTrace[wrapperFrameIndex])) {
+ frameCount--;
+ wrapperFrameIndex--;
+ }
}
final String string = throwables[i].toString();
frames.add(i == count - 1 ? string : WRAPPED_MARKER + string);
- frames.addAll(trace);
- }
- return frames;
- }
-
- /**
- * Gets a {@link List} of stack frames, the message
- * is not included. Only the trace of the specified exception is
- * returned, any caused by trace is stripped.
- *
- * <p>
- * This works by re-parsing the text produced by {@link
Throwable#printStackTrace()}: a line is treated as a frame if, after leading
- * whitespace, it starts with {@code "at "} followed by a class/method
reference and {@code '('} (see {@link #isStackFrame(String)}). It
- * will mis-parse if the exception message contains a line of exactly that
shape: such a line is counted as a frame and the real frames
- * that follow the remaining message lines are dropped.
- * </p>
- *
- * @param throwable is any throwable.
- * @return List of stack frames.
- */
- static List<String> getStackFrameList(final Throwable throwable) {
- final String stackTrace = getStackTrace(throwable);
- final String linebreak = System.lineSeparator();
- final StringTokenizer frames = new StringTokenizer(stackTrace,
linebreak);
- final List<String> list = new ArrayList<>();
- boolean traceStarted = false;
- while (frames.hasMoreTokens()) {
- final String token = frames.nextToken();
- if (isStackFrame(token)) {
- traceStarted = true;
- list.add(token);
- } else if (traceStarted) {
- break;
+ for (int j = 0; j < frameCount; j++) {
+ frames.add("\tat " + trace[j]);
}
}
- return list;
+ return frames;
}
/**
@@ -732,49 +699,6 @@ public static boolean isChecked(final Throwable throwable)
{
return throwable != null && !(throwable instanceof Error) &&
!(throwable instanceof RuntimeException);
}
- /**
- * Tests whether a line from {@link #getStackTrace(Throwable)} output
looks like a stack frame, mirroring the syntax emitted by
- * {@link Throwable#printStackTrace()}: leading whitespace, then {@code
"at "}, then a class/method reference containing no whitespace,
- * then {@code '('}, for example {@code "\tat
com.example.Foo.bar(Foo.java:42)"}. The reference is matched as any non-empty
run of
- * non-whitespace characters, because {@link StackTraceElement#toString()}
never emits whitespace before the opening parenthesis: this
- * accepts classic frames as well as class loader or module prefixes
({@code "app//"}, {@code "java.base/"}), module versions
- * ({@code "[email protected]/"}), lambda and hidden-class names ({@code
"$$Lambda$17/0x..."}), {@code <init>}/{@code <clinit>} and
- * JVM-language name mangling, without maintaining a character whitelist
that could reject a legitimate frame (and thereby suppress
- * it and every frame below it).
- *
- * <p>
- * This is deliberately stricter than matching any line whose first
non-whitespace characters are {@code "at"}, so that ordinary
- * message text such as {@code " attack detected"} or {@code "at your
request"} is not mistaken for a frame; a message line crafted to
- * match the full frame syntax is still indistinguishable from a real
frame.
- * </p>
- *
- * @param token one line of printed stack trace text.
- * @return whether the line has the syntax of a printed stack frame.
- */
- private static boolean isStackFrame(final String token) {
- int i = 0;
- final int len = token.length();
- while (i < len && Character.isWhitespace(token.charAt(i))) {
- i++;
- }
- // Frames printed by Throwable are indented: require leading
whitespace, then "at ".
- if (i == 0 || !token.startsWith("at ", i)) {
- return false;
- }
- i += 3;
- final int paren = token.indexOf('(', i);
- if (paren <= i) {
- return false;
- }
- // StackTraceElement.toString() never emits whitespace between "at "
and '(': any whitespace there means message text.
- for (int j = i; j < paren; j++) {
- if (Character.isWhitespace(token.charAt(j))) {
- return false;
- }
- }
- return true;
- }
-
/**
* Tests whether a throwable represents an unchecked exception.
*
@@ -805,11 +729,8 @@ public static boolean isUnchecked(final Throwable
throwable) {
* </p>
*
* <p>
- * <strong>Note:</strong> the frames are recovered by re-parsing the text
produced by {@link Throwable#printStackTrace()}, they are not read from
- * {@link Throwable#getStackTrace()}. A line inside an exception
<em>message</em> that mimics a stack frame (leading whitespace, then {@code "at
"},
- * then a class/method reference with {@code '('}) is indistinguishable
from a real frame: untrusted message content can therefore inject fabricated
- * frames into this output and cause the real frames that follow to be
dropped. Do not treat this output as forensic evidence when exception messages
- * may contain untrusted input; read {@link Throwable#getStackTrace()} for
structured frames that cannot be forged by message content.
+ * Frames are read from {@link Throwable#getStackTrace()} and compared as
{@link StackTraceElement} objects before formatting. Exception messages
+ * remain part of the exception headers, but are not parsed as stack
frames. Custom {@code printStackTrace} output is not used.
* </p>
*
* @param throwable The throwable to output.
@@ -840,11 +761,8 @@ public static void printRootCauseStackTrace(final
Throwable throwable) {
* </p>
*
* <p>
- * <strong>Note:</strong> the frames are recovered by re-parsing the text
produced by {@link Throwable#printStackTrace()}, they are not read from
- * {@link Throwable#getStackTrace()}. A line inside an exception
<em>message</em> that mimics a stack frame (leading whitespace, then {@code "at
"},
- * then a class/method reference with {@code '('}) is indistinguishable
from a real frame: untrusted message content can therefore inject fabricated
- * frames into this output and cause the real frames that follow to be
dropped. Do not treat this output as forensic evidence when exception messages
- * may contain untrusted input; read {@link Throwable#getStackTrace()} for
structured frames that cannot be forged by message content.
+ * Frames are read from {@link Throwable#getStackTrace()} and compared as
{@link StackTraceElement} objects before formatting. Exception messages
+ * remain part of the exception headers, but are not parsed as stack
frames. Custom {@code printStackTrace} output is not used.
* </p>
*
* @param throwable The throwable to output, may be null.
@@ -883,11 +801,8 @@ public static void printRootCauseStackTrace(final
Throwable throwable, final Pri
* </p>
*
* <p>
- * <strong>Note:</strong> the frames are recovered by re-parsing the text
produced by {@link Throwable#printStackTrace()}, they are not read from
- * {@link Throwable#getStackTrace()}. A line inside an exception
<em>message</em> that mimics a stack frame (leading whitespace, then {@code "at
"},
- * then a class/method reference with {@code '('}) is indistinguishable
from a real frame: untrusted message content can therefore inject fabricated
- * frames into this output and cause the real frames that follow to be
dropped. Do not treat this output as forensic evidence when exception messages
- * may contain untrusted input; read {@link Throwable#getStackTrace()} for
structured frames that cannot be forged by message content.
+ * Frames are read from {@link Throwable#getStackTrace()} and compared as
{@link StackTraceElement} objects before formatting. Exception messages
+ * remain part of the exception headers, but are not parsed as stack
frames. Custom {@code printStackTrace} output is not used.
* </p>
*
* @param throwable The throwable to output, may be null.
diff --git
a/src/test/java/org/apache/commons/lang3/exception/ExceptionUtilsTest.java
b/src/test/java/org/apache/commons/lang3/exception/ExceptionUtilsTest.java
index 12d9dbf26..4a210a708 100644
--- a/src/test/java/org/apache/commons/lang3/exception/ExceptionUtilsTest.java
+++ b/src/test/java/org/apache/commons/lang3/exception/ExceptionUtilsTest.java
@@ -20,6 +20,7 @@
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNotSame;
import static org.junit.jupiter.api.Assertions.assertNull;
@@ -411,6 +412,69 @@ void testGetRootCauseStackTrace_Throwable() {
assertFalse(match);
}
+ @Test
+ void testGetRootCauseStackTraceListComparesElements() {
+ final StackTraceElement causeFrame = new StackTraceElement("Type",
"call", "Type.java", -1);
+ final StackTraceElement wrapperFrame = new StackTraceElement("Type",
"call", "Type.java", -3);
+ assertEquals(causeFrame.toString(), wrapperFrame.toString());
+ assertNotEquals(causeFrame, wrapperFrame);
+ final Throwable cause = new Throwable("cause");
+ final Throwable wrapper = new Throwable("wrapper", cause);
+ cause.setStackTrace(new StackTraceElement[] {causeFrame});
+ wrapper.setStackTrace(new StackTraceElement[] {wrapperFrame});
+ assertEquals(Arrays.asList(cause.toString(), "\tat " + causeFrame,
ExceptionUtils.WRAPPED_MARKER + wrapper, "\tat " + wrapperFrame),
+ ExceptionUtils.getRootCauseStackTraceList(wrapper));
+ }
+
+ @Test
+ void testGetRootCauseStackTraceListEmptyFrames() {
+ final Throwable cause = new Throwable("cause");
+ final Throwable wrapper = new Throwable("wrapper", cause);
+ cause.setStackTrace(new StackTraceElement[0]);
+ wrapper.setStackTrace(new StackTraceElement[0]);
+ assertEquals(Arrays.asList(cause.toString(),
ExceptionUtils.WRAPPED_MARKER + wrapper),
+ ExceptionUtils.getRootCauseStackTraceList(wrapper));
+ }
+
+ @Test
+ void testGetRootCauseStackTraceListFrameShapedMessage() {
+ final Throwable throwable = new Throwable("denied\n\tat
forged.Type.call(Type.java:42)\nmore text");
+ final StackTraceElement frame = new StackTraceElement("real.Type",
"call", "Type.java", 7);
+ throwable.setStackTrace(new StackTraceElement[] {frame});
+ assertEquals(Arrays.asList(throwable.toString(), "\tat " + frame),
ExceptionUtils.getRootCauseStackTraceList(throwable));
+ }
+
+ @Test
+ void testGetRootCauseStackTraceListIgnoresCustomPrintStackTrace() {
+ final Throwable throwable = new Throwable("message") {
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ public void printStackTrace(final PrintWriter writer) {
+ throw new AssertionError("Must use structured stack frames");
+ }
+ };
+ final StackTraceElement frame = new StackTraceElement("real.Type",
"call", null, -2);
+ throwable.setStackTrace(new StackTraceElement[] {frame});
+ assertEquals(Arrays.asList(throwable.toString(), "\tat " + frame),
ExceptionUtils.getRootCauseStackTraceList(throwable));
+ }
+
+ @Test
+ void testGetRootCauseStackTraceListSharedSuffix() {
+ final StackTraceElement shared = new StackTraceElement("Shared",
"call", "Shared.java", 1);
+ final StackTraceElement causeOnly = new StackTraceElement("Cause",
"call", "Cause.java", 2);
+ final StackTraceElement wrapperOnly = new StackTraceElement("Wrapper",
"call", "Wrapper.java", 3);
+ final Throwable cause = new Throwable("cause");
+ final Throwable wrapper = new Throwable("wrapper", cause);
+ cause.setStackTrace(new StackTraceElement[] {shared, causeOnly,
shared});
+ wrapper.setStackTrace(new StackTraceElement[] {shared, wrapperOnly,
shared});
+ assertEquals(Arrays.asList(cause.toString(), "\tat " + shared, "\tat "
+ causeOnly,
+ ExceptionUtils.WRAPPED_MARKER + wrapper, "\tat " + shared,
"\tat " + wrapperOnly, "\tat " + shared),
+ ExceptionUtils.getRootCauseStackTraceList(wrapper));
+ assertEquals(3, cause.getStackTrace().length);
+ assertEquals(3, wrapper.getStackTrace().length);
+ }
+
@Test
void testGetRootCauseStackTraceList_Throwable() {
assertEquals(0,
ExceptionUtils.getRootCauseStackTraceList(null).size());
@@ -462,70 +526,6 @@ void
testGetRootCauseStackTraceMessageLinesNotMistakenForFrames() {
assertTrue(sawRealFrame, "real frames were suppressed");
}
- /**
- * Tests that every frame shape {@code StackTraceElement.toString()} can
emit is accepted by the tightened frame matcher,
- * in particular JDK 9+ module-versioned frames ({@code
mod@version/pkg.Class}), which a character whitelist without
- * {@code '@'} would reject — silently dropping that frame and every real
frame below it. Uses a throwable that prints a
- * fixed trace so the shapes are deterministic without constructing
module-versioned {@link StackTraceElement}s.
- */
- @Test
- void testGetStackFrameListAcceptsAllRealFrameShapes() {
- final String[] frames = {
- "\tat com.example.Foo.bar(Foo.java:42)",
// classic
- "\tat app//com.foo.Main.main(Main.java:10)",
// class loader prefix
- "\tat [email protected]/com.foo.Helper.help(Helper.java:7)",
// module name @ version
- "\tat java.base/java.lang.Thread.run(Thread.java:833)",
// module, no version
- "\tat com.foo.Main$$Lambda$17/0x0000000800c02a48.run(Unknown
Source)", // lambda / hidden class
- "\tat com.example.Foo.<init>(Foo.java:5)",
// constructor
- "\tat java.base/java.lang.Object.wait(Native Method)"
// native
- };
- final StringBuilder text = new
StringBuilder("java.lang.RuntimeException:
boom").append(System.lineSeparator());
- for (final String frame : frames) {
- text.append(frame).append(System.lineSeparator());
- }
- final Throwable fixed = new RuntimeException("boom") {
- private static final long serialVersionUID = 1L;
-
- @Override
- public void printStackTrace(final PrintWriter writer) {
- writer.print(text);
- }
- };
- final List<String> list = ExceptionUtils.getStackFrameList(fixed);
- assertEquals(Arrays.asList(frames), list, "a legitimate frame shape
was rejected (and frames below it dropped)");
- }
-
- /**
- * Tests that message text is still rejected by the frame matcher: forged
lines lacking the no-whitespace-before-'('
- * frame syntax must not start or extend the frame list.
- */
- @Test
- void testGetStackFrameListRejectsForgedMessageLines() {
- final String[] forged = {
- " attack detected", // "at" not
followed by space-delimited reference
- "at your request, more text", // no leading
whitespace
- "\tat your request, more text", // no '(' at all
- "\tat forged frame entry(Evil.java:1)", // whitespace
between "at " and '('
- "\tat (Evil.java:1)" // empty reference
- };
- final StringBuilder text = new
StringBuilder("java.lang.RuntimeException:
boom").append(System.lineSeparator());
- for (final String line : forged) {
- text.append(line).append(System.lineSeparator());
- }
- text.append("\tat
com.example.Foo.bar(Foo.java:42)").append(System.lineSeparator());
- final Throwable fixed = new RuntimeException("boom") {
- private static final long serialVersionUID = 1L;
-
- @Override
- public void printStackTrace(final PrintWriter writer) {
- writer.print(text);
- }
- };
- final List<String> list = ExceptionUtils.getStackFrameList(fixed);
- assertEquals(Arrays.asList("\tat com.example.Foo.bar(Foo.java:42)"),
list,
- "forged message text was classified as a stack frame");
- }
-
@Test
/** getStackFrames returns empty string array when the argument is null */
void testgetStackFramesHappyPath() {
