fix(engine): avoid transform regex backtracking - #3704
Conversation
miguel-heygen
left a comment
There was a problem hiding this comment.
Reviewed at 3ae234375f07dde6950132d20a9c6ecf8cfe5500; no blockers.
The fix follows the js/polynomial-redos recommendation directly: remove the ambiguous overlapping repetitions rather than tuning them. In the 2D pattern, each removed \s* overlapped a following [^,]+/[^,)]+ capture that already accepts whitespace; in the 3D pattern, the leading capture already accepts it. Number(...) and the existing per-part trim() retain the conversion behavior (packages/engine/src/utils/alphaBlit.ts:923-936). Delimiters now separate every repeated region, so the patterns scan linearly on failure.
I independently compared the old and new parser behavior across 200,000 deterministic random strings plus 100,000 structured matrix/matrix3d inputs, including blanks, tabs/newlines, -0, decimals, exponents, hex, non-finite values, parentheses, and empty fields: zero output mismatches. The eight committed 100k-space malformed shapes completed in 0.12–0.25 ms each with the new patterns.
The regression coverage is well targeted: it exercises every 2D comma boundary plus the 3D prefix with long whitespace (packages/engine/src/utils/alphaBlit.test.ts:850-861), and separately pins the intentionally permissive whitespace/numeric conversion and true empty-field rejection (:839-848). Existing 3D projection and warning tests remain unchanged.
At review time lint, format, unit tests, runtime contract, and initial probes are green; the longer build/Windows/regression and JavaScript CodeQL jobs are still running and remain landing gates. Left unmerged as requested.
Verdict: APPROVE
Reasoning: The patch removes the precise ambiguity CodeQL identified, preserves the parser's observable language after numeric conversion, and adds adversarial coverage at each former backtracking boundary.
— Magi
Malformed CSS matrix strings with long whitespace runs can make parseTransformMatrix spend seconds backtracking. Remove redundant whitespace quantifiers from the matrix and matrix3d patterns: their captures already accept whitespace, and existing numeric conversion handles it. This addresses CodeQL alerts #194 and #195 without changing accepted numeric values, malformed-input results, 3D projection, or warning behavior.
Validation: both original patterns exceeded a 2-second subprocess timeout on 100,000 spaces. All 105 alphaBlit tests pass after the fix, including eight long malformed prefixes and whitespace/numeric compatibility coverage. Core/parser builds, engine typecheck, changed-file lint/format, and signed-commit hooks passed. CI and CodeQL must pass before landing.