From 511633fa1c76f00fe9c4e2a0935a02b5601cae6a Mon Sep 17 00:00:00 2001 From: SnapOtter Date: Sat, 25 Jul 2026 10:46:11 +0800 Subject: [PATCH] fix(files): store null instead of 0x0 for undecoded upload dimensions (#636) validateImageBuffer() intentionally reports {width: 0, height: 0} for every CLI_DECODED_FORMATS member. The file library's upload and save-result endpoints treated that 0 as a real measurement and wrote it into the DB. Adds a measuredDimensions() helper that treats non-positive width/height as unmeasured and stores null instead. Fixes #635 --- apps/api/src/routes/user-files.ts | 33 +++++++-- tests/integration/platform/user-files.test.ts | 71 +++++++++++++++++++ 2 files changed, 99 insertions(+), 5 deletions(-) diff --git a/apps/api/src/routes/user-files.ts b/apps/api/src/routes/user-files.ts index a8f4e35c..602d4e65 100644 --- a/apps/api/src/routes/user-files.ts +++ b/apps/api/src/routes/user-files.ts @@ -27,7 +27,7 @@ import { saveThumbnail, streamStoredFile, } from "../lib/file-storage.js"; -import { validateImageBuffer } from "../lib/file-validation.js"; +import { type ValidationResult, validateImageBuffer } from "../lib/file-validation.js"; import { sanitizeFilename } from "../lib/filename.js"; import { decodeToSharpCompat, needsCliDecode } from "../lib/format-decoders.js"; import { decodeHeic } from "../lib/heic-converter.js"; @@ -81,6 +81,27 @@ function extToMime(ext: string): string { return map[clean] ?? "application/octet-stream"; } +/** + * The width/height to store for a validated image, or null if they weren't + * actually measured. + * + * validateImageBuffer() reports {width: 0, height: 0} by design for + * CLI_DECODED_FORMATS members (HEIC, RAW, PSD, TGA, ...): it intentionally + * skips decoding on upload, so 0 there means "unknown", not a real size. + * 0x0 is never a genuine image dimension, so treat it the same as an + * invalid/failed validation and store null instead of the literal 0. + */ +function measuredDimensions(validation: ValidationResult | null): { + width: number | null; + height: number | null; +} { + if (!validation) return { width: null, height: null }; + return { + width: validation.width > 0 ? validation.width : null, + height: validation.height > 0 ? validation.height : null, + }; +} + function serializeFile(row: typeof schema.userFiles.$inferSelect) { return { id: row.id, @@ -294,6 +315,7 @@ export async function userFileRoutes(app: FastifyInstance): Promise { const mimeType = isValidImage ? formatToMime(validation.format) : part.mimetype || "application/octet-stream"; + const dimensions = measuredDimensions(isValidImage ? validation : null); // Persist to disk const storedName = await saveFile(safeBuffer, safeName); @@ -309,8 +331,8 @@ export async function userFileRoutes(app: FastifyInstance): Promise { storedName, mimeType, size: fileSize, - width: isValidImage ? validation.width : null, - height: isValidImage ? validation.height : null, + width: dimensions.width, + height: dimensions.height, version: 1, parentId: null, toolChain: sourceToolId ? [sourceToolId] : null, @@ -774,6 +796,7 @@ export async function userFileRoutes(app: FastifyInstance): Promise { const resultName = `${baseName}${ext}`; const mimeType = isValidImage ? formatToMime(validation.format) : extToMime(ext); + const dimensions = measuredDimensions(isValidImage ? validation : null); // Sanitize SVG results to prevent XXE, SSRF, and script injection const safeResultBuffer = isSvgBuffer(fileBuffer) ? sanitizeSvg(fileBuffer) : fileBuffer; @@ -800,8 +823,8 @@ export async function userFileRoutes(app: FastifyInstance): Promise { storedName, mimeType, size: fileSize, - width: isValidImage ? validation.width : null, - height: isValidImage ? validation.height : null, + width: dimensions.width, + height: dimensions.height, version: nextVersion, parentId, toolChain: newChain, diff --git a/tests/integration/platform/user-files.test.ts b/tests/integration/platform/user-files.test.ts index 5e1edf23..cd4b0500 100644 --- a/tests/integration/platform/user-files.test.ts +++ b/tests/integration/platform/user-files.test.ts @@ -19,6 +19,10 @@ const PNG = readFixture(fixtures.image.base.png200); const JPG = readFixture(fixtures.image.base.jpg100); const _WEBP = readFixture(fixtures.image.base.webp50); const TINY_PNG = readFixture(fixtures.image.edge.px1); +// BMP is a CLI_DECODED_FORMATS member (see file-validation.ts): validateImageBuffer() +// intentionally returns {valid: true, width: 0, height: 0} for it without decoding, +// since real decoding happens lazily elsewhere. Used to cover the 0x0-vs-null bug. +const BMP = readFixture(fixtures.image.formats("bmp")); let testApp: TestApp; let app: TestApp["app"]; @@ -99,6 +103,31 @@ describe("File upload", () => { const result = JSON.parse(res.body); expect(result.error).toMatch(/no valid files/i); }); + + // Regression test for #635: validateImageBuffer() reports {width: 0, height: 0} + // by design for CLI_DECODED_FORMATS members (never decoded on upload), and the + // route must treat that sentinel as "unknown" (null), not write the literal 0x0. + it("stores null dimensions (not 0x0) for a CLI-decoded format upload", async () => { + const { body, contentType } = createMultipartPayload([ + { name: "file", filename: "photo.bmp", contentType: "image/bmp", content: BMP }, + ]); + + const res = await app.inject({ + method: "POST", + url: "/api/v1/files/upload", + headers: { + "content-type": contentType, + authorization: `Bearer ${adminToken}`, + }, + body, + }); + + expect(res.statusCode).toBe(201); + const result = JSON.parse(res.body); + expect(result.files).toHaveLength(1); + expect(result.files[0].width).toBeNull(); + expect(result.files[0].height).toBeNull(); + }); }); // ── List ───────────────────────────────────────────────────────── @@ -477,6 +506,48 @@ describe("Save result", () => { expect(res.statusCode).toBe(404); }); + // Regression test for #635: same 0x0-vs-null bug as the upload endpoint, but on + // the separate save-result write path (a tool result saved as a new version). + it("stores null dimensions (not 0x0) for a CLI-decoded format result", async () => { + // Upload the parent file + const { body: uploadBody, contentType: uploadCt } = createMultipartPayload([ + { + name: "file", + filename: "parent-for-bmp-result.png", + contentType: "image/png", + content: PNG, + }, + ]); + + const uploadRes = await app.inject({ + method: "POST", + url: "/api/v1/files/upload", + headers: { "content-type": uploadCt, authorization: `Bearer ${adminToken}` }, + body: uploadBody, + }); + + const parentId = JSON.parse(uploadRes.body).files[0].id; + + // Save a BMP "processed" result as a new version + const { body: saveBody, contentType: saveCt } = createMultipartPayload([ + { name: "file", filename: "result.bmp", contentType: "image/bmp", content: BMP }, + { name: "parentId", content: parentId }, + { name: "toolId", content: "convert" }, + ]); + + const res = await app.inject({ + method: "POST", + url: "/api/v1/files/save-result", + headers: { "content-type": saveCt, authorization: `Bearer ${adminToken}` }, + body: saveBody, + }); + + expect(res.statusCode).toBe(201); + const result = JSON.parse(res.body); + expect(result.file.width).toBeNull(); + expect(result.file.height).toBeNull(); + }); + it("builds version chain correctly across multiple saves", async () => { // Upload parent const { body: uploadBody, contentType: uploadCt } = createMultipartPayload([