mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-12 07:09:59 +00:00
fix(producer): scope and harden the markup-payload sniff
Review follow-up on the STUDIO-5433 defense.
Correctness
- The sniff ran above the documented video/audio failure split, so an
<audio> src that resolved to an HTML payload aborted the whole render
instead of degrading to duration 0. It now runs inside the same try, so
video surfaces the typed error while audio still drops out, with a
warning naming the element.
- Raw fs errors (EISDIR on a directory src, EACCES, the existsSync->open
ENOENT race, EMFILE) escaped and failed the compile with an unclassified
error carrying an unredacted temp path. The sniff is now a classifier that
never throws: an unreadable file reports "not markup" and the real probe
produces the real error.
- Elements whose duration the compiler never resolves (a data-end video, a
looping audio) skipped the sniff entirely, so the original ffprobe error
still escaped, and looping audio was reported as owner "system" after
every frame had been captured. Video is now caught in the asset preflight,
which sees every local src regardless of authored timing; audio is
classified per-element in audioMixer as source/invalid_media/owner "user",
keeping audio failures non-fatal as they already were.
- Detection is a byte-level check for a leading "<" (BOM-, whitespace- and
NUL-tolerant, looped read) instead of a <!doctype|<html|<?xml string
prefix, which missed a NUL-prefixed payload, >256B of leading whitespace,
UTF-16-encoded HTML, and a prolog-less <svg. No supported container starts
with "<", so the allowlist no longer grows per payload shape.
- finally { await fh.close() } could replace the in-flight typed error with
the close error.
Routing and privacy
- MARKUP_NOT_MEDIA is now in SAFE_RENDER_ERROR_CODES, the Lambda terminal
name map, the CDK and SAM non-retryable plan lists, and the Cloud Run
non-retryable set, and the class carries owner/retryable. Previously the
API emitted errorCode: undefined and a deterministic authoring bug burned
the full distributed retry budget.
- The message no longer carries 32 raw payload bytes or the src.
redactTelemetryString preserves host and path for HTTP srcs, so
per-tenant CDN paths reached a message the server forwards to clients.
Correlation is a sha256 element fingerprint, matching
AssetMediaTypeMismatchError.
- The message names both causes (unresolved nested-composition URL, or an
HTML/XML error page served as 200) rather than misdiagnosing an S3 403
body as an authoring bug.
Tests
- Byte-level detection is unit-tested in engine: markup shapes, BOMs,
UTF-16, nine container signatures, unreadable inputs.
- Replaced the tautological assertions. The old checks for "html" in and
"moov" absent from a fixed message template could not fail for any input.
- New coverage for audio degradation, the audioMixer classification, the
preflight video/image/audio split, and the API error metadata.
- The sibling htmlCompiler.mediaType failure was a vitest-under-bun runner
mismatch, not a missing ffmpeg binary. It passes, including the 4-wide
probe-semaphore invariant the sniff now runs inside.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
8a11e9776d
commit
f3689c1481
@@ -0,0 +1,19 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { MarkupNotMediaError } from "@hyperframes/engine";
|
||||
import { extractSafeRenderErrorMetadata } from "./server.js";
|
||||
|
||||
// Kept out of server.errorCode.test.ts so that suite keeps exactly one typed
|
||||
// error class in scope: two same-shaped classes there defeat the dead-code
|
||||
// analyzer's member resolution and it reports the sibling's fields as unused.
|
||||
describe("extractSafeRenderErrorMetadata — markup payloads", () => {
|
||||
it("transports the code, owner, and retry policy", () => {
|
||||
// Without the SAFE_RENDER_ERROR_CODES entry the API emits
|
||||
// `errorCode: undefined` and the failure is indistinguishable from an
|
||||
// untyped crash — as unroutable as the `moov atom not found` it replaces.
|
||||
expect(extractSafeRenderErrorMetadata(new MarkupNotMediaError(["0123456789abcdef"]))).toEqual({
|
||||
errorCode: "MARKUP_NOT_MEDIA",
|
||||
errorOwner: "user",
|
||||
retryable: false,
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -121,6 +121,7 @@ interface PreparedRenderInput {
|
||||
const DEFAULT_SERVER_FPS = { num: 30, den: 1 } as const;
|
||||
const SAFE_RENDER_ERROR_CODES = new Set<string>([
|
||||
"ASSET_MEDIA_TYPE_MISMATCH",
|
||||
"MARKUP_NOT_MEDIA",
|
||||
"INVALID_VIDEO_METADATA",
|
||||
"VIDEO_SOURCE_UNRENDERABLE",
|
||||
"VIDEO_EXTRACTION_FAILED",
|
||||
|
||||
@@ -2,6 +2,7 @@ import { afterAll, beforeAll, describe, expect, it } from "vitest";
|
||||
import { copyFileSync, existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { MARKUP_NOT_MEDIA, MarkupNotMediaError } from "@hyperframes/engine";
|
||||
import {
|
||||
ASSET_MEDIA_TYPE_MISMATCH,
|
||||
AssetMediaTypeMismatchError,
|
||||
@@ -191,6 +192,64 @@ describe("preflightCompositionAssetMediaTypes", () => {
|
||||
await expect(run({ imageSrc: "corrupt-media" })).resolves.toBeUndefined();
|
||||
});
|
||||
|
||||
// STUDIO-5433. This preflight sees every local media src regardless of its
|
||||
// authored timing, so it is the only place a markup payload behind a
|
||||
// `data-end` video or a `loop`ing audio is caught before frames are captured
|
||||
// — for those elements the compiler never resolves a duration, so its own
|
||||
// sniff never runs.
|
||||
describe("markup payloads", () => {
|
||||
beforeAll(() => {
|
||||
writeFileSync(
|
||||
join(projectDir, "streamed-preview.html"),
|
||||
"<!DOCTYPE html><html><body>not media</body></html>",
|
||||
);
|
||||
writeFileSync(
|
||||
join(projectDir, "brand-mark.svg"),
|
||||
'<svg xmlns="http://www.w3.org/2000/svg" width="8" height="8"><rect width="8" height="8"/></svg>',
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects an HTML payload under a video element", async () => {
|
||||
let caught: unknown;
|
||||
try {
|
||||
await run({ videoSrc: "streamed-preview.html" });
|
||||
} catch (error) {
|
||||
caught = error;
|
||||
}
|
||||
expect(caught).toBeInstanceOf(MarkupNotMediaError);
|
||||
expect(caught).toMatchObject({
|
||||
code: MARKUP_NOT_MEDIA,
|
||||
owner: "user",
|
||||
retryable: false,
|
||||
});
|
||||
const message = (caught as Error).message;
|
||||
expect(message).not.toContain("streamed-preview.html");
|
||||
expect(message).not.toContain(fixtureDir);
|
||||
});
|
||||
|
||||
it("reports markup ahead of the type mismatch the same file also produces", async () => {
|
||||
// An HTML page under a <video> is both "not media" and "not video". The
|
||||
// markup verdict is the actionable one; the mismatch is a symptom of it.
|
||||
await expect(run({ videoSrc: "streamed-preview.html" })).rejects.toBeInstanceOf(
|
||||
MarkupNotMediaError,
|
||||
);
|
||||
});
|
||||
|
||||
it("leaves an audio source to the mixer's per-element classification", async () => {
|
||||
// A bad audio source is non-fatal by existing policy: the render ships
|
||||
// without the track and reports `audioError`. Aborting the whole compile
|
||||
// here would turn renders that used to succeed into hard failures, so
|
||||
// audioMixer classifies it as source/invalid_media/owner:user instead.
|
||||
await expect(run({ audioSrc: "streamed-preview.html" })).resolves.toBeUndefined();
|
||||
});
|
||||
|
||||
it("leaves an SVG image source alone", async () => {
|
||||
// ffprobe reads SVG through its svg_pipe demuxer, so markup is a
|
||||
// legitimate <img> payload and must not be swept up by the sniff.
|
||||
await expect(run({ imageSrc: "brand-mark.svg" })).resolves.toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
it("re-probes a path after its media contents are replaced", async () => {
|
||||
const mutablePath = join(projectDir, "mutable-media");
|
||||
const signal = new AbortController().signal;
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
import { createHash } from "node:crypto";
|
||||
import { existsSync } from "node:fs";
|
||||
import {
|
||||
fingerprintElementId,
|
||||
isMarkupPayload,
|
||||
MarkupNotMediaError,
|
||||
probeMediaProfile,
|
||||
resolveProjectRelativeSrc,
|
||||
type AudioElement,
|
||||
@@ -39,10 +41,6 @@ export class AssetMediaTypeMismatchError extends Error {
|
||||
}
|
||||
}
|
||||
|
||||
function fingerprintElementId(elementId: string): string {
|
||||
return createHash("sha256").update(elementId).digest("hex").slice(0, 16);
|
||||
}
|
||||
|
||||
function detectedAssetMediaType(profile: MediaProbeProfile): DetectedAssetMediaType {
|
||||
if (profile.visualKind === "moving") return "video";
|
||||
if (profile.visualKind === "still") return "image";
|
||||
@@ -133,10 +131,27 @@ export async function preflightCompositionAssetMediaTypes(input: {
|
||||
}
|
||||
|
||||
const mismatches: AssetMediaTypeMismatch[] = [];
|
||||
const markupFingerprints: string[] = [];
|
||||
const entries = [...byPath];
|
||||
await Promise.all(
|
||||
entries.map(([resolvedPath, pathReferences]) =>
|
||||
withMediaProbeSlot(async () => {
|
||||
// STUDIO-5433. This preflight is the only place every local media src is
|
||||
// seen regardless of its authored timing, so it is where a markup
|
||||
// payload behind a `data-end` video gets caught — the compiler's own
|
||||
// sniff only runs for elements whose duration it has to resolve.
|
||||
//
|
||||
// Video only, deliberately. An `<img>` may legitimately be an SVG,
|
||||
// which ffprobe reads through its `svg_pipe` demuxer. And a bad audio
|
||||
// source is non-fatal by existing policy (audioStage ships the render
|
||||
// without the track and reports `audioError`), so it is classified
|
||||
// per-element in audioMixer instead of aborted here.
|
||||
const markupCandidates = pathReferences.filter((ref) => ref.expected === "video");
|
||||
if (markupCandidates.length > 0 && (await isMarkupPayload(resolvedPath))) {
|
||||
markupFingerprints.push(...markupCandidates.map((ref) => fingerprintElementId(ref.id)));
|
||||
return;
|
||||
}
|
||||
|
||||
let profile: MediaProbeProfile;
|
||||
try {
|
||||
profile = await probeMediaProfile(resolvedPath, { signal: input.signal });
|
||||
@@ -156,5 +171,8 @@ export async function preflightCompositionAssetMediaTypes(input: {
|
||||
),
|
||||
);
|
||||
|
||||
// Thrown ahead of the mismatch aggregate: "this file is an HTML page" is the
|
||||
// actionable diagnosis, while the type mismatch it also produces is a symptom.
|
||||
if (markupFingerprints.length > 0) throw new MarkupNotMediaError(markupFingerprints);
|
||||
if (mismatches.length > 0) throw new AssetMediaTypeMismatchError(mismatches);
|
||||
}
|
||||
|
||||
@@ -6,11 +6,10 @@ import { join } from "node:path";
|
||||
import { parseHTML } from "linkedom";
|
||||
import { interpolateVolumeGain } from "@hyperframes/core/media-volume-envelope";
|
||||
import { defaultLogger } from "../logger.js";
|
||||
import { MarkupNotMediaError } from "@hyperframes/engine";
|
||||
import {
|
||||
assertNotHtmlPayload,
|
||||
collectExternalAssets,
|
||||
compileForRender,
|
||||
HtmlNotVideoError,
|
||||
injectSdkPositionEditsRenderScript,
|
||||
detectAncestorBackgroundImage,
|
||||
detectRenderModeHints,
|
||||
@@ -2270,163 +2269,25 @@ describe("sub-composition variable injection (render path, #2064)", () => {
|
||||
});
|
||||
});
|
||||
|
||||
// ── HTML payload sniff (STUDIO-5433) ───────────────────────────────────────
|
||||
// ── Markup payload sniff (STUDIO-5433) ─────────────────────────────────────
|
||||
//
|
||||
// Producer's `resolveMediaDuration` is a two-step pipeline (download → probe)
|
||||
// that runs on every media element without an authored duration. Prior to
|
||||
// this defense, an authoring bug that handed a `.html` payload through as a
|
||||
// video src produced an opaque `[mov,mp4,m4a,3gp,3g2,mj2 @ ...] moov atom
|
||||
// not found` from ffprobe — the `mov,mp4,…` prefix was ffprobe's default
|
||||
// demuxer probe order, NOT the file's true format, so every alert routed as
|
||||
// a codec/ffmpeg bug. The sniff below converts the class into a domain-typed
|
||||
// `HtmlNotVideoError` naming the src so it can be alerted and routed
|
||||
// correctly, independent of the (separate) authoring-side root cause fix.
|
||||
// that runs on every media element without an authored duration. Before this
|
||||
// defense, an authoring bug that handed a `.html` payload through as a video
|
||||
// src produced an opaque `[mov,mp4,m4a,3gp,3g2,mj2 @ ...] moov atom not found`
|
||||
// from ffprobe — the `mov,mp4,…` prefix is ffprobe's demuxer probe order, NOT
|
||||
// the file's true format, so every alert routed as a codec/ffmpeg bug. The
|
||||
// byte-level sniff itself is unit-tested in
|
||||
// `engine/src/utils/markupPayload.test.ts`; what is pinned here is the
|
||||
// compiler's handling of the verdict, which differs by element type.
|
||||
|
||||
describe("assertNotHtmlPayload", () => {
|
||||
it("throws HtmlNotVideoError when the file starts with <!DOCTYPE html>", async () => {
|
||||
const dir = mkdtempSync(join(tmpdir(), "hf-html-sniff-doctype-"));
|
||||
const filePath = join(dir, "nested.html");
|
||||
writeFileSync(
|
||||
filePath,
|
||||
"<!DOCTYPE html>\n<html><head><title>streamed-preview</title></head><body></body></html>",
|
||||
);
|
||||
|
||||
let caught: unknown;
|
||||
try {
|
||||
// URL src (kept verbatim by redactTelemetryString apart from the query
|
||||
// string) — makes the src-attribution assertion below meaningful.
|
||||
await assertNotHtmlPayload(filePath, "https://cdn.example.com/streamed-preview.html");
|
||||
} catch (err) {
|
||||
caught = err;
|
||||
}
|
||||
expect(caught).toBeInstanceOf(HtmlNotVideoError);
|
||||
const err = caught as HtmlNotVideoError;
|
||||
// The host of a plain URL survives redactTelemetryString; the trailing
|
||||
// `.html` basename gets replaced with `[file]` by the asset-basename rule.
|
||||
// The `[src=…]` framing is what matters for observability.
|
||||
expect(err.message).toContain("[src=https://cdn.example.com/");
|
||||
expect(err.message).toContain("cdn.example.com");
|
||||
// Sample of the file's first bytes appears in the message (case-insensitive
|
||||
// "html" comes from either `<!DOCTYPE html>` or `<html>`).
|
||||
expect(err.message.toLowerCase()).toContain("html");
|
||||
expect(err.code).toBe("HTML_NOT_VIDEO");
|
||||
});
|
||||
|
||||
it("redacts a relative-path src through redactTelemetryString before emitting", async () => {
|
||||
// A bare-relative path (`assets/nested.html`) is exactly the shape the
|
||||
// producer telemetry-redaction rules collapse to `[path]`. The error
|
||||
// message must go through redactTelemetryString so we neither leak the
|
||||
// path structure nor drop the `[src=…]` framing.
|
||||
const dir = mkdtempSync(join(tmpdir(), "hf-html-sniff-redact-"));
|
||||
const filePath = join(dir, "nested.html");
|
||||
writeFileSync(filePath, "<!DOCTYPE html><html></html>");
|
||||
|
||||
let caught: unknown;
|
||||
try {
|
||||
await assertNotHtmlPayload(filePath, "assets/nested.html");
|
||||
} catch (err) {
|
||||
caught = err;
|
||||
}
|
||||
expect(caught).toBeInstanceOf(HtmlNotVideoError);
|
||||
const err = caught as HtmlNotVideoError;
|
||||
expect(err.message).toContain("[src=[path]]");
|
||||
expect(err.message).not.toContain("assets/nested.html");
|
||||
});
|
||||
|
||||
it("throws when the file starts with <html> (no doctype, missing lang, upper/lower case)", async () => {
|
||||
const dir = mkdtempSync(join(tmpdir(), "hf-html-sniff-htmltag-"));
|
||||
const filePath = join(dir, "raw.html");
|
||||
writeFileSync(filePath, "<HTML><body>hi</body></HTML>");
|
||||
|
||||
await expect(assertNotHtmlPayload(filePath, "raw.html")).rejects.toThrow(HtmlNotVideoError);
|
||||
});
|
||||
|
||||
it("throws when the file starts with <?xml (SVG / generic XML)", async () => {
|
||||
const dir = mkdtempSync(join(tmpdir(), "hf-html-sniff-xml-"));
|
||||
const filePath = join(dir, "asset.svg");
|
||||
writeFileSync(
|
||||
filePath,
|
||||
'<?xml version="1.0" encoding="UTF-8"?><svg xmlns="http://www.w3.org/2000/svg"/>',
|
||||
);
|
||||
|
||||
await expect(assertNotHtmlPayload(filePath, "asset.svg")).rejects.toThrow(HtmlNotVideoError);
|
||||
});
|
||||
|
||||
it("tolerates a UTF-8 BOM and leading whitespace before the prefix", async () => {
|
||||
const dir = mkdtempSync(join(tmpdir(), "hf-html-sniff-bom-"));
|
||||
const filePath = join(dir, "bom.html");
|
||||
// 0xEF 0xBB 0xBF + newline + spaces + <!doctype ...
|
||||
const bomBuf = Buffer.from([0xef, 0xbb, 0xbf]);
|
||||
writeFileSync(
|
||||
filePath,
|
||||
Buffer.concat([bomBuf, Buffer.from("\n <!doctype html><html></html>")]),
|
||||
);
|
||||
|
||||
await expect(assertNotHtmlPayload(filePath, "bom.html")).rejects.toThrow(HtmlNotVideoError);
|
||||
});
|
||||
|
||||
it("does NOT throw for a real MP4 container (ftypmp42 header)", async () => {
|
||||
// A minimal MP4 file signature: `\x00\x00\x00\x18 ftypmp42 ...`.
|
||||
// We only care that the sniff prefix-match ignores it — downstream
|
||||
// ffprobe is what actually parses the container, and this test does
|
||||
// not exercise ffprobe.
|
||||
const dir = mkdtempSync(join(tmpdir(), "hf-html-sniff-mp4-"));
|
||||
const filePath = join(dir, "clip.mp4");
|
||||
const mp4Header = Buffer.from([
|
||||
0x00, 0x00, 0x00, 0x18, 0x66, 0x74, 0x79, 0x70, 0x6d, 0x70, 0x34, 0x32, 0x00, 0x00, 0x00,
|
||||
0x00, 0x6d, 0x70, 0x34, 0x32, 0x69, 0x73, 0x6f, 0x6d,
|
||||
]);
|
||||
writeFileSync(filePath, mp4Header);
|
||||
|
||||
// Should resolve without throwing.
|
||||
await assertNotHtmlPayload(filePath, "clip.mp4");
|
||||
});
|
||||
|
||||
it("does NOT throw for a WebM container (EBML header)", async () => {
|
||||
const dir = mkdtempSync(join(tmpdir(), "hf-html-sniff-webm-"));
|
||||
const filePath = join(dir, "clip.webm");
|
||||
// Matroska/WebM starts with EBML header: 0x1A 0x45 0xDF 0xA3
|
||||
const webmHeader = Buffer.from([0x1a, 0x45, 0xdf, 0xa3, 0x9f, 0x42, 0x86, 0x81, 0x01]);
|
||||
writeFileSync(filePath, webmHeader);
|
||||
|
||||
await assertNotHtmlPayload(filePath, "clip.webm");
|
||||
});
|
||||
|
||||
it("does NOT throw for an empty file (0 bytes — separate code path handles it)", async () => {
|
||||
const dir = mkdtempSync(join(tmpdir(), "hf-html-sniff-empty-"));
|
||||
const filePath = join(dir, "empty.bin");
|
||||
writeFileSync(filePath, "");
|
||||
|
||||
await assertNotHtmlPayload(filePath, "empty.bin");
|
||||
});
|
||||
|
||||
it("does NOT throw when the html prefix is deep inside the file, not at the start", async () => {
|
||||
// Regression guard: the check must be a prefix match, not a substring
|
||||
// scan. A legitimate media container that happens to contain the
|
||||
// substring `<html` further in must NOT be misclassified.
|
||||
const dir = mkdtempSync(join(tmpdir(), "hf-html-sniff-substr-"));
|
||||
const filePath = join(dir, "not-html.bin");
|
||||
writeFileSync(
|
||||
filePath,
|
||||
Buffer.concat([Buffer.from([0x00, 0x00, 0x01, 0xba]), Buffer.from("<html later on")]),
|
||||
);
|
||||
|
||||
await assertNotHtmlPayload(filePath, "not-html.bin");
|
||||
});
|
||||
});
|
||||
|
||||
describe("compileForRender HTML sniff (STUDIO-5433)", () => {
|
||||
it("aborts with HtmlNotVideoError when a <video> src points at an HTML payload", async () => {
|
||||
// Mimics the STUDIO-5433 failure mode: an a-roll element with
|
||||
// `content.src` pointing at a `streamed-preview.html` (legitimate
|
||||
// 6.5 KB `<!DOCTYPE html>` page, NOT an MP4). The producer's ffprobe
|
||||
// step would previously emit an opaque `moov atom not found` — with
|
||||
// the sniff in place, we abort with a typed error naming the src.
|
||||
const projectDir = mkdtempSync(join(tmpdir(), "hf-html-sniff-e2e-"));
|
||||
describe("compileForRender markup sniff (STUDIO-5433)", () => {
|
||||
function writeProject(mediaTag: string): string {
|
||||
const projectDir = mkdtempSync(join(tmpdir(), "hf-markup-sniff-e2e-"));
|
||||
mkdirSync(join(projectDir, "assets"));
|
||||
writeFileSync(
|
||||
join(projectDir, "assets", "nested.html"),
|
||||
"<!DOCTYPE html>\n<html><head><title>streamed-preview</title></head><body><p>Not a video.</p></body></html>",
|
||||
"<!DOCTYPE html>\n<html><head><title>streamed-preview</title></head><body></body></html>",
|
||||
);
|
||||
writeFileSync(
|
||||
join(projectDir, "index.html"),
|
||||
@@ -2434,7 +2295,7 @@ describe("compileForRender HTML sniff (STUDIO-5433)", () => {
|
||||
<html>
|
||||
<body>
|
||||
<div data-composition-id="root" data-width="640" data-height="360" data-start="0" data-duration="4">
|
||||
<video id="v1" src="assets/nested.html" data-start="0" muted></video>
|
||||
${mediaTag}
|
||||
</div>
|
||||
<script>
|
||||
window.__timelines = window.__timelines || {};
|
||||
@@ -2443,6 +2304,15 @@ describe("compileForRender HTML sniff (STUDIO-5433)", () => {
|
||||
</body>
|
||||
</html>`,
|
||||
);
|
||||
return projectDir;
|
||||
}
|
||||
|
||||
it("aborts with MarkupNotMediaError before ffprobe when a <video> src is an HTML payload", async () => {
|
||||
// Mimics STUDIO-5433: an a-roll element whose src points at a legitimate
|
||||
// 6.5 KB `<!DOCTYPE html>` preview page instead of the rendered MP4.
|
||||
const projectDir = writeProject(
|
||||
'<video id="v1" src="assets/nested.html" data-start="0" muted></video>',
|
||||
);
|
||||
|
||||
let caught: unknown;
|
||||
try {
|
||||
@@ -2450,14 +2320,42 @@ describe("compileForRender HTML sniff (STUDIO-5433)", () => {
|
||||
} catch (err) {
|
||||
caught = err;
|
||||
}
|
||||
expect(caught).toBeInstanceOf(HtmlNotVideoError);
|
||||
const err = caught as HtmlNotVideoError;
|
||||
// Bare relative src `assets/nested.html` is redacted to `[path]` by
|
||||
// redactTelemetryString — but the `[src=…]` framing survives.
|
||||
expect(err.message).toContain("[src=[path]]");
|
||||
expect(err.message.toLowerCase()).toContain("html");
|
||||
// The sniff must run BEFORE ffprobe, so no ffprobe-specific noise
|
||||
// (`moov atom not found`) can appear.
|
||||
expect(err.message).not.toMatch(/moov/i);
|
||||
expect(caught).toBeInstanceOf(MarkupNotMediaError);
|
||||
const err = caught as MarkupNotMediaError;
|
||||
// Routing metadata, not just a readable string: these are what the server's
|
||||
// SAFE_RENDER_ERROR_CODES allowlist and the distributed retry sets key off.
|
||||
expect(err.code).toBe("MARKUP_NOT_MEDIA");
|
||||
expect(err.owner).toBe("user");
|
||||
expect(err.retryable).toBe(false);
|
||||
// Correlation is the hashed element id — the authored src never reaches a
|
||||
// message that producer forwards to API clients.
|
||||
expect(err.elementFingerprints).toHaveLength(1);
|
||||
expect(err.message).not.toContain("assets/nested.html");
|
||||
// Also the ordering pin: this fixture is an input ffprobe rejects, so if
|
||||
// the sniff ran after the probe the rejection would be ffprobe's untyped
|
||||
// error and this assertion would fail.
|
||||
});
|
||||
|
||||
it("drops an <audio> markup payload to duration 0 and warns instead of failing the render", async () => {
|
||||
// The audio/video split is the compiler's contract: an unprobeable audio
|
||||
// src is excluded from the render, and only video surfaces its probe
|
||||
// failure. A hard abort here would take down renders that used to succeed
|
||||
// without the offending audio.
|
||||
const projectDir = writeProject(
|
||||
'<audio id="a1" src="assets/nested.html" data-start="0"></audio>',
|
||||
);
|
||||
const warnings: string[] = [];
|
||||
const log = { ...defaultLogger, warn: (message: string) => warnings.push(message) };
|
||||
|
||||
const compiled = await compileForRender(
|
||||
projectDir,
|
||||
join(projectDir, "index.html"),
|
||||
projectDir,
|
||||
{ log },
|
||||
);
|
||||
|
||||
expect(compiled.html).not.toContain('id="a1" src="assets/nested.html" data-end');
|
||||
expect(warnings.join("\n")).toContain("HTML/XML document");
|
||||
expect(warnings.join("\n")).toContain("a1");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -11,7 +11,6 @@
|
||||
*/
|
||||
|
||||
import { createReadStream, existsSync, mkdirSync, readFileSync } from "fs";
|
||||
import { open as openFile } from "node:fs/promises";
|
||||
import { join, dirname, resolve, basename } from "path";
|
||||
import { parseHTML } from "linkedom";
|
||||
import {
|
||||
@@ -22,7 +21,6 @@ import {
|
||||
shouldClampResolvedMediaDuration,
|
||||
CSS_URL_RE,
|
||||
isNonRelativeUrl,
|
||||
redactTelemetryString,
|
||||
type ResolvedDuration,
|
||||
type UnresolvedElement,
|
||||
} from "@hyperframes/core";
|
||||
@@ -53,6 +51,8 @@ import {
|
||||
type AudioVolumeKeyframe,
|
||||
type MediaProbeProfile,
|
||||
analyzeKeyframeIntervals,
|
||||
assertNotMarkupPayload,
|
||||
MarkupNotMediaError,
|
||||
probeMediaProfile,
|
||||
} from "@hyperframes/engine";
|
||||
import { assertPublicHttpsUrl, downloadToTemp, isHttpUrl } from "../utils/urlDownloader.js";
|
||||
@@ -148,83 +148,6 @@ class EmptyCompositionError extends Error {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Thrown when a media file handed to `resolveMediaDuration` is actually an
|
||||
* HTML (or generic XML) payload rather than a video/audio container.
|
||||
*
|
||||
* Motivating incident: STUDIO-5433 — an authoring bug produced an a-roll
|
||||
* element with `content.src` pointing at a `streamed-preview.html` URL that
|
||||
* downloaded to a legitimate 6.5 KB `<!DOCTYPE html>` page instead of the
|
||||
* expected MP4. ffprobe's opaque `[mov,mp4,m4a,3gp,3g2,mj2 @ ...] moov atom
|
||||
* not found` masked the actual cause (the `mov,mp4,…` prefix is just
|
||||
* ffprobe's default demuxer probe order, not the file's true format), and
|
||||
* every "moov atom not found" alert routed as an ffmpeg/codec bug instead of
|
||||
* an authoring bug.
|
||||
*
|
||||
* Sniffing 256 bytes off the front of the downloaded file is nearly free
|
||||
* (single small read via `fs.promises.open`), and converting this class of
|
||||
* failure into a domain-typed error means:
|
||||
* 1. Operators see `html-not-video: src=X` in logs, not `moov atom not found`.
|
||||
* 2. Follow-on telemetry / alerts can partition on this shape.
|
||||
* 3. The authoring-side root-cause fix (re-point nested a-roll src to a
|
||||
* rendered MP4 before it reaches producer) can ship independently — this
|
||||
* defense catches the class regardless of when authoring lands.
|
||||
*
|
||||
* Exported so callers that want `instanceof` narrowing (tests, upstream
|
||||
* telemetry classifiers) can discriminate this from other probe failures.
|
||||
*/
|
||||
export class HtmlNotVideoError extends Error {
|
||||
readonly code = "HTML_NOT_VIDEO" as const;
|
||||
readonly src: string;
|
||||
readonly headSample: string;
|
||||
|
||||
constructor(redactedSrc: string, headSample: string) {
|
||||
super(
|
||||
`Refusing to probe HTML payload as video: [src=${redactedSrc}]. ` +
|
||||
`First 32 bytes: ${headSample}. ` +
|
||||
"This usually means an unresolved nested composition URL was handed to ffprobe.",
|
||||
);
|
||||
this.name = "HtmlNotVideoError";
|
||||
this.src = redactedSrc;
|
||||
this.headSample = headSample;
|
||||
}
|
||||
}
|
||||
|
||||
// Matches `<!doctype`, `<html`, or `<?xml` at the very start (case-insensitive,
|
||||
// BOM- and leading-whitespace-tolerant handled at the caller). `<?xml` catches
|
||||
// SVG and generic XML documents — same class of "not a video container".
|
||||
const HTML_PAYLOAD_PREFIX_RE = /^(?:<!doctype|<html|<\?xml)/i;
|
||||
// UTF-8 BOM as a JS string code point (0xFEFF).
|
||||
const UTF8_BOM_CHAR = 0xfeff;
|
||||
|
||||
/**
|
||||
* Sniff the first ~256 bytes of a downloaded media file. If it starts with an
|
||||
* HTML or generic XML prefix, throw `HtmlNotVideoError` naming the offending
|
||||
* `src` instead of letting ffprobe emit an opaque `moov atom not found`.
|
||||
*
|
||||
* Callers pass the caller-provided `src` (URL or path used in the composition
|
||||
* HTML), which is redacted via `redactTelemetryString` before it reaches the
|
||||
* error message — the `filePath` argument is the on-disk copy we actually
|
||||
* read from. Exported for direct unit-test coverage.
|
||||
*/
|
||||
export async function assertNotHtmlPayload(filePath: string, src: string): Promise<void> {
|
||||
const fh = await openFile(filePath, "r");
|
||||
try {
|
||||
const buf = Buffer.alloc(256);
|
||||
const { bytesRead } = await fh.read(buf, 0, 256, 0);
|
||||
if (bytesRead === 0) return;
|
||||
let head = buf.subarray(0, bytesRead).toString("utf8");
|
||||
if (head.charCodeAt(0) === UTF8_BOM_CHAR) head = head.slice(1);
|
||||
const trimmed = head.replace(/^\s+/, "");
|
||||
if (HTML_PAYLOAD_PREFIX_RE.test(trimmed)) {
|
||||
const sample = trimmed.slice(0, 32).replace(/\s+/g, " ");
|
||||
throw new HtmlNotVideoError(redactTelemetryString(src), sample);
|
||||
}
|
||||
} finally {
|
||||
await fh.close();
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Recursively walk every `data-composition-src` reference reachable from
|
||||
* `html` (including nested sub-compositions) and verify each resolves to a
|
||||
@@ -492,6 +415,7 @@ async function resolveMediaDuration(
|
||||
downloadDir: string,
|
||||
tagName: string,
|
||||
elementIdentity: string,
|
||||
log?: ProducerLogger,
|
||||
): Promise<{ duration: number; resolvedPath: string }> {
|
||||
let filePath = src;
|
||||
|
||||
@@ -512,24 +436,33 @@ async function resolveMediaDuration(
|
||||
return { duration: 0, resolvedPath: filePath };
|
||||
}
|
||||
|
||||
// Defensive HTML sniff (STUDIO-5433). If an authoring bug hands us a
|
||||
// `.html` payload (e.g. an unresolved nested-composition preview URL),
|
||||
// refuse to probe and throw a typed HtmlNotVideoError. Without this,
|
||||
// ffprobe emits an opaque `[mov,mp4,...] moov atom not found` that masks
|
||||
// the actual cause and routes as a codec/ffmpeg bug. Runs before the
|
||||
// media-probe slot so we don't consume concurrency budget on payloads
|
||||
// that were never going to probe.
|
||||
await assertNotHtmlPayload(filePath, src);
|
||||
|
||||
return withMediaProbeSlot(async () => {
|
||||
let profile: MediaProbeProfile;
|
||||
try {
|
||||
// Markup sniff (STUDIO-5433): if an authoring bug hands us an HTML/XML
|
||||
// payload (e.g. an unresolved nested-composition preview URL), fail with
|
||||
// a typed MarkupNotMediaError instead of letting ffprobe emit an opaque
|
||||
// `[mov,mp4,...] moov atom not found` that routes as a codec bug.
|
||||
// Deliberately inside this try: the audio/video split below is the
|
||||
// contract, so a bad audio src must still degrade to duration 0 rather
|
||||
// than take down the whole render.
|
||||
await assertNotMarkupPayload(filePath, elementIdentity);
|
||||
profile = await probeMediaProfile(filePath);
|
||||
} catch (error) {
|
||||
// Preserve the historical split: invalid video sources surface their
|
||||
// probe failure, while invalid/unreadable audio sources resolve to zero
|
||||
// duration and are excluded by the compiler.
|
||||
if (tagName !== "video") return { duration: 0, resolvedPath: filePath };
|
||||
if (tagName !== "video") {
|
||||
if (error instanceof MarkupNotMediaError) {
|
||||
// Dropping it silently is what let STUDIO-5433 resurface downstream
|
||||
// as `prepare/ffmpeg_failed` with owner "system".
|
||||
log?.warn(
|
||||
`[compile] Audio "${elementIdentity}" (${src}) is an HTML/XML document, not a media ` +
|
||||
"file — the element is dropped from the render. Point it at a rendered media file.",
|
||||
);
|
||||
}
|
||||
return { duration: 0, resolvedPath: filePath };
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
assertAssetMediaTypeProfile(tagName === "video" ? "video" : "audio", profile, elementIdentity);
|
||||
@@ -577,9 +510,15 @@ async function compileHtmlFile(
|
||||
// Phase 1: Resolve missing durations (parallel ffprobe)
|
||||
const resolvedResults = await Promise.all(
|
||||
mediaUnresolved.map((el) =>
|
||||
resolveMediaDuration(el.src!, el.mediaStart, baseDir, downloadDir, el.tagName, el.id).then(
|
||||
({ duration }) => ({ id: el.id, duration }),
|
||||
),
|
||||
resolveMediaDuration(
|
||||
el.src!,
|
||||
el.mediaStart,
|
||||
baseDir,
|
||||
downloadDir,
|
||||
el.tagName,
|
||||
el.id,
|
||||
log,
|
||||
).then(({ duration }) => ({ id: el.id, duration })),
|
||||
),
|
||||
);
|
||||
const resolutions: ResolvedDuration[] = resolvedResults.filter((r) => r.duration > 0);
|
||||
@@ -601,6 +540,7 @@ async function compileHtmlFile(
|
||||
downloadDir,
|
||||
el.tagName,
|
||||
el.id,
|
||||
log,
|
||||
);
|
||||
return { id: el.id, tagName: el.tagName, duration: el.duration, maxDuration, src: el.src! };
|
||||
}),
|
||||
|
||||
Reference in New Issue
Block a user