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>
This commit is contained in:
Vance Ingalls
2026-07-31 19:52:17 -07:00
co-authored by Claude Opus 5
parent 19dc83bc2b
commit 4d563fa752
2 changed files with 28 additions and 7 deletions
+12 -2
View File
@@ -618,8 +618,18 @@ describe("parseFrameRate", () => {
expect(parseFrameRate(input)).toBe(0);
});
// Fell through to a bare parseFloat that stops at trailing garbage.
it.each(["30/1/2", "60fps"])("returns 0 for malformed input %s", (input) => {
// Fell through to a bare parseFloat that stops at trailing garbage. The
// rational operands had the same defect after the plain path was fixed.
it.each(["30/1/2", "60fps", "60fps/1", "60/1fps", "30garbage/1garbage", "/", "/1", "30/"])(
"returns 0 for malformed input %s",
(input) => {
expect(parseFrameRate(input)).toBe(0);
},
);
// raw * 100 overflows for a finite-but-huge rate, so the rounded value was
// Infinity even though the pre-round guard passed.
it.each(["1e307", "1e307/1", "1e308/0.5"])("returns 0 when rounding overflows: %s", (input) => {
expect(parseFrameRate(input)).toBe(0);
});
+16 -5
View File
@@ -356,21 +356,32 @@ export function parseFrameRate(frameRateStr: string | undefined): number {
const parts = frameRateStr.split("/");
if (parts.length > 2) return 0;
// Number(), never parseFloat — on BOTH the rational operands and the plain
// form. parseFloat stops at trailing garbage, so "60fps" parsed as 60 and,
// once the plain path was fixed but the operands were not, "60fps/1" and
// "30garbage/1garbage" still slipped through the rational branch.
const strict = (part: string | undefined): number =>
part === undefined || part.trim() === "" ? NaN : Number(part.trim());
const raw =
parts.length === 2
? (() => {
const num = parseFloat(parts[0] ?? "");
const den = parseFloat(parts[1] ?? "");
const num = strict(parts[0]);
const den = strict(parts[1]);
if (!Number.isFinite(num) || !Number.isFinite(den) || den === 0) return NaN;
return num / den;
})()
: // Not parseFloat: it stops at trailing garbage, so "60fps" parsed as
// 60. Number() rejects the whole string.
Number(frameRateStr.trim());
: strict(frameRateStr);
if (!Number.isFinite(raw) || raw <= 0) return 0;
// Checked AFTER rounding as well as before. `raw * 100` overflows for a
// finite-but-huge rate ("1e307", "1e307/1"), so `rounded` became Infinity
// and sailed past the positivity check — reaching exactly the `-r Infinity`
// failure the finite guard above exists to prevent.
const rounded = Math.round(raw * 100) / 100;
if (!Number.isFinite(rounded)) return 0;
// A real but very slow rate must not collapse to 0 and inherit the
// caller's 30fps default.
return rounded > 0 ? rounded : 0.01;