From 6a50c617b612da180bec1ad0be6f00615df0eeff Mon Sep 17 00:00:00 2001 From: Carlos Alcaraz <193642530+calcarazgre646@users.noreply.github.com> Date: Thu, 4 Jun 2026 19:32:33 -0300 Subject: [PATCH] fix(player): reject non-finite composition dimensions from attributes and stage-size MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- packages/player/src/composition-probe.ts | 10 ++- .../player/src/hyperframes-player.test.ts | 60 +++++++++++++++++ packages/player/src/hyperframes-player.ts | 10 ++- .../src/runtime-message-handler.test.ts | 67 +++++++++++++++++++ .../player/src/runtime-message-handler.ts | 4 ++ 5 files changed, 147 insertions(+), 4 deletions(-) create mode 100644 packages/player/src/runtime-message-handler.test.ts diff --git a/packages/player/src/composition-probe.ts b/packages/player/src/composition-probe.ts index 17de82d86..5ac4c18dd 100644 --- a/packages/player/src/composition-probe.ts +++ b/packages/player/src/composition-probe.ts @@ -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; diff --git a/packages/player/src/hyperframes-player.test.ts b/packages/player/src/hyperframes-player.test.ts index 6fa8962cc..56e709efa 100644 --- a/packages/player/src/hyperframes-player.test.ts +++ b/packages/player/src/hyperframes-player.test.ts @@ -1614,3 +1614,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); + }); +}); diff --git a/packages/player/src/hyperframes-player.ts b/packages/player/src/hyperframes-player.ts index 39f0d2901..81b34f6f0 100644 --- a/packages/player/src/hyperframes-player.ts +++ b/packages/player/src/hyperframes-player.ts @@ -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"; @@ -170,12 +170,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": diff --git a/packages/player/src/runtime-message-handler.test.ts b/packages/player/src/runtime-message-handler.test.ts new file mode 100644 index 000000000..9539c6511 --- /dev/null +++ b/packages/player/src/runtime-message-handler.test.ts @@ -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(); + }); +}); diff --git a/packages/player/src/runtime-message-handler.ts b/packages/player/src/runtime-message-handler.ts index 3fb42f2dc..42deff886 100644 --- a/packages/player/src/runtime-message-handler.ts +++ b/packages/player/src/runtime-message-handler.ts @@ -86,7 +86,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);