fix(studio): avoid inline style regex backtracking - #3708
Conversation
miguel-heygen
left a comment
There was a problem hiding this comment.
Reviewed at bc107b1de735057cc257f0da1e5dfba91cb33689; no blockers.
The replacement follows CodeQL js/polynomial-redos's structural recommendation and is equivalent to the removed operations (packages/studio/src/utils/sourcePatcher.ts:214-217):
/\s*\/$/.test(tag)is true exactly whentag.endsWith("/"), because\s*may match zero characters;- on that branch,
tag.replace(/\s*\/$/, "")removes the terminal slash plus all whitespace immediately before it, exactly whattag.slice(0, -1).trimEnd()does; - when whitespace follows the slash—including CR/LF or Unicode line separators—both forms classify the tag as non-self-closing and preserve its bytes before appending the style.
I checked the old/new classification and base string over every Unicode code point in before-slash, after-slash, and double-slash positions, plus 200,000 deterministic randomized boundary strings: zero mismatches. The new path handles 100k spaces in ~0.09 ms and one million in ~1.3 ms.
The table test is a useful independent oracle: five whitespace prefixes × eight terminal suffixes × both ID and selector entry points produces the stated 80 assertions (packages/studio/src/utils/sourcePatcher.test.ts:60-76). The expected result derives self-closing state from the suffix, not from the production operation, and pins byte preservation for ordinary tags, terminal slash normalization, and newline/Unicode boundaries. Both public entry points converge on the same patchInlineStyleInTag, so there is one policy rather than two copies.
CI is still running at this head; initial regression/lint gates are green, while full build, Studio/Windows checks, and JavaScript CodeQL remain landing gates. Left unmerged as requested.
Verdict: APPROVE
Reasoning: The built-in string operations are byte-for-byte equivalent to the former regex semantics while removing backtracking, and the shared implementation is covered across both patch entry points and boundary classes.
— Magi
Fixes CodeQL alerts #395 and #396. Adding an inline style to a tag without an existing style can spend quadratic time retrying the self-closing regex over long whitespace runs.
Replace the regex checks with
endsWith("/")and remove the terminal slash beforetrimEnd(). This preserves the existing exact-ending requirement and whitespace removal while avoiding backtracking. Two production lines change; no markup parsing or editing policy changes.Validation: original regex exceeded a two-second subprocess timeout on 100k spaces. All 36 source-patcher tests pass, including 80 new ID/selector assertions covering ordinary and Unicode whitespace, slash/newline boundaries, long malformed tags and self-closing tags. A deterministic 100k-input old/new comparison found no output differences. Full workspace build, Studio typecheck, lint and format pass.