mirror of
https://github.com/snapotter-hq/SnapOtter.git
synced 2026-08-03 07:46:42 +02:00
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
This commit is contained in:
@@ -27,7 +27,7 @@ import {
|
|||||||
saveThumbnail,
|
saveThumbnail,
|
||||||
streamStoredFile,
|
streamStoredFile,
|
||||||
} from "../lib/file-storage.js";
|
} 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 { sanitizeFilename } from "../lib/filename.js";
|
||||||
import { decodeToSharpCompat, needsCliDecode } from "../lib/format-decoders.js";
|
import { decodeToSharpCompat, needsCliDecode } from "../lib/format-decoders.js";
|
||||||
import { decodeHeic } from "../lib/heic-converter.js";
|
import { decodeHeic } from "../lib/heic-converter.js";
|
||||||
@@ -81,6 +81,27 @@ function extToMime(ext: string): string {
|
|||||||
return map[clean] ?? "application/octet-stream";
|
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) {
|
function serializeFile(row: typeof schema.userFiles.$inferSelect) {
|
||||||
return {
|
return {
|
||||||
id: row.id,
|
id: row.id,
|
||||||
@@ -294,6 +315,7 @@ export async function userFileRoutes(app: FastifyInstance): Promise<void> {
|
|||||||
const mimeType = isValidImage
|
const mimeType = isValidImage
|
||||||
? formatToMime(validation.format)
|
? formatToMime(validation.format)
|
||||||
: part.mimetype || "application/octet-stream";
|
: part.mimetype || "application/octet-stream";
|
||||||
|
const dimensions = measuredDimensions(isValidImage ? validation : null);
|
||||||
|
|
||||||
// Persist to disk
|
// Persist to disk
|
||||||
const storedName = await saveFile(safeBuffer, safeName);
|
const storedName = await saveFile(safeBuffer, safeName);
|
||||||
@@ -309,8 +331,8 @@ export async function userFileRoutes(app: FastifyInstance): Promise<void> {
|
|||||||
storedName,
|
storedName,
|
||||||
mimeType,
|
mimeType,
|
||||||
size: fileSize,
|
size: fileSize,
|
||||||
width: isValidImage ? validation.width : null,
|
width: dimensions.width,
|
||||||
height: isValidImage ? validation.height : null,
|
height: dimensions.height,
|
||||||
version: 1,
|
version: 1,
|
||||||
parentId: null,
|
parentId: null,
|
||||||
toolChain: sourceToolId ? [sourceToolId] : null,
|
toolChain: sourceToolId ? [sourceToolId] : null,
|
||||||
@@ -774,6 +796,7 @@ export async function userFileRoutes(app: FastifyInstance): Promise<void> {
|
|||||||
const resultName = `${baseName}${ext}`;
|
const resultName = `${baseName}${ext}`;
|
||||||
|
|
||||||
const mimeType = isValidImage ? formatToMime(validation.format) : extToMime(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
|
// Sanitize SVG results to prevent XXE, SSRF, and script injection
|
||||||
const safeResultBuffer = isSvgBuffer(fileBuffer) ? sanitizeSvg(fileBuffer) : fileBuffer;
|
const safeResultBuffer = isSvgBuffer(fileBuffer) ? sanitizeSvg(fileBuffer) : fileBuffer;
|
||||||
@@ -800,8 +823,8 @@ export async function userFileRoutes(app: FastifyInstance): Promise<void> {
|
|||||||
storedName,
|
storedName,
|
||||||
mimeType,
|
mimeType,
|
||||||
size: fileSize,
|
size: fileSize,
|
||||||
width: isValidImage ? validation.width : null,
|
width: dimensions.width,
|
||||||
height: isValidImage ? validation.height : null,
|
height: dimensions.height,
|
||||||
version: nextVersion,
|
version: nextVersion,
|
||||||
parentId,
|
parentId,
|
||||||
toolChain: newChain,
|
toolChain: newChain,
|
||||||
|
|||||||
@@ -19,6 +19,10 @@ const PNG = readFixture(fixtures.image.base.png200);
|
|||||||
const JPG = readFixture(fixtures.image.base.jpg100);
|
const JPG = readFixture(fixtures.image.base.jpg100);
|
||||||
const _WEBP = readFixture(fixtures.image.base.webp50);
|
const _WEBP = readFixture(fixtures.image.base.webp50);
|
||||||
const TINY_PNG = readFixture(fixtures.image.edge.px1);
|
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 testApp: TestApp;
|
||||||
let app: TestApp["app"];
|
let app: TestApp["app"];
|
||||||
@@ -99,6 +103,31 @@ describe("File upload", () => {
|
|||||||
const result = JSON.parse(res.body);
|
const result = JSON.parse(res.body);
|
||||||
expect(result.error).toMatch(/no valid files/i);
|
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 ─────────────────────────────────────────────────────────
|
// ── List ─────────────────────────────────────────────────────────
|
||||||
@@ -477,6 +506,48 @@ describe("Save result", () => {
|
|||||||
expect(res.statusCode).toBe(404);
|
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 () => {
|
it("builds version chain correctly across multiple saves", async () => {
|
||||||
// Upload parent
|
// Upload parent
|
||||||
const { body: uploadBody, contentType: uploadCt } = createMultipartPayload([
|
const { body: uploadBody, contentType: uploadCt } = createMultipartPayload([
|
||||||
|
|||||||
Reference in New Issue
Block a user