perf: fast-fail link tokenizing when no ')' remains ahead - #4070
perf: fast-fail link tokenizing when no ')' remains ahead#4070koding88 wants to merge 3 commits into
Conversation
The inline link rule backtracks quadratically over unterminated
link sequences. '[a](b'.repeat(n) parses in O(n^2) — 97KB took
~2.5s — because at every '[' the regex's greedy unquoted-href run
re-partitions the whole remainder while hunting for a closing ')'
that never comes.
Lexer.inlineTokens now anchors one paren scan per inline run
(lastIndexOf(')') over the paragraph), and Tokenizer.link consults
it: since every src it receives during the run is a suffix of that
anchor, 'is there a ) ahead?' is O(1). When none remains the regex
cannot match anyway, so skipping is semantics-preserving; anchors
are saved/restored around nested runs (e.g. link labels) and live
in a WeakMap so they stay off the public tokenizer API.
Measured on Node 24:
'[a](b'.repeat(20000) ~2500ms -> ~24ms
'[a](b'.repeat(80000) (OOM-adjacent) -> ~60ms
Differential check: 1506 generated documents (links with titles,
balanced parens, angle destinations, images, autolinks, code spans,
reflinks + defs, nested brackets, unterminated sequences) x gfm on/
off produce byte-identical output before and after the change.
Adds test/specs/redos/quadratic_unterminated_links.cjs following
the existing guard convention; it exceeds the redos budget on
master and passes with this change.
|
@koding88 is attempting to deploy a commit to the MarkedJS Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Pull request overview
This PR addresses a quadratic-time performance issue in inline link tokenization for unterminated inline link patterns (e.g., "[a](b".repeat(n)), by adding a fast-fail check that avoids triggering pathological regex backtracking when no closing ) exists in the remaining source.
Changes:
- Add a per-inline-run “last
)” anchor hint stored/restored byLexer.inlineTokensto enable O(1) “any)ahead?” checks. - Update
Tokenizer.linkto consult the hint and skip link-regex evaluation when no)can possibly occur in the remaining suffix. - Add a new ReDoS regression spec for quadratic unterminated inline links.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test/specs/redos/quadratic_unterminated_links.cjs | Adds a regression case to ensure unterminated inline-link inputs no longer exceed the ReDoS budget. |
| src/Tokenizer.ts | Introduces the WeakMap-based hint and uses it in link() to fast-fail before running the expensive regex. |
| src/Lexer.ts | Anchors/restores the hint around each inline tokenization run so Tokenizer.link can make O(1) decisions on suffix inputs. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Per review feedback: drop the WeakMap anchor machinery in favor of a single state flag. Lexer.inlineTokens scans for ')' once per run and stores it in state.linkParenPossible (same pattern as linkEmitted), restoring the previous value after nested runs. Tokenizer.link bails out only when the flag is false, which is exactly when its regex cannot match. This keeps the fast-fail while avoiding a repeated full-text includes() on reflink-heavy paragraphs, where attempting link() at every token would otherwise re-scan the remainder each time (quadratic_inline_masking[2]: ~1.03s vs ~144ms here, upstream ~140ms).
|
Thank you for the review @UziTech — implemented along the lines you suggested, with one measurement worth sharing. Your exact snippet ( So I kept your idea but hoisted the scan to one per inline run:
Current numbers on Node 24 (same machine):
Differential: 1,506 generated docs × gfm on/off → byte-identical to master. Full Happy to flatten it further to the plain two-liner if you'd rather trade the masking[2] budget for fewer moving parts — say the word and I'll push that instead. |
| // quadratic backtracking over unterminated-link inputs like | ||
| // `'[a](b'.repeat(n)`. | ||
| const prevLinkParenPossible = this.state.linkParenPossible; | ||
| this.state.linkParenPossible = src.includes(')'); |
There was a problem hiding this comment.
This also only needs to be checked if prevLinkParenPossible is true, correct? Since inlineTokens will only be called with a substring of the previous call. If the previous call didn't include a ')' than neither will this one.
When the outer inline run contains no ')' (linkParenPossible=false),
a nested run's substring can't contain one either, so skip the
redundant src.includes(')') scan. Addresses UziTech's observation
on line 354: 'This also only needs to be checked if prevLinkParenPossible
is true.'
|
Good catch on both points — pushed Line 354 (skip scan when outer had no this.state.linkParenPossible = prevLinkParenPossible && src.includes(')');so the Verified: 1791 spec + 191 unit tests pass, and the quadratic input ( |
UziTech
left a comment
There was a problem hiding this comment.
Thanks for helping make marked faster! 💯
Marked version: master (c430a64, v18.0.10)
Markdown flavor: n/a (parser performance)
Description
The inline link rule backtracks quadratically over unterminated link sequences:
[a](b×10k)[a](b×20k)Doubling the input quadruples the time — O(n²). At every
[, the rule's greedy unquoted-href run[^ \t\n\x00-\x1f]+swallows the entire remainder and then re-partitions it char-by-char while hunting for a closing `)" that never comes; the lexer advances one character and repeats. This is the same class as #4013 (empty-href + whitespace variant), but with non-empty content between brackets, so that fix does not cover it — and none of the existing redos guards match this shape.The change
Lexer.inlineTokensanchors onelastIndexOf(')')scan per inline run, andTokenizer.linkconsults it before running the regex: since every src it receives during the run is a suffix of that anchor, "is there a `) ahead?" is answerable in O(1). When no paren remains, the regex cannot match anyway, so skipping is semantics-preserving — the skip only removes the dead backtracking.Details:
tokenizer.link()call outside a lexer run lazily anchors itself.An earlier design that re-anchored on run-id mismatch instead of saving/restoring was 100× slower on reflink-heavy input (
quadratic_inline_masking[2]blew its budget at ~10s); save/restore fixed that while keeping the unterminated-link win.Verification
gfmon/off → byte-identical output before/after.test/specs/redos/quadratic_unterminated_links.cjs: exceeds the redos budget on master (~13.7s), passes with this change.npm testgreen: 1791 spec tests, 191 unit tests, UMD/CJS, types, ESLint.AI disclosure: OpenAI Codex assisted with implementation and validation; I reviewed the parser behavior, ran the differential suite, and own the contribution.
Contributor
Committer
In most cases, this should be a different person than the contributor.