fix(autoInject): eliminate O(n²) stripComments and regex stack overflow (fixes failing ReDoS test)#2076
Open
JSap0914 wants to merge 1 commit into
Open
Conversation
stripComments used character-by-character string concatenation
(stripped += string[index]) which is O(n²) in V8 for large inputs —
~1 second for a 6 M-character string. Switch to slice-based
accumulation: collect non-comment slices into an array, push only at
comment boundaries, join at the end. This reduces the cost to O(n).
Additionally, ARROW_FN_ARGS regex applied to a very long string with no
'=>' caused RangeError: Maximum call stack size exceeded due to deep
recursive NFA backtracking. Arrow functions always contain '=>', so
guard the regex call with src.indexOf('=>') !== -1 to skip it entirely
for inputs that can never match.
Together these fix the failing test:
autoInject > should not be subject to ReDoS
Fixes caolan#1975 regression introduced by caolan#1980.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The existing test
autoInject > should not be subject to ReDoS(added in #1767) currently fails onmasterdue to two issues introduced after the #1980 merge:Root causes
1.
stripCommentsis O(n²) for large inputsThe function builds the output string with character-by-character concatenation:
For the
'text/*'.repeat(1000000)test input (6 M chars) this takes ~1 second in V8, consuming most of the 2-second mocha timeout.2.
ARROW_FN_ARGSregex causes a stack overflow on very long stringsApplied to the same 6 M-character string (which contains no
=>), V8's NFA regex engine enters deep recursive backtracking and throws:Fix
stripComments– replace char-by-char concatenation with slice accumulation: pushstring.slice(segmentStart, index)only at comment boundaries andparts.join("")at the end. This makes the function O(n) and preserves identical semantics (verified against all existing tests including the multi-line and nested-comment cases added in #1767 and #1780).parseParams– guard theARROW_FN_ARGSregex withsrc.indexOf("=>") !== -1. Arrow functions must contain=>; skipping the regex for inputs that lack it prevents the stack overflow with no change in matching behaviour.Verification
No existing test was modified.