diff --git a/TODO.md b/TODO.md index a3b390e..4316a42 100644 --- a/TODO.md +++ b/TODO.md @@ -18,6 +18,13 @@ Tag v1.0.0. # Completed Steps +- 2026-09-23: Hardened the JPEG EXIF scan behind `backup-metadata --exif` (issue + 11). Every segment length is checked against the remaining bytes and lengths + under 2 stop the scan, so a truncated or corrupt original can neither throw + nor loop. A malformed or unparseable EXIF segment is recorded as + `imageMetadata.exifError`, and a failure to read the original as + `imageMetadataError` in the per-file JSON, instead of the field being left + out. - 2026-09-22: Stopped `make test` collecting tests from checkouts nested under `.claude/` (issue 25). vitest ignores `.gitignore` when finding tests, so a nested checkout ran the whole suite again; `vitest.config.ts` now adds diff --git a/src/metadata-backup.ts b/src/metadata-backup.ts index 92c068e..b022189 100644 --- a/src/metadata-backup.ts +++ b/src/metadata-backup.ts @@ -15,18 +15,35 @@ export interface MetadataBackupOptions { onProgress?: ProgressCallback; } -// Extract the raw EXIF APP1 segment from JPEG bytes. Returns the EXIF -// data buffer (starting after the APP1 length field, at the "Exif\0\0" -// header) or undefined if no APP1 marker is found. -const extractExifFromJpeg = (buf: Uint8Array): Buffer | undefined => { - if (buf[0] !== 0xff || buf[1] !== 0xd8) return undefined; +// Find the raw EXIF APP1 segment in JPEG bytes. Returns `exif` (the segment +// data, starting at the "Exif\0\0" header) when there is one, nothing when the +// bytes are not a JPEG or carry no EXIF, and `error` when the segment layout is +// malformed. Each segment length is checked against the bytes that remain and +// each step moves forward by at least 4 bytes, so the scan ends on any input. +export const extractExifFromJpeg = ( + buf: Uint8Array, +): { exif?: Buffer; error?: string } => { + if (buf[0] !== 0xff || buf[1] !== 0xd8) return {}; let offset = 2; - while (offset < buf.length - 1) { - if (buf[offset] !== 0xff) return undefined; + while (offset < buf.length) { + if (offset + 2 > buf.length) + return { error: `truncated segment marker at byte ${offset}` }; + if (buf[offset] !== 0xff) + return { error: `no segment marker at byte ${offset}` }; const marker = buf[offset + 1]!; - if (marker === 0xda) break; // start of scan, no more markers - if (offset + 3 >= buf.length) break; + if (marker === 0xda) return {}; // start of scan, no more markers + if (offset + 4 > buf.length) + return { error: `truncated segment length at byte ${offset}` }; const len = (buf[offset + 2]! << 8) | buf[offset + 3]!; + // The length counts its own two bytes, so anything under 2 is invalid. + if (len < 2) + return { + error: `segment length ${len} at byte ${offset} is too small`, + }; + if (offset + 2 + len > buf.length) + return { + error: `segment length ${len} at byte ${offset} runs past the end of the file`, + }; if (marker === 0xe1) { // APP1 — check for "Exif\0\0" header if ( @@ -35,65 +52,70 @@ const extractExifFromJpeg = (buf: Uint8Array): Buffer | undefined => { buf[offset + 6] === 0x69 && buf[offset + 7] === 0x66 ) { - return Buffer.from( - buf.buffer, - buf.byteOffset + offset + 4, - len - 2, - ); + return { + exif: Buffer.from( + buf.buffer, + buf.byteOffset + offset + 4, + len - 2, + ), + }; } } offset += 2 + len; } - return undefined; + return { error: "file ends before the image data" }; }; -const extractImageMetadata = ( +// Extract dimensions, EXIF and XMP from a file's bytes. When the EXIF segment +// is malformed or cannot be parsed, the record carries the reason in +// `exifError`. +export const extractImageMetadata = ( fileBytes: Uint8Array, ): Record | undefined => { + const result: Record = {}; + + // Try to get dimensions from JPEG decode try { - const result: Record = {}; - - // Try to get dimensions from JPEG decode - try { - const decoded = jpeg.decode(fileBytes, { - useTArray: true, - formatAsRGBA: false, - }); - result.format = "jpeg"; - result.width = decoded.width; - result.height = decoded.height; - } catch { - // Not a JPEG or corrupt; still try EXIF extraction - } - - const exifBuf = extractExifFromJpeg(fileBytes); - if (exifBuf) { - try { - result.exif = exifReader(exifBuf); - } catch { - result.exifRaw = exifBuf.toString("base64"); - } - } - - // Extract XMP (look for "http://ns.adobe.com/xap" in the bytes) - const xmpStart = Buffer.from(fileBytes).indexOf("", xmpEnd); - result.xmp = Buffer.from(fileBytes) - .subarray(xmpStart, end !== -1 ? end + 2 : xmpEnd + 50) - .toString("utf-8"); - } - } - - return Object.keys(result).length > 0 ? result : undefined; + const decoded = jpeg.decode(fileBytes, { + useTArray: true, + formatAsRGBA: false, + }); + result.format = "jpeg"; + result.width = decoded.width; + result.height = decoded.height; } catch { - return undefined; + // Not every original is a JPEG (PNG, HEIC, video), so a failed decode + // is expected and only means no dimensions; a malformed JPEG is still + // reported below through `exifError`. } + + const { exif, error } = extractExifFromJpeg(fileBytes); + if (error) result.exifError = error; + if (exif) { + try { + result.exif = exifReader(exif); + } catch (err) { + result.exifRaw = exif.toString("base64"); + result.exifError = err instanceof Error ? err.message : String(err); + } + } + + // Extract XMP (look for "http://ns.adobe.com/xap" in the bytes) + const xmpStart = Buffer.from(fileBytes).indexOf("", xmpEnd); + result.xmp = Buffer.from(fileBytes) + .subarray(xmpStart, end !== -1 ? end + 2 : xmpEnd + 50) + .toString("utf-8"); + } + } + + return Object.keys(result).length > 0 ? result : undefined; }; // Read a file's original bytes through the library's content cache and extract @@ -103,13 +125,9 @@ const extractImageMetadata = ( const extractExif = async ( photo: Photo, ): Promise | undefined> => { - try { - const { path } = await photo.original(); - const fileBytes = new Uint8Array(readFileSync(path)); - return extractImageMetadata(fileBytes); - } catch { - return undefined; - } + const { path } = await photo.original(); + const fileBytes = new Uint8Array(readFileSync(path)); + return extractImageMetadata(fileBytes); }; // Dump every decrypted metadata layer the account holds into a directory tree @@ -215,8 +233,13 @@ export const runMetadataBackup = async ( if (wantExif && !writtenFileIDs.has(file.id)) { log(`[${file.metadata.title}] Extracting EXIF...`); - const exifData = await extractExif(photo); - if (exifData) fileMeta.imageMetadata = exifData; + try { + const exifData = await extractExif(photo); + if (exifData) fileMeta.imageMetadata = exifData; + } catch (err) { + fileMeta.imageMetadataError = + err instanceof Error ? err.message : String(err); + } } writtenFileIDs.add(file.id); diff --git a/test/cli/metadata-backup.test.ts b/test/cli/metadata-backup.test.ts index 560ca76..b1f435b 100644 --- a/test/cli/metadata-backup.test.ts +++ b/test/cli/metadata-backup.test.ts @@ -621,5 +621,18 @@ describe("quak backup-metadata", () => { expect(fileMeta.imageMetadata.format).toBe("jpeg"); expect(fileMeta.imageMetadata.width).toBe(100); expect(fileMeta.imageMetadata.height).toBe(80); + expect(fileMeta.imageMetadataError).toBeUndefined(); + + // File 200 has no original on the mock server, so extraction fails + // and the reason is recorded instead of the field being left out. + const workDir = collDirs.find((d) => d.includes("Work"))!; + const failedMeta = JSON.parse( + readFileSync( + join(outDir, "collections", workDir, "200.json"), + "utf-8", + ), + ); + expect(failedMeta.imageMetadata).toBeUndefined(); + expect(failedMeta.imageMetadataError).toEqual(expect.any(String)); }); }); diff --git a/test/cli/metadata-exif.test.ts b/test/cli/metadata-exif.test.ts new file mode 100644 index 0000000..fdaafa2 --- /dev/null +++ b/test/cli/metadata-exif.test.ts @@ -0,0 +1,122 @@ +/** + * Tests for the JPEG EXIF scan behind `quak backup-metadata --exif`. + * + * The originals come from users' libraries, so a truncated or corrupt JPEG + * must neither hang the scan nor throw out of it, and a malformed file must be + * told apart from one that simply has no EXIF: the record carries the reason in + * `exifError`. Each input below is a short hand-built byte array. + */ + +import { describe, expect, it } from "vitest"; +import { + extractExifFromJpeg, + extractImageMetadata, +} from "../../src/metadata-backup.js"; + +const SOI = [0xff, 0xd8]; // start of image +const SOS = [0xff, 0xda, 0x00, 0x02]; // start of scan, where the scan stops +const EXIF_HEADER = [0x45, 0x78, 0x69, 0x66, 0x00, 0x00]; // "Exif\0\0" + +// A big-endian TIFF block with one IFD entry: Orientation (0x0112), SHORT, 6. +const TIFF_ORIENTATION_6 = [ + 0x4d, 0x4d, 0x00, 0x2a, 0x00, 0x00, 0x00, 0x08, 0x00, 0x01, 0x01, 0x12, + 0x00, 0x03, 0x00, 0x00, 0x00, 0x01, 0x00, 0x06, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, +]; + +// An APP1 segment whose length field matches its data. +const app1 = (data: number[]): number[] => { + const len = data.length + 2; + return [0xff, 0xe1, len >> 8, len & 0xff, ...data]; +}; + +const bytes = (...parts: number[][]): Uint8Array => + new Uint8Array(parts.flat()); + +describe("extractExifFromJpeg", () => { + it("returns the EXIF segment of a valid JPEG", () => { + const data = [...EXIF_HEADER, ...TIFF_ORIENTATION_6]; + const scan = extractExifFromJpeg(bytes(SOI, app1(data), SOS)); + expect(scan.error).toBeUndefined(); + expect([...scan.exif!]).toEqual(data); + }); + + it("returns nothing for a file that is not a JPEG", () => { + const png = bytes([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]); + expect(extractExifFromJpeg(png)).toEqual({}); + }); + + it("returns nothing for a JPEG without EXIF", () => { + const app0 = [0xff, 0xe0, 0x00, 0x04, 0x00, 0x00]; + expect(extractExifFromJpeg(bytes(SOI, app0, SOS))).toEqual({}); + }); + + it("reports a JPEG truncated inside a segment header", () => { + const scan = extractExifFromJpeg(bytes(SOI, [0xff, 0xe1, 0x00])); + expect(scan.exif).toBeUndefined(); + expect(scan.error).toMatch(/truncated segment length/); + }); + + it("reports a JPEG that ends before the image data", () => { + const app0 = [0xff, 0xe0, 0x00, 0x04, 0x00, 0x00]; + const scan = extractExifFromJpeg(bytes(SOI, app0)); + expect(scan.error).toMatch(/ends before the image data/); + }); + + it("stops on a zero-length segment instead of looping", () => { + // A length of 0 would otherwise step the scan by 2 bytes at a time + // through the rest of the file, reading garbage as markers. + const zero = [0xff, 0xe0, 0x00, 0x00]; + const scan = extractExifFromJpeg( + bytes(SOI, zero, zero, zero, zero, SOS), + ); + expect(scan.error).toMatch(/segment length 0 at byte 2 is too small/); + }); + + it("stops on a segment length of 1", () => { + const scan = extractExifFromJpeg( + bytes(SOI, [0xff, 0xe0, 0x00, 0x01], SOS), + ); + expect(scan.error).toMatch(/segment length 1 at byte 2 is too small/); + }); + + it("reports a segment length that runs past the end of the file", () => { + // APP1 claims 0x4000 bytes but only the "Exif\0\0" header follows. + const scan = extractExifFromJpeg( + bytes(SOI, [0xff, 0xe1, 0x40, 0x00], EXIF_HEADER), + ); + expect(scan.exif).toBeUndefined(); + expect(scan.error).toMatch(/runs past the end of the file/); + }); +}); + +describe("extractImageMetadata", () => { + it("parses EXIF from a valid JPEG", () => { + const meta = extractImageMetadata( + bytes(SOI, app1([...EXIF_HEADER, ...TIFF_ORIENTATION_6]), SOS), + ); + expect(meta?.exifError).toBeUndefined(); + expect(meta?.exif).toMatchObject({ Image: { Orientation: 6 } }); + }); + + it("returns nothing for a file that is not a JPEG", () => { + const text = new TextEncoder().encode("just some text, not an image"); + expect(extractImageMetadata(text)).toBeUndefined(); + }); + + it("records the reason when the JPEG is malformed", () => { + const meta = extractImageMetadata( + bytes(SOI, [0xff, 0xe1, 0x40, 0x00], EXIF_HEADER), + ); + expect(meta?.exif).toBeUndefined(); + expect(meta?.exifError).toMatch(/runs past the end of the file/); + }); + + it("keeps the raw bytes and the reason when EXIF cannot be parsed", () => { + const data = [...EXIF_HEADER, 0x58, 0x58]; + const meta = extractImageMetadata(bytes(SOI, app1(data), SOS)); + expect(meta?.exif).toBeUndefined(); + expect(meta?.exifRaw).toBe(Buffer.from(data).toString("base64")); + expect(meta?.exifError).toEqual(expect.any(String)); + }); +});