mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-01 19:42:03 +00:00
fix(player): reject non-finite composition dimensions from attributes and stage-size (#1205)
width/height attributes went through parseInt with no validation, so a typo like width="abc" reached scaleIframeToFit as NaN (invalid scale(NaN) transform) and width="0" as a division by zero — both blank the player with no signal. The stage-size message check had the sibling gap: `> 0` alone lets Infinity through, which scales the iframe to 0. Reuse the composition probe's readPositiveDimension guard for the attribute path (the probe path already rejected these) and add the same finite-check the adjacent timeline branch uses for stage-size. Mirrors the clampPlaybackRate hardening from #1120. Co-authored-by: Carlos Alcaraz <193642530+calcarazgre646@users.noreply.github.com>
This commit is contained in:
co-authored by
Carlos Alcaraz
parent
937ba2cebe
commit
513819ee84
@@ -36,7 +36,15 @@ export interface ProbeCallbacks {
|
||||
onRuntimeInjected?: () => void;
|
||||
}
|
||||
|
||||
function readPositiveDimension(value: string | null): number | null {
|
||||
/**
|
||||
* Parse a composition dimension, rejecting anything that isn't a positive
|
||||
* finite number. Exported because the `width`/`height` attribute handlers in
|
||||
* hyperframes-player.ts need the same guard: dimensions feed
|
||||
* scaleIframeToFit's `w / compositionWidth` division, where NaN produces an
|
||||
* invalid `scale(NaN)` transform and zero a division by zero — both render
|
||||
* the player blank with no signal.
|
||||
*/
|
||||
export function readPositiveDimension(value: string | null): number | null {
|
||||
if (value === null) return null;
|
||||
const parsed = Number.parseInt(value, 10);
|
||||
return Number.isFinite(parsed) && parsed > 0 ? parsed : null;
|
||||
|
||||
@@ -1946,3 +1946,63 @@ describe("HyperframesPlayer playback rate", () => {
|
||||
expect(player.playbackRate).toBe(5);
|
||||
});
|
||||
});
|
||||
|
||||
// ── Composition dimension attributes ──
|
||||
//
|
||||
// width/height feed scaleIframeToFit's `w / compositionWidth` division. A
|
||||
// non-numeric, zero, or negative attribute must fall back to the defaults
|
||||
// instead of reaching the scale math as NaN (invalid `scale(NaN)` transform)
|
||||
// or zero (division by zero) — both blank the player with no signal.
|
||||
|
||||
describe("HyperframesPlayer composition dimension attributes", () => {
|
||||
type PlayerWithDimensions = HTMLElement & {
|
||||
_compositionWidth?: number;
|
||||
_compositionHeight?: number;
|
||||
};
|
||||
|
||||
let player: PlayerWithDimensions;
|
||||
|
||||
beforeEach(async () => {
|
||||
await import("./hyperframes-player.js");
|
||||
player = document.createElement("hyperframes-player") as PlayerWithDimensions;
|
||||
document.body.appendChild(player);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
document.body.innerHTML = "";
|
||||
});
|
||||
|
||||
it("applies a valid width and height", () => {
|
||||
player.setAttribute("width", "1280");
|
||||
player.setAttribute("height", "720");
|
||||
expect(player._compositionWidth).toBe(1280);
|
||||
expect(player._compositionHeight).toBe(720);
|
||||
});
|
||||
|
||||
it("falls back to defaults for non-numeric values", () => {
|
||||
player.setAttribute("width", "abc");
|
||||
player.setAttribute("height", "abc");
|
||||
expect(player._compositionWidth).toBe(1920);
|
||||
expect(player._compositionHeight).toBe(1080);
|
||||
});
|
||||
|
||||
it("falls back to defaults for zero", () => {
|
||||
player.setAttribute("width", "0");
|
||||
player.setAttribute("height", "0");
|
||||
expect(player._compositionWidth).toBe(1920);
|
||||
expect(player._compositionHeight).toBe(1080);
|
||||
});
|
||||
|
||||
it("falls back to defaults for negative values", () => {
|
||||
player.setAttribute("width", "-500");
|
||||
player.setAttribute("height", "-500");
|
||||
expect(player._compositionWidth).toBe(1920);
|
||||
expect(player._compositionHeight).toBe(1080);
|
||||
});
|
||||
|
||||
it("recovers the defaults when the attribute is removed", () => {
|
||||
player.setAttribute("width", "1280");
|
||||
player.removeAttribute("width");
|
||||
expect(player._compositionWidth).toBe(1920);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { CompositionProbe, type ProbeResult } from "./composition-probe.js";
|
||||
import { CompositionProbe, type ProbeResult, readPositiveDimension } from "./composition-probe.js";
|
||||
import { isControlsClick, setupControls, setupPoster } from "./controls-setup.js";
|
||||
import { adoptShadowStyles, createCompositionIframe, scaleIframeToFit } from "./iframe-dom.js";
|
||||
import { DirectTimelineClock } from "./direct-timeline-clock.js";
|
||||
@@ -195,12 +195,16 @@ class HyperframesPlayer extends HTMLElement {
|
||||
if (val !== null) this.iframe.srcdoc = prepareSrcdocForElement(this, val);
|
||||
else this.iframe.removeAttribute("srcdoc");
|
||||
break;
|
||||
// Reject NaN/zero/negative dimensions the same way the composition
|
||||
// probe does (a typo like width="abc" or width="0" would otherwise
|
||||
// reach scaleIframeToFit as scale(NaN) or a division by zero and
|
||||
// blank the player); fall back to the defaults instead.
|
||||
case "width":
|
||||
this._compositionWidth = parseInt(val || "1920", 10);
|
||||
this._compositionWidth = readPositiveDimension(val) ?? 1920;
|
||||
this._rescale();
|
||||
break;
|
||||
case "height":
|
||||
this._compositionHeight = parseInt(val || "1080", 10);
|
||||
this._compositionHeight = readPositiveDimension(val) ?? 1080;
|
||||
this._rescale();
|
||||
break;
|
||||
case "controls":
|
||||
|
||||
@@ -0,0 +1,67 @@
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
|
||||
import { handleRuntimeMessage, type MessageHandlerCallbacks } from "./runtime-message-handler.js";
|
||||
import type { ParentMediaManager } from "./parent-media.js";
|
||||
import type { ShaderLoaderState } from "./shader-loader-state.js";
|
||||
|
||||
// Only the stage-size branch is exercised here; the rest of the callback
|
||||
// surface is satisfied with inert spies so the handler's type contract
|
||||
// stays honest without pulling in the real player.
|
||||
const makeCallbacks = (): MessageHandlerCallbacks => ({
|
||||
updateControlsTime: vi.fn(),
|
||||
updateControlsPlaying: vi.fn(),
|
||||
dispatchEvent: vi.fn(),
|
||||
seek: vi.fn(),
|
||||
play: vi.fn(),
|
||||
getLoop: vi.fn(() => false),
|
||||
media: { mirrorTime: vi.fn(), promoteToParentProxy: vi.fn() } as unknown as ParentMediaManager,
|
||||
getPlaybackState: vi.fn(() => ({ currentTime: 0, duration: 0, paused: true, lastUpdateMs: 0 })),
|
||||
setPlaybackState: vi.fn(),
|
||||
getShaderLoadingMode: vi.fn(() => "auto"),
|
||||
shaderLoader: { update: vi.fn() } as unknown as ShaderLoaderState,
|
||||
setCompositionSize: vi.fn(),
|
||||
sendControl: vi.fn(),
|
||||
getIframeDoc: vi.fn(() => null),
|
||||
});
|
||||
|
||||
const stageSizeEvent = (width: unknown, height: unknown, source: object): MessageEvent =>
|
||||
({
|
||||
source,
|
||||
data: { source: "hf-preview", type: "stage-size", width, height },
|
||||
}) as unknown as MessageEvent;
|
||||
|
||||
describe("handleRuntimeMessage stage-size", () => {
|
||||
it("applies a finite positive stage size", () => {
|
||||
const frameWindow = {} as Window;
|
||||
const callbacks = makeCallbacks();
|
||||
|
||||
handleRuntimeMessage(stageSizeEvent(1280, 720, frameWindow), frameWindow, callbacks);
|
||||
|
||||
expect(callbacks.setCompositionSize).toHaveBeenCalledWith(1280, 720);
|
||||
});
|
||||
|
||||
it.each([
|
||||
["Infinity width", Infinity, 720],
|
||||
["Infinity height", 1280, Infinity],
|
||||
["NaN width", NaN, 720],
|
||||
["zero width", 0, 720],
|
||||
["negative height", 1280, -720],
|
||||
["string width", "1280", 720],
|
||||
])("ignores stage-size with %s", (_label, width, height) => {
|
||||
const frameWindow = {} as Window;
|
||||
const callbacks = makeCallbacks();
|
||||
|
||||
handleRuntimeMessage(stageSizeEvent(width, height, frameWindow), frameWindow, callbacks);
|
||||
|
||||
expect(callbacks.setCompositionSize).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("ignores messages from a different source window", () => {
|
||||
const frameWindow = {} as Window;
|
||||
const callbacks = makeCallbacks();
|
||||
|
||||
handleRuntimeMessage(stageSizeEvent(1280, 720, {}), frameWindow, callbacks);
|
||||
|
||||
expect(callbacks.setCompositionSize).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -95,7 +95,11 @@ export function handleRuntimeMessage(
|
||||
|
||||
if (
|
||||
data["type"] === "stage-size" &&
|
||||
// Finite-check like the timeline branch above: `> 0` alone lets
|
||||
// Infinity through, which scales the iframe to 0 and blanks it.
|
||||
Number.isFinite(data["width"]) &&
|
||||
(data["width"] as number) > 0 &&
|
||||
Number.isFinite(data["height"]) &&
|
||||
(data["height"] as number) > 0
|
||||
) {
|
||||
callbacks.setCompositionSize(data["width"] as number, data["height"] as number);
|
||||
|
||||
Reference in New Issue
Block a user