Skip to content

Commit 4d563fa

Browse files
vanceingallsclaude
andcommitted
fix(engine): guard the rounded frame rate and strict-parse rationals
Two paths the previous guards still let through. Rounding could recreate Infinity after the finite check. `raw * 100` overflows for a finite-but-huge rate — "1e307", "1e307/1" — so `rounded` became Infinity and passed the positivity check, reaching exactly the `-r Infinity` failure the finite guard exists to prevent. The rounded result is now checked too. The rational operands still used parseFloat. The plain-number path switched to Number() so trailing garbage fails the whole string, but the numerator and denominator did not, so "60fps/1", "60/1fps" and "30garbage/1garbage" returned valid rates while the contract says malformed frame rates fail closed. Both operands are now parsed strictly, and an empty operand ("/", "/1", "30/") is rejected rather than coerced. Tests: 8 malformed inputs and 3 overflow cases in the direct table. Reverting either fix fails 5. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 19dc83b commit 4d563fa

2 files changed

Lines changed: 28 additions & 7 deletions

File tree

packages/engine/src/utils/ffprobe.test.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -618,8 +618,18 @@ describe("parseFrameRate", () => {
618618
expect(parseFrameRate(input)).toBe(0);
619619
});
620620

621-
// Fell through to a bare parseFloat that stops at trailing garbage.
622-
it.each(["30/1/2", "60fps"])("returns 0 for malformed input %s", (input) => {
621+
// Fell through to a bare parseFloat that stops at trailing garbage. The
622+
// rational operands had the same defect after the plain path was fixed.
623+
it.each(["30/1/2", "60fps", "60fps/1", "60/1fps", "30garbage/1garbage", "/", "/1", "30/"])(
624+
"returns 0 for malformed input %s",
625+
(input) => {
626+
expect(parseFrameRate(input)).toBe(0);
627+
},
628+
);
629+
630+
// raw * 100 overflows for a finite-but-huge rate, so the rounded value was
631+
// Infinity even though the pre-round guard passed.
632+
it.each(["1e307", "1e307/1", "1e308/0.5"])("returns 0 when rounding overflows: %s", (input) => {
623633
expect(parseFrameRate(input)).toBe(0);
624634
});
625635

packages/engine/src/utils/ffprobe.ts

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -356,21 +356,32 @@ export function parseFrameRate(frameRateStr: string | undefined): number {
356356
const parts = frameRateStr.split("/");
357357
if (parts.length > 2) return 0;
358358

359+
// Number(), never parseFloat — on BOTH the rational operands and the plain
360+
// form. parseFloat stops at trailing garbage, so "60fps" parsed as 60 and,
361+
// once the plain path was fixed but the operands were not, "60fps/1" and
362+
// "30garbage/1garbage" still slipped through the rational branch.
363+
const strict = (part: string | undefined): number =>
364+
part === undefined || part.trim() === "" ? NaN : Number(part.trim());
365+
359366
const raw =
360367
parts.length === 2
361368
? (() => {
362-
const num = parseFloat(parts[0] ?? "");
363-
const den = parseFloat(parts[1] ?? "");
369+
const num = strict(parts[0]);
370+
const den = strict(parts[1]);
364371
if (!Number.isFinite(num) || !Number.isFinite(den) || den === 0) return NaN;
365372
return num / den;
366373
})()
367-
: // Not parseFloat: it stops at trailing garbage, so "60fps" parsed as
368-
// 60. Number() rejects the whole string.
369-
Number(frameRateStr.trim());
374+
: strict(frameRateStr);
370375

371376
if (!Number.isFinite(raw) || raw <= 0) return 0;
372377

378+
// Checked AFTER rounding as well as before. `raw * 100` overflows for a
379+
// finite-but-huge rate ("1e307", "1e307/1"), so `rounded` became Infinity
380+
// and sailed past the positivity check — reaching exactly the `-r Infinity`
381+
// failure the finite guard above exists to prevent.
373382
const rounded = Math.round(raw * 100) / 100;
383+
if (!Number.isFinite(rounded)) return 0;
384+
374385
// A real but very slow rate must not collapse to 0 and inherit the
375386
// caller's 30fps default.
376387
return rounded > 0 ? rounded : 0.01;

0 commit comments

Comments
 (0)