Skip to content

Commit dea87ba

Browse files
theRizwanclaude
authored and
Calamari
committed
Fix quadratic parsing of leading whitespace
`geckoRe` and `javaScriptCoreRe` began with `^\s*` followed immediately by a group that also matches whitespace — `(.*?)` and `([^@]*)` respectively. A leading run of spaces could therefore be divided between the two in n ways, and a line that did not match forced the engine through every split: n= 1000 4.2 ms n= 2000 14.6 ms n= 4000 57.9 ms n= 8000 231.3 ms n=16000 924.0 ms `err.stack` embeds the message, so attacker-influenced text reaches this directly: a 30k-space message took ~3.3s of blocked event loop. The leading `\s*` is redundant, since the following group already matches whitespace. Its only real effect was keeping leading whitespace out of the captured methodName, so that is handled where the capture is read. After: 8000 -> 0.16ms, 16000 -> 0.37ms, 32000 -> 0.63ms, ~2x per doubling. `chromeRe`, `winjsRe` and `nodeRe` are untouched: each has a literal `at ` after `^\s*`, which anchors the boundary and prevents the ambiguity. Co-authored-by: Claude Code <[email protected]>
1 parent 68b3947 commit dea87ba

2 files changed

Lines changed: 28 additions & 4 deletions

File tree

‎src/stack-trace-parser.js‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ function parseWinjs(line) {
7171
};
7272
}
7373

74-
const geckoRe = /^\s*(.*?)(?:\((.*?)\))?(?:^|@)((?:file|https?|blob|chrome|webpack|rsc|resource|\[native).*?|[^@]*bundle)(?::(\d+))?(?::(\d+))?\s*$/i;
74+
const geckoRe = /^(.*?)(?:\((.*?)\))?(?:^|@)((?:file|https?|blob|chrome|webpack|rsc|resource|\[native).*?|[^@]*bundle)(?::(\d+))?(?::(\d+))?\s*$/i;
7575
const geckoEvalRe = /(\S+) line (\d+)(?: > eval line \d+)* > eval/i;
7676

7777
function parseGecko(line) {
@@ -93,14 +93,14 @@ function parseGecko(line) {
9393

9494
return {
9595
file: parts[3],
96-
methodName: parts[1] || UNKNOWN_FUNCTION,
96+
methodName: (parts[1] && parts[1].trim()) || UNKNOWN_FUNCTION,
9797
arguments: parts[2] ? parts[2].split(',') : [],
9898
lineNumber: parts[4] ? +parts[4] : null,
9999
column: parts[5] ? +parts[5] : null,
100100
};
101101
}
102102

103-
const javaScriptCoreRe = /^\s*(?:([^@]*)(?:\((.*?)\))?@)?(\S.*?):(\d+)(?::(\d+))?\s*$/i;
103+
const javaScriptCoreRe = /^(?:([^@]*)(?:\((.*?)\))?@)?(\S.*?):(\d+)(?::(\d+))?\s*$/i;
104104

105105
function parseJSC(line) {
106106
const parts = javaScriptCoreRe.exec(line);
@@ -111,7 +111,7 @@ function parseJSC(line) {
111111

112112
return {
113113
file: parts[3],
114-
methodName: parts[1] || UNKNOWN_FUNCTION,
114+
methodName: (parts[1] && parts[1].trim()) || UNKNOWN_FUNCTION,
115115
arguments: [],
116116
lineNumber: +parts[4],
117117
column: parts[5] ? +parts[5] : null,

‎test/stack-trace-parser.spec.js‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,30 @@ import * as stackTraceParser from '../src';
33
import CapturedExceptions from './fixtures/captured-errors';
44

55
describe('stackTraceParser', () => {
6+
// `geckoRe` and `javaScriptCoreRe` began with `^\\s*` followed by a group that
7+
// also matches whitespace, so a leading run of spaces could be split between
8+
// them in n ways and parsing a non-matching line was quadratic in its length.
9+
// A 30k-character run took over three seconds; err.stack embeds the message,
10+
// so attacker-influenced text reaches this directly.
11+
it('parses a long leading whitespace run in linear time', function () {
12+
this.timeout(5000);
13+
14+
const time = (n) => {
15+
const line = ' '.repeat(n) + 'x';
16+
const start = Date.now();
17+
stackTraceParser.parse(line);
18+
return Date.now() - start;
19+
};
20+
21+
time(4000); // warm up
22+
const small = time(8000);
23+
const large = time(32000);
24+
25+
// Quadratic would be ~16x for a 4x input; allow generous headroom for a
26+
// slow or noisy CI machine while still failing on the old behaviour.
27+
expect(large).to.be.lessThan(Math.max(small, 1) * 8 + 100);
28+
});
29+
630
it('parses node error with space in path', () => {
731
const stackFrames = stackTraceParser.parse(
832
CapturedExceptions.NODE_SPACE.stack

0 commit comments

Comments
 (0)