From 4e7d629b567c4d6f8713157105777e591c9129a5 Mon Sep 17 00:00:00 2001 From: sneak Date: Wed, 23 Sep 2026 04:21:09 +0000 Subject: [PATCH] Skip thumbnail repairs the server always refuses (closes #109) The server accepts a new thumbnail only from the file's owner and only when it is no larger than the thumbnail size it records. Both thumbnail helpers now skip files another account owns without fetching them. The fixer skips a file whose recorded thumbnail size is 0 or unknown before downloading it, and otherwise tries smaller encodings until the encrypted thumbnail fits, skipping the file if none does. Model: opus-5-5 --- README.md | 8 +- TODO.md | 7 ++ src/cli-commands.ts | 2 +- src/thumbnails.ts | 118 ++++++++++++++++++------ test/thumbnails/thumbnails.test.ts | 143 +++++++++++++++++++++++++++++ 5 files changed, 246 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index cc1dd38..aa2727c 100644 --- a/README.md +++ b/README.md @@ -491,7 +491,13 @@ on. The exit code is non-zero if any ML data request failed. only, because the bundled decoder (`jpeg-js`) decodes only JPEG. A non-JPEG image (PNG, HEIC) or a video is reported as `skipped` (unsupported format), kept distinct from a `failed` repair, and does not affect the exit code; a genuine -failure still exits non-zero. +failure still exits non-zero. The server accepts a new thumbnail only from the +file's owner and only when it is no larger than the thumbnail size it records +for the file. So a file another account owns, in an album shared with you, is +skipped by both thumbnail helpers without being fetched, and the fixer skips a +file whose recorded thumbnail size is 0 or unknown. Otherwise the fixer lowers +the quality and size of the thumbnail until it fits, and skips the file if even +the smallest does not. ### Backup layout diff --git a/TODO.md b/TODO.md index b487621..61c505f 100644 --- a/TODO.md +++ b/TODO.md @@ -18,6 +18,13 @@ Tag v1.0.0. # Completed Steps +- 2026-09-23: Stopped `helper fix-missing-thumbnails` retrying files the server + always refuses (issue 109). Both thumbnail helpers skip a file another account + owns without fetching it. The fixer skips a file whose recorded thumbnail size + is 0 or unknown before downloading it, and otherwise tries smaller encodings + (720 px quality 50 down to 160 px quality 20) until the encrypted thumbnail is + no larger than that size, skipping the file if none fits. + - 2026-09-23: Tested the live-photo hash check's error paths (issue 117). Tests download a live photo whose ZIP names an unknown compression method, one whose ZIP has no image entry and one with no video entry, and check that nothing is diff --git a/src/cli-commands.ts b/src/cli-commands.ts index a38c2af..4a16798 100644 --- a/src/cli-commands.ts +++ b/src/cli-commands.ts @@ -495,7 +495,7 @@ export const fixMissingThumbnailsCommand = async ( ctx.stderr.write(` Skipped: ${skipped}\n`); ctx.stderr.write(` Failed: ${failed}\n`); if (skipped > 0) { - ctx.stderr.write("\nSkipped (unsupported format):\n"); + ctx.stderr.write("\nSkipped:\n"); for (const r of results.filter((r) => r.status === "skipped")) { ctx.stderr.write( ` ${r.fileID}\t${r.title}\t${r.reason}\n`, diff --git a/src/thumbnails.ts b/src/thumbnails.ts index 4183b1e..bb51f1e 100644 --- a/src/thumbnails.ts +++ b/src/thumbnails.ts @@ -7,8 +7,21 @@ import { ApiError } from "./api/client.js"; import { encryptBlob, toBase64 } from "./crypto/index.js"; import type { EnteFile } from "./model/types.js"; -const THUMB_MAX_DIMENSION = 720; -const THUMB_JPEG_QUALITY = 50; +// The server refuses a thumbnail larger than the one it already records for the +// file (`thumbnail.size`, the encrypted size), so these encodings are tried +// from largest to smallest and the first that fits is uploaded. +const THUMB_ENCODINGS = [ + { maxDimension: 720, quality: 50 }, + { maxDimension: 720, quality: 30 }, + { maxDimension: 480, quality: 30 }, + { maxDimension: 320, quality: 20 }, + { maxDimension: 160, quality: 20 }, +]; + +// The server accepts a new thumbnail only from the file's owner, so files other +// people own in albums shared with this account are never checked or repaired. +const NOT_OWNED_REASON = + "owned by another account (only the owner can replace its thumbnail)"; export interface MissingThumbnailInfo { fileID: number; @@ -19,11 +32,12 @@ export interface MissingThumbnailInfo { // Three outcomes, not two. "fixed": a thumbnail was generated and uploaded. // "failed": something went wrong (download, encode, upload) and the file still -// has no thumbnail. "skipped": the file is a format this helper cannot -// regenerate — a video, or an image that is not a baseline JPEG. Skipped is a -// deliberate, expected outcome, not an error (issue #17): the repair path is -// JPEG-only because `jpeg-js` is, and a PNG or HEIC is left for a format-aware -// tool rather than reported as a failure. +// has no thumbnail. "skipped": the server would refuse any thumbnail for the +// file or this helper cannot regenerate it — a file another account owns, a +// recorded thumbnail size nothing fits within, a video, or an image that is +// not a baseline JPEG. Skipped is a deliberate, expected outcome, not an error +// (issue #17): the repair path is JPEG-only because `jpeg-js` is, and a PNG or +// HEIC is left for a format-aware tool rather than reported as a failure. export type ThumbnailFixStatus = "fixed" | "skipped" | "failed"; export interface ThumbnailFixResult { @@ -45,6 +59,7 @@ export type ProgressCallback = (message: string) => void; // exists, so it is logged and the file is left unreported. That distinction is // what stops `fix-missing-thumbnails` from regenerating and uploading over // thumbnails that were fine all along while the CDN was briefly returning 500s. +// Files another account owns are logged as skipped and not checked. export const listMissingThumbnails = async ( lib: Library, client: Client, @@ -52,6 +67,7 @@ export const listMissingThumbnails = async ( ): Promise => { const log = onProgress ?? (() => {}); const api = client.getApiClient(); + const { userID } = client.whoami(); const missing: MissingThumbnailInfo[] = []; const seen = new Set(); @@ -60,6 +76,13 @@ export const listMissingThumbnails = async ( for (const photo of album.photos.list()) { if (seen.has(photo.fileID)) continue; seen.add(photo.fileID); + const file = lib.getFile(album.collectionID, photo.fileID); + if (file && file.ownerID !== userID) { + log( + `[${album.name}] Skipping ${photo.title}: ${NOT_OWNED_REASON}`, + ); + continue; + } try { const stream = await api.getThumbnailStream(photo.fileID); const reader = stream.getReader(); @@ -135,17 +158,13 @@ const resizeRGBA = ( return dst; }; -const generateThumbnail = (fileBytes: Uint8Array): Uint8Array => { - const decoded = jpeg.decode(fileBytes, { - useTArray: true, - formatAsRGBA: true, - }); +const generateThumbnail = ( + decoded: { data: Uint8Array; width: number; height: number }, + maxDimension: number, + quality: number, +): Uint8Array => { const { width: srcW, height: srcH } = decoded; - const scale = Math.min( - THUMB_MAX_DIMENSION / srcW, - THUMB_MAX_DIMENSION / srcH, - 1, - ); + const scale = Math.min(maxDimension / srcW, maxDimension / srcH, 1); const dstW = Math.round(srcW * scale); const dstH = Math.round(srcH * scale); @@ -158,7 +177,7 @@ const generateThumbnail = (fileBytes: Uint8Array): Uint8Array => { const encoded = jpeg.encode( { data: pixels, width: dstW, height: dstH }, - THUMB_JPEG_QUALITY, + quality, ); return new Uint8Array(encoded.data); }; @@ -171,14 +190,35 @@ const generateThumbnail = (fileBytes: Uint8Array): Uint8Array => { const isJpeg = (bytes: Uint8Array): boolean => bytes.length >= 2 && bytes[0] === 0xff && bytes[1] === 0xd8; -// The reason a file cannot have a JPEG thumbnail regenerated for it from its -// metadata alone, before any bytes are fetched, or undefined when it might. A -// non-image (video, live photo) is unsupported outright; a still image still -// has to be checked against its actual bytes once downloaded. -const unsupportedByType = (file: EnteFile): string | undefined => { +// The reason a file cannot have a JPEG thumbnail regenerated for it, known from +// its record alone before any bytes are fetched, or undefined when it might. A +// still image still has to be checked against its actual bytes once +// downloaded. +const reasonToSkip = (file: EnteFile, userID: number): string | undefined => { + if (file.ownerID !== userID) { + return NOT_OWNED_REASON; + } if (file.metadata.fileType !== "image") { return `unsupported file type: ${file.metadata.fileType} (only JPEG images can be regenerated)`; } + if (!file.thumbnail.size) { + return `recorded thumbnail size is ${file.thumbnail.size ?? "unknown"} (the server refuses a thumbnail larger than the one it records)`; + } + return undefined; +}; + +// Encrypt the largest encoding of the decoded image whose ciphertext is no +// larger than `maxSize`, or return undefined when even the smallest is larger. +const encryptThumbnailWithin = ( + decoded: { data: Uint8Array; width: number; height: number }, + key: Uint8Array, + maxSize: number, +): { header: Uint8Array; ciphertext: Uint8Array } | undefined => { + for (const { maxDimension, quality } of THUMB_ENCODINGS) { + const thumbJpeg = generateThumbnail(decoded, maxDimension, quality); + const encrypted = encryptBlob(thumbJpeg, key); + if (encrypted.ciphertext.length <= maxSize) return encrypted; + } return undefined; }; @@ -197,6 +237,7 @@ export const fixMissingThumbnails = async ( const log = onProgress ?? (() => {}); const results: ThumbnailFixResult[] = []; const api = client.getApiClient(); + const { userID } = client.whoami(); // Resolve each requested fileID to its file record and owning album by // enumerating the library, each file taken from the first album that holds @@ -237,18 +278,19 @@ export const fixMissingThumbnails = async ( const { file, collectionName } = entry; const title = file.metadata.title; - const typeReason = unsupportedByType(file); - if (typeReason) { - log(`[${collectionName}] Skipping ${title}: ${typeReason}`); + const skipReason = reasonToSkip(file, userID); + if (skipReason) { + log(`[${collectionName}] Skipping ${title}: ${skipReason}`); results.push({ fileID, title, collection: collectionName, status: "skipped", - reason: typeReason, + reason: skipReason, }); continue; } + const maxSize = file.thumbnail.size!; try { const photo = lib.photos.byID({ fileID }); @@ -277,12 +319,28 @@ export const fixMissingThumbnails = async ( } log(`[${collectionName}] Generating thumbnail for ${title}...`); - const thumbJpeg = generateThumbnail(fileBytes); + const decoded = jpeg.decode(fileBytes, { + useTArray: true, + formatAsRGBA: true, + }); + const fitting = encryptThumbnailWithin(decoded, file.key, maxSize); + if (!fitting) { + const reason = `no thumbnail encoding fits the recorded thumbnail size of ${maxSize} bytes`; + log(`[${collectionName}] Skipping ${title}: ${reason}`); + results.push({ + fileID, + title, + collection: collectionName, + status: "skipped", + reason, + }); + continue; + } + const { header, ciphertext } = fitting; log( - `[${collectionName}] Encrypting and uploading thumbnail (${thumbJpeg.length} bytes)...`, + `[${collectionName}] Uploading thumbnail (${ciphertext.length} bytes)...`, ); - const { header, ciphertext } = encryptBlob(thumbJpeg, file.key); const md5 = createHash("md5").update(ciphertext).digest("base64"); const { objectKey, url } = await api.getUploadURL( ciphertext.length, diff --git a/test/thumbnails/thumbnails.test.ts b/test/thumbnails/thumbnails.test.ts index 940eaaa..580a312 100644 --- a/test/thumbnails/thumbnails.test.ts +++ b/test/thumbnails/thumbnails.test.ts @@ -215,6 +215,9 @@ const buildThumbMock = async (opts?: { thumbnail: { decryptionHeader: toBase64(sodium.randombytes_buf(24)), }, + // The encrypted size of the thumbnail the server records; large + // enough here that the default encoding fits. + info: { thumbSize: 1_000_000 }, updationTime: TEST_TIME, }; }; @@ -446,6 +449,32 @@ const openLib = (client: Client): Promise => precacheOriginals: false, }); +/** The mock's raw record for one file, for a test to change before login. */ +const rawFile = (m: ThumbMockState, fileID: number): Record => + m.filesByCollection[1]!.find((f) => f.id === fileID)!; + +/** Replace the original the mock serves for one file. */ +const replaceOriginal = ( + m: ThumbMockState, + fileID: number, + body: Uint8Array, +): void => { + const push = sodium.crypto_secretstream_xchacha20poly1305_init_push( + m.fileKeys[fileID]!, + ); + m.fileCiphertexts[fileID] = + sodium.crypto_secretstream_xchacha20poly1305_push( + push.state, + body, + null, + sodium.crypto_secretstream_xchacha20poly1305_TAG_FINAL, + ); + rawFile(m, fileID).file = { decryptionHeader: toBase64(push.header) }; +}; + +const isOriginalDownload = (url: string): boolean => + url.includes("files.ente.io") || url.includes("/files/download/"); + const login = (fetch: typeof globalThis.fetch, retry?: RetryOptions) => Client.login({ email: TEST_EMAIL, @@ -581,6 +610,33 @@ describe("listMissingThumbnails", () => { // Should still be 2, not 4 (each file checked only once) expect(missing.length).toBe(2); }); + + it("skips a file another account owns without fetching its thumbnail", async () => { + const otherMock = await buildThumbMock(); + rawFile(otherMock, 102).ownerID = 7; + const logs: string[] = []; + const counted = countingFetch( + buildThumbFetch(otherMock), + (url) => url.includes("thumbnails.ente.io") && url.includes("102"), + ); + const client = await login(counted.fetch); + const lib = await openLib(client); + + const missing = await listMissingThumbnails(lib, client, (msg) => + logs.push(msg), + ); + lib.close(); + + expect(missing.map((m) => m.fileID)).toEqual([101]); + expect(counted.matched()).toBe(0); + expect( + logs.some( + (l) => + l.includes("Skipping file-102.jpg") && + l.includes("another account"), + ), + ).toBe(true); + }); }); describe("fixMissingThumbnails", () => { @@ -688,6 +744,93 @@ describe("fixMissingThumbnails", () => { expect(fixMock.uploadedThumbnails.length).toBe(1); expect(fixMock.uploadedThumbnails[0]!.fileID).toBe(101); }); + + it("skips a file another account owns without downloading it", async () => { + // The server accepts a thumbnail only from the file's owner. + const fixMock = await buildThumbMock(); + rawFile(fixMock, 101).ownerID = 7; + const counted = countingFetch( + buildThumbFetch(fixMock), + isOriginalDownload, + ); + const client = await login(counted.fetch); + const lib = await openLib(client); + + const results = await fixMissingThumbnails(lib, client, [101]); + lib.close(); + + expect(results[0]!.status).toBe("skipped"); + expect(results[0]!.reason).toContain("another account"); + expect(counted.matched()).toBe(0); + expect(fixMock.uploadedThumbnails.length).toBe(0); + }); + + it("skips a file whose recorded thumbnail size is 0 without downloading it", async () => { + // The server refuses a thumbnail larger than the one it records, and + // no thumbnail is 0 bytes. + const fixMock = await buildThumbMock(); + rawFile(fixMock, 101).info = { thumbSize: 0 }; + const counted = countingFetch( + buildThumbFetch(fixMock), + isOriginalDownload, + ); + const client = await login(counted.fetch); + const lib = await openLib(client); + + const results = await fixMissingThumbnails(lib, client, [101]); + lib.close(); + + expect(results[0]!.status).toBe("skipped"); + expect(results[0]!.reason).toContain("recorded thumbnail size is 0"); + expect(counted.matched()).toBe(0); + expect(fixMock.uploadedThumbnails.length).toBe(0); + }); + + it("re-encodes smaller until the thumbnail fits the recorded size", async () => { + // A noisy 400x300 JPEG, which the default encoding (quality 50, not + // resized because it is under 720 px) cannot compress below the size + // recorded here: one byte less than that encoding's ciphertext. + const fixMock = await buildThumbMock(); + const w = 400; + const h = 300; + const noisy = new Uint8Array( + jpegJs.encode( + { + data: sodium.randombytes_buf(w * h * 4), + width: w, + height: h, + }, + 90, + ).data, + ); + replaceOriginal(fixMock, 101, noisy); + const decoded = jpegJs.decode(noisy, { + useTArray: true, + formatAsRGBA: true, + }); + const defaultSize = + jpegJs.encode(decoded, 50).data.length + + sodium.crypto_secretstream_xchacha20poly1305_ABYTES; + const recordedSize = defaultSize - 1; + rawFile(fixMock, 101).info = { thumbSize: recordedSize }; + + const client = await login(buildThumbFetch(fixMock)); + const lib = await openLib(client); + + const results = await fixMissingThumbnails(lib, client, [101]); + lib.close(); + + expect(results[0]!.status).toBe("fixed"); + const upload = fixMock.uploadedThumbnails[0]!; + expect(upload.ciphertext.length).toBeLessThanOrEqual(recordedSize); + const decrypted = decryptBlob( + upload.ciphertext, + fromBase64(upload.decryptionHeader), + fixMock.fileKeys[101]!, + ); + expect(decrypted[0]).toBe(0xff); + expect(decrypted[1]).toBe(0xd8); + }); }); describe("Client.getApiClient", () => {