Merge pull request #2912 from heygen-com/ffprobe-1-png

fix(engine): stop the PNG walk at cICP, anchor IHDR, use native crc32

Author: vanceingalls

packages/engine/src/utils/ffprobe.test.ts

@@ -642,3 +642,111 @@ describe("extractPngMetadataFromBuffer cICP ordering", () => {
     expect(extractPngMetadataFromBuffer(onlyCicp)).toBeNull();
   });
 });
+
+describe("PNG chunk walk — integrity of the fallback itself", () => {
+  const IHDR_4K = [0, 0, 0x0f, 0, 0, 0, 0x08, 0x70, 16, 2, 0, 0, 0];
+  const CICP_PQ = [9, 16, 0, 1];
+
+  // Regression: the walk used to continue past cICP to IEND, which made
+  // whole-file integrity a precondition for returning anything. A damaged
+  // trailing chunk in an otherwise-good HDR PNG nulled the whole result, and
+  // extractMediaMetadata then re-throws the ffprobe error it had swallowed
+  // rather than using the fallback it just computed.
+  it("returns metadata even when a chunk AFTER cICP is corrupt", () => {
+    const bad = pngChunk("tEXt", [65, 66]);
+    bad[bad.length - 1] ^= 0xff; // break the CRC
+    const png = buildPngWithChunks([
+      pngChunk("IHDR", IHDR_4K),
+      pngChunk("cICP", CICP_PQ),
+      pngChunk("IDAT", [0x78, 0x9c, 0x63, 0x00, 0x00, 0x00, 0x02, 0x00, 0x01]),
+      bad,
+      pngChunk("IEND", []),
+    ]);
+    expect(extractPngMetadataFromBuffer(png)).toEqual({
+      width: 3840,
+      height: 2160,
+      colorSpace: { colorPrimaries: "bt2020", colorTransfer: "smpte2084", colorSpace: "gbr" },
+    });
+  });
+
+  it("survives outright truncation after cICP", () => {
+    const png = buildPngWithChunks([pngChunk("IHDR", IHDR_4K), pngChunk("cICP", CICP_PQ)]);
+    const truncated = Buffer.concat([png, Buffer.from([0, 0, 0x7f, 0xff, 73, 68, 65, 84])]);
+    expect(extractPngMetadataFromBuffer(truncated)?.width).toBe(3840);
+  });
+
+  // Regression: IHDR had no first-chunk anchor, so a later one overwrote the
+  // real dimensions and the producer laid out a 1-pixel image.
+  it("ignores a second IHDR", () => {
+    const png = buildPngWithChunks([
+      pngChunk("IHDR", IHDR_4K),
+      pngChunk("cICP", CICP_PQ),
+      pngChunk("IDAT", [0x78, 0x9c, 0x63, 0x00, 0x00, 0x00, 0x02, 0x00, 0x01]),
+      pngChunk("IHDR", [0, 0, 0, 1, 0, 0, 0, 1, 16, 2, 0, 0, 0]),
+      pngChunk("IEND", []),
+    ]);
+    const meta = extractPngMetadataFromBuffer(png);
+    expect(meta?.width).toBe(3840);
+    expect(meta?.height).toBe(2160);
+  });
+
+  // A truncated 8-byte IHDR used to be accepted, reading height out of the
+  // CRC bytes; the spec length is 13.
+  it("rejects a short IHDR rather than reading garbage dimensions", () => {
+    const png = buildPngWithChunks([
+      pngChunk("IHDR", [0, 0, 0, 7, 0, 0, 0, 9]),
+      pngChunk("cICP", CICP_PQ),
+      pngChunk("IEND", []),
+    ]);
+    expect(extractPngMetadataFromBuffer(png)).toBeNull();
+  });
+
+  it("still rejects a PNG whose IHDR or cICP itself is corrupt", () => {
+    const badIhdr = pngChunk("IHDR", IHDR_4K);
+    badIhdr[badIhdr.length - 1] ^= 0xff;
+    expect(
+      extractPngMetadataFromBuffer(buildPngWithChunks([badIhdr, pngChunk("IEND", [])])),
+    ).toBeNull();
+  });
+});
+
+describe("crc32 works on every runtime the package declares", () => {
+  afterEach(() => {
+    vi.resetModules();
+    vi.doUnmock("node:zlib");
+  });
+
+  /** Load ffprobe.ts as it would evaluate on Node 22.0/22.1. */
+  async function loadWithoutNativeCrc32() {
+    const actual = await vi.importActual<typeof import("node:zlib")>("node:zlib");
+    vi.resetModules();
+    // zlib.crc32 landed in 22.2.0, but engine and cli both declare
+    // `"node": ">=22"` behind a major-only gate. A NAMED import of a missing
+    // export throws at module evaluation, so ffprobe.ts would fail to load
+    // entirely on those runtimes — before any PNG is touched.
+    vi.doMock("node:zlib", () => ({ ...actual, crc32: undefined }));
+    return import("./ffprobe.js");
+  }
+
+  it("parses an HDR PNG identically with the native crc32 unavailable", async () => {
+    const png = buildPngWithChunks([
+      pngChunk("IHDR", [0, 0, 0x0f, 0, 0, 0, 0x08, 0x70, 16, 2, 0, 0, 0]),
+      pngChunk("cICP", [9, 16, 0, 1]),
+      pngChunk("IEND", []),
+    ]);
+    const withNative = extractPngMetadataFromBuffer(png);
+    expect(withNative?.colorSpace?.colorTransfer).toBe("smpte2084");
+
+    const fresh = await loadWithoutNativeCrc32();
+    expect(fresh.extractPngMetadataFromBuffer(png)).toEqual(withNative);
+  });
+
+  it("rejects a corrupt chunk on the fallback path too", async () => {
+    const bad = pngChunk("IHDR", [0, 0, 0x0f, 0, 0, 0, 0x08, 0x70, 16, 2, 0, 0, 0]);
+    bad[bad.length - 1] ^= 0xff;
+    const png = buildPngWithChunks([bad, pngChunk("IEND", [])]);
+
+    const fresh = await loadWithoutNativeCrc32();
+    expect(fresh.extractPngMetadataFromBuffer(png)).toBeNull();
+  });
+});

packages/engine/src/utils/ffprobe.ts

@@ -1,6 +1,7 @@
 // fallow-ignore-file code-duplication complexity
 import { spawn } from "child_process";
 import { readFileSync } from "fs";
+import * as zlib from "node:zlib";
 import { basename, extname } from "path";
 import { redactTelemetryString } from "@hyperframes/core";
 import { FFPROBE_PATH_ENV, getFfprobeBinary } from "./ffmpegBinaries.js";
@@ -167,10 +168,24 @@ interface StillImageMetadata {
   colorSpace: VideoColorSpace | null;
 }
 
-function crc32(buf: Buffer): number {
-  let crc = 0xffffffff;
-  for (let i = 0; i < buf.length; i++) {
-    crc ^= buf[i] ?? 0;
+// node:zlib's crc32 is native and takes a running seed, so the chunk type and
+// the chunk data can be CRC'd in sequence without concatenating them into a
+// throwaway buffer: ~210 ms -> ~1.3 ms on a 12 MiB PNG.
+//
+// It landed in Node 22.2.0, and this repo declares `"node": ">=22"` with a
+// major-only runtime gate, so 22.0 and 22.1 are still supported. A NAMED
+// import of a missing export throws at module evaluation — i.e. `ffprobe.ts`
+// would fail to load at all on those, long before any PNG is parsed — so the
+// namespace import plus this capability check is deliberate. Raising the
+// floor to 22.2.0 instead would be a user-facing support change, which does
+// not belong in a PNG bug fix.
+const nativeCrc32 = typeof zlib.crc32 === "function" ? zlib.crc32 : undefined;
+
+/** Bit-at-a-time fallback for Node 22.0/22.1. Correct, just slower. */
+function crc32Fallback(data: Buffer, seed: number): number {
+  let crc = seed ^ 0xffffffff;
+  for (let i = 0; i < data.length; i++) {
+    crc ^= data[i] ?? 0;
     for (let bit = 0; bit < 8; bit++) {
       const mask = -(crc & 1);
       crc = (crc >>> 1) ^ (0xedb88320 & mask);
@@ -179,6 +194,12 @@ function crc32(buf: Buffer): number {
   return (crc ^ 0xffffffff) >>> 0;
 }
 
+function chunkCrc32(chunkType: string, chunkData: Buffer): number {
+  const typeBytes = Buffer.from(chunkType, "ascii");
+  if (nativeCrc32) return nativeCrc32(chunkData, nativeCrc32(typeBytes));
+  return crc32Fallback(chunkData, crc32Fallback(typeBytes, 0));
+}
+
 export function extractPngMetadataFromBuffer(buf: Buffer): StillImageMetadata | null {
   if (
     buf.length < 8 ||
@@ -205,10 +226,14 @@ export function extractPngMetadataFromBuffer(buf: Buffer): StillImageMetadata |
     if (pos + 12 + chunkLen > buf.length) return null;
     const chunkData = buf.subarray(pos + 8, pos + 8 + chunkLen);
     const chunkCrc = buf.readUInt32BE(pos + 8 + chunkLen);
-    const chunkBytes = Buffer.concat([Buffer.from(chunkType, "ascii"), chunkData]);
-    if (crc32(chunkBytes) !== chunkCrc) return null;
-
-    if (chunkType === "IHDR" && chunkLen >= 8) {
+    if (chunkCrc32(chunkType, chunkData) !== chunkCrc) return null;
+
+    // First IHDR only. PNG permits exactly one and it must come first, but a
+    // malformed file can carry more — without this anchor a trailing
+    // [IHDR 1x1] silently replaced the real 4K dimensions, and the producer
+    // laid out a one-pixel image. `>= 13` is the spec length; the old `>= 8`
+    // accepted a truncated header and read height out of the CRC bytes.
+    if (chunkType === "IHDR" && chunkLen >= 13 && width === 0 && height === 0) {
       width = buf.readUInt32BE(pos + 8);
       height = buf.readUInt32BE(pos + 12);
     }
@@ -242,6 +267,16 @@ export function extractPngMetadataFromBuffer(buf: Buffer): StillImageMetadata |
       };
     }
 
+    // Everything this parser extracts has been found, so stop walking.
+    //
+    // Not just an optimisation: cICP must precede IDAT (enforced above), so
+    // continuing only ever visits chunks we ignore — while making whole-file
+    // integrity a precondition for returning anything. A truncated or
+    // bad-CRC trailing chunk in an otherwise-good HDR PNG used to null the
+    // entire result, and the caller then re-throws the swallowed ffprobe
+    // error instead of using the fallback it just computed.
+    if (width > 0 && height > 0 && colorSpaceFromCicp !== null) break;
+
     if (chunkType === "IEND") break;
     pos += 12 + chunkLen;
   }