feat(server): report image dimensions with signed asset URLs#10198

merged4
main
media-dims/contract
openedSep 5, 2026byJJulius Marminge
View on GitHubTwo-way sync enabledPR last imported 4d ago
5 unresolved threads · Jump to file
Selected thread: apps/server/src/assets/AssetAccess.ts:398 — 🟠 **High** `assets/AssetAccess.ts:398` `assetsCreateUrl` can hang when the workspace image is replaced by a FIFO: `readImageDimensionsFromHeader` reopens it with blocking flag `"r"`, waiting indefinitely for a writer and potentially exhausting the filesystem worker pool. Read the header through `openMediaFile` instead so the existing `O_NONBLOCK` protection is preserved. <details> <summary>🚀 Reply "<strong>fix it for me</strong>" or copy this <strong>AI Prompt</strong> for your agent:</summary> ```text In file @apps/server/src/assets/AssetAccess.ts around line 398: `assetsCreateUrl` can hang when the workspace image is replaced by a FIFO: `readImageDimensionsFromHeader` reopens it with blocking flag `"r"`, waiting indefinitely for a writer and potentially exhausting the filesystem worker pool. Read the header through `openMediaFile` instead so the existing `O_NONBLOCK` protection is preserved. ``` </details> <!-- macroscope-meta: {"kind":"code_review","variant":"f57f0b61c1d8f13d673564c863408287"} -->
0 / 8 viewed8 files changedLine totals unavailable

packages/shared/src/imageDimensions.ts · Earlier revision c97ccee2 · right 110–111

Original diff
@@ -0,0 +1,143 @@
+/**
+ * Reads pixel dimensions from the header bytes of a PNG, JPEG, GIF, or WebP
+ * file so a client can reserve the exact box before the bytes arrive. Any
+ * other format, a truncated header, or a malformed file yields null; callers
+ * fall back to measuring after decode.
+ */
+export interface ImageDimensions {
+  readonly width: number;
+  readonly height: number;
+}
+
+/** Enough for every supported header, including JPEG files with an EXIF/ICC prefix. */
+export const IMAGE_DIMENSIONS_HEADER_BYTES = 64 * 1024;
+
+export function readImageDimensions(bytes: Uint8Array): ImageDimensions | null {
+  const dimensions = readPng(bytes) ?? readGif(bytes) ?? readWebp(bytes) ?? readJpeg(bytes);
+  return dimensions && dimensions.width > 0 && dimensions.height > 0 ? dimensions : null;
+}
+
+const view = (bytes: Uint8Array) => new DataView(bytes.buffer, bytes.byteOffset, bytes.byteLength);
+
+function readPng(bytes: Uint8Array): ImageDimensions | null {
+  if (bytes.length < 24) return null;
+  if (
+    bytes[0] !== 0x89 ||
+    bytes[1] !== 0x50 ||
+    bytes[2] !== 0x4e ||
+    bytes[3] !== 0x47 ||
+    bytes[12] !== 0x49 ||
+    bytes[13] !== 0x48 ||
+    bytes[14] !== 0x44 ||
+    bytes[15] !== 0x52
+  ) {
+    return null;
+  }
+  const data = view(bytes);
+  return { width: data.getUint32(16), height: data.getUint32(20) };
+}
+
+function readGif(bytes: Uint8Array): ImageDimensions | null {
+  if (bytes.length < 10) return null;
+  if (bytes[0] !== 0x47 || bytes[1] !== 0x49 || bytes[2] !== 0x46 || bytes[3] !== 0x38) return null;
+  const data = view(bytes);
+  return { width: data.getUint16(6, true), height: data.getUint16(8, true) };
+}
+
+function readWebp(bytes: Uint8Array): ImageDimensions | null {
+  if (bytes.length < 16) return null;
+  if (
+    bytes[0] !== 0x52 ||
+    bytes[1] !== 0x49 ||
+    bytes[2] !== 0x46 ||
+    bytes[3] !== 0x46 ||
+    bytes[8] !== 0x57 ||
+    bytes[9] !== 0x45 ||
+    bytes[10] !== 0x42 ||
+    bytes[11] !== 0x50
+  ) {
+    return null;
+  }
+  const data = view(bytes);
+  const chunk = String.fromCharCode(bytes[12]!, bytes[13]!, bytes[14]!, bytes[15]!);
+  switch (chunk) {
+    case "VP8 ":
+      if (bytes.length < 30) return null;
+      // Lossy: 14-bit dimensions after the 3-byte frame tag and 3-byte start code.
+      return {
+        width: data.getUint16(26, true) & 0x3fff,
+        height: data.getUint16(28, true) & 0x3fff,
+      };
+    case "VP8L": {
+      if (bytes.length < 25) return null;
+      // Lossless: width-1 in bits 0-13 and height-1 in bits 14-27 of the
+      // 32 bits after the signature byte.
+      const packed = data.getUint32(21, true);
+      return { width: (packed & 0x3fff) + 1, height: ((packed >>> 14) & 0x3fff) + 1 };
+    }
+    case "VP8X":
+      if (bytes.length < 30) return null;
+      // Extended: 24-bit canvas dimensions minus one.
+      return {
+        width: (bytes[24]! | (bytes[25]! << 8) | (bytes[26]! << 16)) + 1,
+        height: (bytes[27]! | (bytes[28]! << 8) | (bytes[29]! << 16)) + 1,
+      };
+    default:
+      return null;
+  }
+}
+
+function readJpeg(bytes: Uint8Array): ImageDimensions | null {
+  if (bytes.length < 4 || bytes[0] !== 0xff || bytes[1] !== 0xd8) return null;
+  const data = view(bytes);
+  let offset = 2;
+  let rotated = false;
+  while (offset + 9 <= bytes.length) {
+    if (bytes[offset] !== 0xff) return null;
+    const marker = bytes[offset + 1]!;
+    // Padding bytes between segments.
+    if (marker === 0xff) {
+      offset += 1;
+      continue;
+    }
+    // Start-of-frame markers carry the dimensions; skip the arithmetic-coding
+    // and Huffman-table markers that share the range.
+    if (marker >= 0xc0 && marker <= 0xcf && marker !== 0xc4 && marker !== 0xc8 && marker !== 0xcc) {
+      const height = data.getUint16(offset + 5);
+      const width = data.getUint16(offset + 7);
+      return rotated ? { width: height, height: width } : { width, height };
+    }
+    if (marker === 0xd9 || marker === 0xda) return null;
+    const length = data.getUint16(offset + 2);

packages/shared/src/imageDimensions.ts · right 110–111

M
macroscopeappBotSep 5, 2026

🟡 Medium src/imageDimensions.ts:110

readJpeg returns null for valid JPEG streams containing a TEM marker before SOF, so imageDimensions is omitted and the layout box cannot be reserved. Because TEM (0xff01) is standalone, handle it by advancing two bytes without reading a length field.

diff
1 if (marker === 0xd9 || marker === 0xda) return null;
2+ if (marker === 0x01) {
3+ offset += 2;
4+ continue;
5+ }
6 const length = data.getUint16(offset + 2);
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
text
1In file @packages/shared/src/imageDimensions.ts around lines 110-111:
2
3`readJpeg` returns `null` for valid JPEG streams containing a `TEM` marker before `SOF`, so `imageDimensions` is omitted and the layout box cannot be reserved. Because `TEM` (`0xff01`) is standalone, handle it by advancing two bytes without reading a length field.
J
Julius MarmingeSep 5, 2026

Fixed in 104ad78637. TEM (0xff01) and the restart markers (0xffd0–0xffd7) are stepped over as two bytes with no length field; test added.

apps/server/src/assets/AssetAccess.ts · Earlier revision c97ccee2 · right 398

Original diff
@@ -347,6 +394,9 @@ export const issueAssetUrl = Effect.fn("AssetAccess.issueAssetUrl")(function* (i
             }),
         ),
       );
+      if (HEADER_IMAGE_EXTENSIONS.has(path.extname(resolved.relativePath).toLowerCase())) {
+        imageDimensions = yield* readImageDimensionsFromHeader(canonicalFile);
M
macroscopeappBotSep 5, 2026

🟠 High assets/AssetAccess.ts:398

assetsCreateUrl can hang when the workspace image is replaced by a FIFO: readImageDimensionsFromHeader reopens it with blocking flag "r", waiting indefinitely for a writer and potentially exhausting the filesystem worker pool. Read the header through openMediaFile instead so the existing O_NONBLOCK protection is preserved.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
text
1In file @apps/server/src/assets/AssetAccess.ts around line 398:
2
3`assetsCreateUrl` can hang when the workspace image is replaced by a FIFO: `readImageDimensionsFromHeader` reopens it with blocking flag `"r"`, waiting indefinitely for a writer and potentially exhausting the filesystem worker pool. Read the header through `openMediaFile` instead so the existing `O_NONBLOCK` protection is preserved.
J
Julius MarmingeSep 5, 2026

Fixed in 104ad78637. Every header read now goes through openMediaFile (O_NONBLOCK | O_NOFOLLOW plus the lstat/inode checks), including the workspace-file and attachment paths that previously used a plain FileSystem.open. A FIFO returns null from openMediaFile's isFile() check before any read.

packages/shared/src/imageDimensions.ts · Earlier revision 104ad786 · right 119–119

Original diff
@@ -0,0 +1,148 @@
+/**
+ * Reads pixel dimensions from the header bytes of a PNG, JPEG, GIF, or WebP
+ * file so a client can reserve the exact box before the bytes arrive. Any
+ * other format, a truncated header, or a malformed file yields null; callers
+ * fall back to measuring after decode.
+ */
+export interface ImageDimensions {
+  readonly width: number;
+  readonly height: number;
+}
+
+/** Enough for every supported header, including JPEG files with an EXIF/ICC prefix. */
+export const IMAGE_DIMENSIONS_HEADER_BYTES = 64 * 1024;
+
+export function readImageDimensions(bytes: Uint8Array): ImageDimensions | null {
+  const dimensions = readPng(bytes) ?? readGif(bytes) ?? readWebp(bytes) ?? readJpeg(bytes);
+  return dimensions && dimensions.width > 0 && dimensions.height > 0 ? dimensions : null;
+}
+
+const view = (bytes: Uint8Array) => new DataView(bytes.buffer, bytes.byteOffset, bytes.byteLength);
+
+function readPng(bytes: Uint8Array): ImageDimensions | null {
+  if (bytes.length < 24) return null;
+  if (
+    bytes[0] !== 0x89 ||
+    bytes[1] !== 0x50 ||
+    bytes[2] !== 0x4e ||
+    bytes[3] !== 0x47 ||
+    bytes[12] !== 0x49 ||
+    bytes[13] !== 0x48 ||
+    bytes[14] !== 0x44 ||
+    bytes[15] !== 0x52
+  ) {
+    return null;
+  }
+  const data = view(bytes);
+  return { width: data.getUint32(16), height: data.getUint32(20) };
+}
+
+function readGif(bytes: Uint8Array): ImageDimensions | null {
+  if (bytes.length < 10) return null;
+  if (bytes[0] !== 0x47 || bytes[1] !== 0x49 || bytes[2] !== 0x46 || bytes[3] !== 0x38) return null;
+  const data = view(bytes);
+  return { width: data.getUint16(6, true), height: data.getUint16(8, true) };
+}
+
+function readWebp(bytes: Uint8Array): ImageDimensions | null {
+  if (bytes.length < 16) return null;
+  if (
+    bytes[0] !== 0x52 ||
+    bytes[1] !== 0x49 ||
+    bytes[2] !== 0x46 ||
+    bytes[3] !== 0x46 ||
+    bytes[8] !== 0x57 ||
+    bytes[9] !== 0x45 ||
+    bytes[10] !== 0x42 ||
+    bytes[11] !== 0x50
+  ) {
+    return null;
+  }
+  const data = view(bytes);
+  const chunk = String.fromCharCode(bytes[12]!, bytes[13]!, bytes[14]!, bytes[15]!);
+  switch (chunk) {
+    case "VP8 ":
+      if (bytes.length < 30) return null;
+      // Lossy: 14-bit dimensions after the 3-byte frame tag and 3-byte start code.
+      return {
+        width: data.getUint16(26, true) & 0x3fff,
+        height: data.getUint16(28, true) & 0x3fff,
+      };
+    case "VP8L": {
+      if (bytes.length < 25) return null;
+      // Lossless: width-1 in bits 0-13 and height-1 in bits 14-27 of the
+      // 32 bits after the signature byte.
+      const packed = data.getUint32(21, true);
+      return { width: (packed & 0x3fff) + 1, height: ((packed >>> 14) & 0x3fff) + 1 };
+    }
+    case "VP8X":
+      if (bytes.length < 30) return null;
+      // Extended: 24-bit canvas dimensions minus one.
+      return {
+        width: (bytes[24]! | (bytes[25]! << 8) | (bytes[26]! << 16)) + 1,
+        height: (bytes[27]! | (bytes[28]! << 8) | (bytes[29]! << 16)) + 1,
+      };
+    default:
+      return null;
+  }
+}
+
+function readJpeg(bytes: Uint8Array): ImageDimensions | null {
+  if (bytes.length < 4 || bytes[0] !== 0xff || bytes[1] !== 0xd8) return null;
+  const data = view(bytes);
+  let offset = 2;
+  let rotated = false;
+  while (offset + 9 <= bytes.length) {
+    if (bytes[offset] !== 0xff) return null;
+    const marker = bytes[offset + 1]!;
+    // Padding bytes between segments.
+    if (marker === 0xff) {
+      offset += 1;
+      continue;
+    }
+    // Start-of-frame markers carry the dimensions; skip the arithmetic-coding
+    // and Huffman-table markers that share the range.
+    if (marker >= 0xc0 && marker <= 0xcf && marker !== 0xc4 && marker !== 0xc8 && marker !== 0xcc) {
+      const height = data.getUint16(offset + 5);
+      const width = data.getUint16(offset + 7);
+      return rotated ? { width: height, height: width } : { width, height };
+    }
+    if (marker === 0xd9 || marker === 0xda) return null;
+    // TEM and the restart markers stand alone, with no length field.
+    if (marker === 0x01 || (marker >= 0xd0 && marker <= 0xd7)) {
+      offset += 2;
+      continue;
+    }
+    const length = data.getUint16(offset + 2);
+    // Viewers apply the EXIF orientation before display, so a phone photo
+    // stored on its side takes the swapped size on screen.
+    if (marker === 0xe1) rotated = exifOrientationSwapsAxes(bytes, offset + 4, offset + 2 + length);

packages/shared/src/imageDimensions.ts · right 119–119

M
macroscopeappBotSep 5, 2026

🟡 Medium src/imageDimensions.ts:119

A JPEG with EXIF orientation 5–8 followed by an XMP APP1 segment returns unswapped SOF dimensions, so clients reserve the wrong aspect ratio for the displayed image. Each APP1 assigns rotated from the current segment, allowing the later non-EXIF segment to clear the earlier rotation; preserve an already-detected rotation when processing subsequent segments.

suggestion
1 if (marker === 0xe1) rotated = rotated || exifOrientationSwapsAxes(bytes, offset + 4, offset + 2 + length);
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
text
1In file @packages/shared/src/imageDimensions.ts around line 119:
2
3A JPEG with EXIF orientation 5–8 followed by an XMP `APP1` segment returns unswapped SOF dimensions, so clients reserve the wrong aspect ratio for the displayed image. Each `APP1` assigns `rotated` from the current segment, allowing the later non-EXIF segment to clear the earlier rotation; preserve an already-detected rotation when processing subsequent segments.
J
Julius MarmingeSep 5, 2026

Fixed in 0c0d4ea243. A rotation found in an earlier APP1 is kept when a later non-EXIF APP1 (XMP) follows; test added for EXIF-then-XMP.

apps/server/src/assets/MediaFile.ts · Earlier revision c97ccee2 · right 106

Original diff
@@ -95,6 +95,17 @@ export const openMediaFile = Effect.fn("openMediaFile")(function* (
   );
 });
 
+/** Reads the leading bytes of an already-validated media file, never past the end. */
+export const readMediaFileHeader = (file: OpenMediaFile, byteCount: number) =>
+  Effect.tryPromise({
+    try: async () => {
+      const buffer = new Uint8Array(byteCount);
+      const { bytesRead } = await file.handle.read(buffer, 0, byteCount, 0);
+      return buffer.subarray(0, bytesRead);
+    },
+    catch: (cause) => new MediaFileStatError({ path: "<open handle>", cause }),
M
macroscopeappBotSep 5, 2026

Service failures should carry a real resource identifier in their attributes; path: "<open handle>" makes the derived message (Failed to read metadata for media file '<open handle>'.) non-identifying, and it also reuses the stat error tag for a read. Consider threading the file path through (as statMediaFile already does) and, if the read should be distinguishable, giving it its own tagged error:

1export const readMediaFileHeader = (filePath: string, file: OpenMediaFile, byteCount: number) => ...
2 catch: (cause) => new MediaFileReadError({ path: filePath, cause }),

The single caller in AssetAccess.issueAssetUrl already has canonicalFile available to pass in.

Posted via Macroscope — Effect Service Conventions

J
Julius MarmingeSep 5, 2026

Fixed in 104ad78637. readMediaFileHeader takes the path and fails with its own MediaFileReadError naming the file.

packages/shared/src/imageDimensions.ts · Earlier revision 104ad786 · right 13

Original diff
@@ -0,0 +1,148 @@
+/**
+ * Reads pixel dimensions from the header bytes of a PNG, JPEG, GIF, or WebP
+ * file so a client can reserve the exact box before the bytes arrive. Any
+ * other format, a truncated header, or a malformed file yields null; callers
+ * fall back to measuring after decode.
+ */
+export interface ImageDimensions {
+  readonly width: number;
+  readonly height: number;
+}
+
+/** Enough for every supported header, including JPEG files with an EXIF/ICC prefix. */
+export const IMAGE_DIMENSIONS_HEADER_BYTES = 64 * 1024;
M
macroscopeappBotSep 5, 2026

🟡 Medium src/imageDimensions.ts:13

A valid JPEG with enough APP/ICC/XMP metadata before its SOF marker returns null, so callers omit imageDimensions even though JPEG is supported. IMAGE_DIMENSIONS_HEADER_BYTES limits the supplied header to 64 KiB, but JPEG metadata may span multiple maximum-sized APP segments and place the frame header beyond that boundary; increase or remove this fixed cap, or continue fetching bytes until readJpeg reaches SOF/knows the header is incomplete.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
text
1In file @packages/shared/src/imageDimensions.ts around line 13:
2
3A valid JPEG with enough APP/ICC/XMP metadata before its SOF marker returns `null`, so callers omit `imageDimensions` even though JPEG is supported. `IMAGE_DIMENSIONS_HEADER_BYTES` limits the supplied header to 64 KiB, but JPEG metadata may span multiple maximum-sized APP segments and place the frame header beyond that boundary; increase or remove this fixed cap, or continue fetching bytes until `readJpeg` reaches SOF/knows the header is incomplete.
J
Julius MarmingeSep 5, 2026

Fixed in 0c0d4ea243. Header window is now 256 KiB, enough for a frame header behind EXIF + ICC + XMP segments. The read is bounded by the file's actual size (the handle read returns the bytes available), so small files cost nothing extra.