diff --git a/README.md b/README.md index 9ada598..038db95 100644 --- a/README.md +++ b/README.md @@ -480,7 +480,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 172c0d7..af70848 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: Checked downloaded originals against their recorded content hash (issue 68). `downloadFile`, which `quak get`, the content cache and backup all use, hashes the decrypted bytes (unkeyed BLAKE2b-512, standard base64) and diff --git a/src/cli-commands.ts b/src/cli-commands.ts index 72a9abd..601f9d1 100644 --- a/src/cli-commands.ts +++ b/src/cli-commands.ts @@ -462,7 +462,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", () => {