Skip to content

Commit 3391799

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 0c71c89 commit 3391799

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
@@ -333,21 +333,32 @@ export function parseFrameRate(frameRateStr: string | undefined): number {
333333
const parts = frameRateStr.split("/");
334334
if (parts.length > 2) return 0;
335335

336+
// Number(), never parseFloat — on BOTH the rational operands and the plain
337+
// form. parseFloat stops at trailing garbage, so "60fps" parsed as 60 and,
338+
// once the plain path was fixed but the operands were not, "60fps/1" and
339+
// "30garbage/1garbage" still slipped through the rational branch.
340+
const strict = (part: string | undefined): number =>
341+
part === undefined || part.trim() === "" ? NaN : Number(part.trim());
342+
336343
const raw =
337344
parts.length === 2
338345
? (() => {
339-
const num = parseFloat(parts[0] ?? "");
340-
const den = parseFloat(parts[1] ?? "");
346+
const num = strict(parts[0]);
347+
const den = strict(parts[1]);
341348
if (!Number.isFinite(num) || !Number.isFinite(den) || den === 0) return NaN;
342349
return num / den;
343350
})()
344-
: // Not parseFloat: it stops at trailing garbage, so "60fps" parsed as
345-
// 60. Number() rejects the whole string.
346-
Number(frameRateStr.trim());
351+
: strict(frameRateStr);
347352

348353
if (!Number.isFinite(raw) || raw <= 0) return 0;
349354

355+
// Checked AFTER rounding as well as before. `raw * 100` overflows for a
356+
// finite-but-huge rate ("1e307", "1e307/1"), so `rounded` became Infinity
357+
// and sailed past the positivity check — reaching exactly the `-r Infinity`
358+
// failure the finite guard above exists to prevent.
350359
const rounded = Math.round(raw * 100) / 100;
360+
if (!Number.isFinite(rounded)) return 0;
361+
351362
// A real but very slow rate must not collapse to 0 and inherit the
352363
// caller's 30fps default.
353364
return rounded > 0 ? rounded : 0.01;

0 commit comments

Comments
 (0)