From 21597bdc1b6363f90675d9a56beb5b7efa2a418a Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Fri, 24 Jul 2026 15:11:41 +0200 Subject: [PATCH] Fix image uploads using the wrong format (#2380) * fix(app): preserve image attachment MIME types Image intake discarded clipboard and desktop path metadata, then guessed JPEG when a later file object had no type. Resolve MIME once at the platform boundary and require it downstream. * fix(app): honor native picker MIME metadata --- .../app/src/attachments/file-types.test.ts | 25 +++++++++ packages/app/src/attachments/file-types.ts | 25 ++++++--- .../file-drop/use-drop-listeners.ts | 13 +++-- packages/app/src/composer/actions.test.ts | 6 ++- packages/app/src/composer/actions.ts | 2 +- .../hooks/image-attachment-picker.native.ts | 4 +- .../src/hooks/image-attachment-picker.test.ts | 45 +++++++++++++--- .../app/src/hooks/image-attachment-picker.ts | 53 +++++++++++++++---- .../src/hooks/picked-image-normalizer.test.ts | 24 +++++++++ .../app/src/hooks/picked-image-normalizer.ts | 10 ++-- .../src/hooks/use-image-attachment-picker.ts | 12 ++--- .../image-attachments-from-files.test.ts | 11 ++-- .../src/utils/image-attachments-from-files.ts | 22 +++++--- 13 files changed, 198 insertions(+), 54 deletions(-) diff --git a/packages/app/src/attachments/file-types.test.ts b/packages/app/src/attachments/file-types.test.ts index a26b57c22..e3b181d95 100644 --- a/packages/app/src/attachments/file-types.test.ts +++ b/packages/app/src/attachments/file-types.test.ts @@ -6,6 +6,7 @@ import { isRasterImageMimeType, isRasterImagePath, RASTER_IMAGE_FILE_EXTENSIONS, + resolveRasterImageMimeType, } from "./file-types"; describe("attachment file types", () => { @@ -37,4 +38,28 @@ describe("attachment file types", () => { new Set(["png", "jpg", "jpeg", "gif", "webp", "bmp", "heic", "heif", "avif", "tif", "tiff"]), ); }); + + it("uses explicit raster MIME metadata before the filename", () => { + expect( + resolveRasterImageMimeType({ mimeType: "image/jpeg", path: "/tmp/screenshot.png" }), + ).toBe("image/jpeg"); + expect( + resolveRasterImageMimeType({ + mimeType: "image/png; charset=binary", + path: "/tmp/screenshot.jpg", + }), + ).toBe("image/png"); + }); + + it("uses the filename only when MIME metadata is absent", () => { + expect(resolveRasterImageMimeType({ mimeType: "", path: "/tmp/screenshot.png" })).toBe( + "image/png", + ); + expect( + resolveRasterImageMimeType({ + mimeType: "application/octet-stream", + path: "/tmp/screenshot.png", + }), + ).toBeNull(); + }); }); diff --git a/packages/app/src/attachments/file-types.ts b/packages/app/src/attachments/file-types.ts index 9fc6f1027..628f833e8 100644 --- a/packages/app/src/attachments/file-types.ts +++ b/packages/app/src/attachments/file-types.ts @@ -41,18 +41,31 @@ export function getRasterImageMimeTypeFromPath(path: string): string | null { return RASTER_IMAGE_MIME_TYPE_BY_EXTENSION[getFileExtension(path)] ?? null; } +export function resolveRasterImageMimeType(input: { + mimeType?: string | null; + path?: string | null; +}): string | null { + const suppliedMimeType = input.mimeType?.trim(); + if (suppliedMimeType) { + const normalizedMimeType = suppliedMimeType.split(";", 1)[0]?.trim().toLowerCase(); + if (normalizedMimeType === "image/jpg") { + return "image/jpeg"; + } + return normalizedMimeType && RASTER_IMAGE_MIME_TYPES.has(normalizedMimeType) + ? normalizedMimeType + : null; + } + return input.path ? getRasterImageMimeTypeFromPath(input.path) : null; +} + export function isRasterImagePath(path: string): boolean { return getRasterImageMimeTypeFromPath(path) !== null; } export function isRasterImageMimeType(mimeType: string | null | undefined): boolean { - const normalized = mimeType?.split(";", 1)[0]?.trim().toLowerCase(); - return Boolean(normalized && RASTER_IMAGE_MIME_TYPES.has(normalized)); + return resolveRasterImageMimeType({ mimeType }) !== null; } export function isRasterImageFile(file: Pick): boolean { - if (isRasterImageMimeType(file.type)) { - return true; - } - return file.type.trim().length === 0 && isRasterImagePath(file.name); + return resolveRasterImageMimeType({ mimeType: file.type, path: file.name }) !== null; } diff --git a/packages/app/src/components/file-drop/use-drop-listeners.ts b/packages/app/src/components/file-drop/use-drop-listeners.ts index 6262109e7..947c95d91 100644 --- a/packages/app/src/components/file-drop/use-drop-listeners.ts +++ b/packages/app/src/components/file-drop/use-drop-listeners.ts @@ -5,9 +5,9 @@ import type { ImageAttachment } from "@/composer/types"; import { getDesktopHost } from "@/desktop/host"; import { persistAttachmentFromBlob, persistAttachmentFromFileUri } from "@/attachments/service"; import { - getRasterImageMimeTypeFromPath, isRasterImageFile, isRasterImagePath, + resolveRasterImageMimeType, } from "@/attachments/file-types"; import { isWeb } from "@/constants/platform"; import type { DroppedItem, DroppedPathItem, FileDropSink } from "./types"; @@ -27,14 +27,21 @@ interface DesktopDragDropEvent { } async function filePathToImageAttachment(path: string): Promise { - const mimeType = getRasterImageMimeTypeFromPath(path) ?? "image/jpeg"; + const mimeType = resolveRasterImageMimeType({ path }); + if (!mimeType) { + throw new Error(`Unsupported image type for '${path}'.`); + } return await persistAttachmentFromFileUri({ uri: path, mimeType }); } async function fileToImageAttachment(file: File): Promise { + const mimeType = resolveRasterImageMimeType({ mimeType: file.type, path: file.name }); + if (!mimeType) { + throw new Error(`Unsupported image type for '${file.name}'.`); + } return await persistAttachmentFromBlob({ blob: file, - mimeType: file.type || "image/jpeg", + mimeType, fileName: file.name, }); } diff --git a/packages/app/src/composer/actions.test.ts b/packages/app/src/composer/actions.test.ts index 8b577fcd7..178fe9eb6 100644 --- a/packages/app/src/composer/actions.test.ts +++ b/packages/app/src/composer/actions.test.ts @@ -321,7 +321,11 @@ describe("pickAndPersistImages", () => { const persister = createFakePersister(); const result = await pickAndPersistImages({ pickImages: async () => [ - { source: { kind: "file_uri", uri: "/tmp/x.jpg" }, mimeType: null, fileName: null }, + { + source: { kind: "file_uri", uri: "/tmp/x.jpg" }, + mimeType: "image/jpeg", + fileName: null, + }, ], persister, }); diff --git a/packages/app/src/composer/actions.ts b/packages/app/src/composer/actions.ts index af3d123d7..44fc31747 100644 --- a/packages/app/src/composer/actions.ts +++ b/packages/app/src/composer/actions.ts @@ -93,7 +93,7 @@ export async function pickAndPersistImages(input: { return await Promise.all( result.map(async (picked) => { const fileName = picked.fileName ?? null; - const mimeType = picked.mimeType || "image/jpeg"; + const mimeType = picked.mimeType; if (picked.source.kind === "blob") { return await input.persister.persistFromBlob({ blob: picked.source.blob, diff --git a/packages/app/src/hooks/image-attachment-picker.native.ts b/packages/app/src/hooks/image-attachment-picker.native.ts index dfb36c575..4c53eb86b 100644 --- a/packages/app/src/hooks/image-attachment-picker.native.ts +++ b/packages/app/src/hooks/image-attachment-picker.native.ts @@ -34,6 +34,8 @@ export async function normalizePickedImageAssets( return normalizePickedImageAssetsWith(assets, exportPickedImageAsPng); } -export async function openImagePathsWithDesktopDialog(_dialog?: unknown): Promise { +export async function pickImagesWithDesktopDialog( + _dialog?: unknown, +): Promise { throw new Error("Desktop dialog API is not available on native."); } diff --git a/packages/app/src/hooks/image-attachment-picker.test.ts b/packages/app/src/hooks/image-attachment-picker.test.ts index 821312db2..a72555496 100644 --- a/packages/app/src/hooks/image-attachment-picker.test.ts +++ b/packages/app/src/hooks/image-attachment-picker.test.ts @@ -1,9 +1,6 @@ import { describe, expect, it } from "vitest"; import type { DesktopDialogBridge, DesktopDialogOpenOptions } from "@/desktop/host"; -import { - normalizePickedImageAssets, - openImagePathsWithDesktopDialog, -} from "./image-attachment-picker"; +import { normalizePickedImageAssets, pickImagesWithDesktopDialog } from "./image-attachment-picker"; function fakeDialogReturning(selection: string | string[] | null): { dialog: DesktopDialogBridge; @@ -42,6 +39,27 @@ describe("image-attachment-picker", () => { expect(result[0]?.mimeType).toBe("image/png"); }); + it("derives the type of a type-less picked File from its name", async () => { + const file = new File(["image"], "picked.png"); + + const result = await normalizePickedImageAssets([ + { + uri: "blob:test", + mimeType: null, + fileName: null, + file, + }, + ]); + + expect(result).toEqual([ + { + source: { kind: "blob", blob: file }, + mimeType: "image/png", + fileName: "picked.png", + }, + ]); + }); + it("keeps filesystem picker results as file uris", async () => { const result = await normalizePickedImageAssets([ { @@ -78,7 +96,7 @@ describe("image-attachment-picker", () => { it("uses the desktop dialog api when available", async () => { const { dialog, recordedOptions } = fakeDialogReturning(["/tmp/one.png", "/tmp/two.jpg"]); - const result = await openImagePathsWithDesktopDialog(dialog); + const result = await pickImagesWithDesktopDialog(dialog); expect(recordedOptions).toHaveLength(1); expect(recordedOptions[0]).toMatchObject({ @@ -86,14 +104,25 @@ describe("image-attachment-picker", () => { directory: false, title: "Attach images", }); - expect(result).toEqual(["/tmp/one.png", "/tmp/two.jpg"]); + expect(result).toEqual([ + { + source: { kind: "file_uri", uri: "/tmp/one.png" }, + mimeType: "image/png", + fileName: "one.png", + }, + { + source: { kind: "file_uri", uri: "/tmp/two.jpg" }, + mimeType: "image/jpeg", + fileName: "two.jpg", + }, + ]); }); it("throws when desktop dialog API is not available", async () => { - await expect(openImagePathsWithDesktopDialog(null)).rejects.toThrow( + await expect(pickImagesWithDesktopDialog(null)).rejects.toThrow( "Desktop dialog API is not available.", ); - await expect(openImagePathsWithDesktopDialog({})).rejects.toThrow( + await expect(pickImagesWithDesktopDialog({})).rejects.toThrow( "Desktop dialog API is not available.", ); }); diff --git a/packages/app/src/hooks/image-attachment-picker.ts b/packages/app/src/hooks/image-attachment-picker.ts index fc13350c7..c01a6ba8a 100644 --- a/packages/app/src/hooks/image-attachment-picker.ts +++ b/packages/app/src/hooks/image-attachment-picker.ts @@ -1,5 +1,6 @@ import type { DesktopDialogBridge } from "@/desktop/host"; -import { RASTER_IMAGE_FILE_EXTENSIONS } from "@/attachments/file-types"; +import { RASTER_IMAGE_FILE_EXTENSIONS, resolveRasterImageMimeType } from "@/attachments/file-types"; +import { getFileNameFromPath } from "@/attachments/utils"; import { i18n } from "@/i18n/i18next"; import { isAbsolutePath } from "@/utils/path"; @@ -7,7 +8,7 @@ export type PickedImageSource = { kind: "file_uri"; uri: string } | { kind: "blo export interface PickedImageAttachmentInput { source: PickedImageSource; - mimeType?: string | null; + mimeType: string; fileName?: string | null; } @@ -30,30 +31,52 @@ async function blobFromUri(uri: string): Promise { return await response.blob(); } +function requirePickedImageMimeType(input: { + mimeType?: string | null; + path?: string | null; +}): string { + const mimeType = resolveRasterImageMimeType(input); + if (!mimeType) { + throw new Error(`Unsupported image type for '${input.path ?? "selected image"}'.`); + } + return mimeType; +} + export async function normalizePickedImageAssets( assets: readonly ExpoImagePickerAssetLike[], ): Promise { return await Promise.all( assets.map(async (asset) => { if (asset.file instanceof Blob) { + const fileName = asset.fileName ?? asset.file.name ?? null; return { source: { kind: "blob", blob: asset.file }, - mimeType: asset.mimeType ?? asset.file.type ?? null, - fileName: asset.fileName ?? asset.file.name ?? null, + mimeType: requirePickedImageMimeType({ + mimeType: asset.mimeType || asset.file.type, + path: fileName ?? asset.uri, + }), + fileName, }; } if (shouldTreatAsFileUri(asset.uri)) { return { source: { kind: "file_uri", uri: asset.uri }, - mimeType: asset.mimeType ?? null, + mimeType: requirePickedImageMimeType({ + mimeType: asset.mimeType, + path: asset.fileName ?? asset.uri, + }), fileName: asset.fileName ?? null, }; } + const blob = await blobFromUri(asset.uri); return { - source: { kind: "blob", blob: await blobFromUri(asset.uri) }, - mimeType: asset.mimeType ?? null, + source: { kind: "blob", blob }, + mimeType: requirePickedImageMimeType({ + mimeType: asset.mimeType || blob.type, + path: asset.fileName ?? asset.uri, + }), fileName: asset.fileName ?? null, }; }), @@ -67,9 +90,9 @@ function normalizeDesktopDialogSelection(selection: string | string[] | null): s return Array.isArray(selection) ? selection : [selection]; } -export async function openImagePathsWithDesktopDialog( +export async function pickImagesWithDesktopDialog( dialog: DesktopDialogBridge | null | undefined, -): Promise { +): Promise { const options = { directory: false, multiple: true, @@ -87,5 +110,15 @@ export async function openImagePathsWithDesktopDialog( throw new Error("Desktop dialog API is not available."); } - return normalizeDesktopDialogSelection(await dialogOpen(options)); + return normalizeDesktopDialogSelection(await dialogOpen(options)).map((path) => { + const mimeType = resolveRasterImageMimeType({ path }); + if (!mimeType) { + throw new Error(`Unsupported image type for '${path}'.`); + } + return { + source: { kind: "file_uri" as const, uri: path }, + mimeType, + fileName: getFileNameFromPath(path), + }; + }); } diff --git a/packages/app/src/hooks/picked-image-normalizer.test.ts b/packages/app/src/hooks/picked-image-normalizer.test.ts index a00144af2..0454cafb3 100644 --- a/packages/app/src/hooks/picked-image-normalizer.test.ts +++ b/packages/app/src/hooks/picked-image-normalizer.test.ts @@ -53,6 +53,30 @@ describe("native image attachment picker", () => { expect(recordedUris).toEqual([]); }); + it("uses explicit native MIME metadata before the URI extension", async () => { + const { exportAsPng, recordedUris } = fakeExportAsPng(); + + const result = await normalizePickedImageAssetsWith( + [ + { + uri: "file:///photos/screenshot.jpg", + mimeType: "image/png", + fileName: "screenshot.png", + }, + ], + exportAsPng, + ); + + expect(result).toEqual([ + { + source: { kind: "file_uri", uri: "file:///photos/screenshot.jpg" }, + mimeType: "image/png", + fileName: "screenshot.png", + }, + ]); + expect(recordedUris).toEqual([]); + }); + it("turns a native picked HEIC-like asset into a PNG attachment input", async () => { const { exportAsPng, recordedUris } = fakeExportAsPng(); diff --git a/packages/app/src/hooks/picked-image-normalizer.ts b/packages/app/src/hooks/picked-image-normalizer.ts index a662d10ff..dfe1b019f 100644 --- a/packages/app/src/hooks/picked-image-normalizer.ts +++ b/packages/app/src/hooks/picked-image-normalizer.ts @@ -2,7 +2,7 @@ export type PickedImageSource = { kind: "file_uri"; uri: string } | { kind: "blo export interface PickedImageAttachmentInput { source: PickedImageSource; - mimeType?: string | null; + mimeType: string; fileName?: string | null; } @@ -62,13 +62,15 @@ function pickedAssetSupportedFormat( asset: ExpoImagePickerAssetLike, ): SupportedPickedImageFormat | null { const uriExtension = extensionFromPath(asset.uri); - if (uriExtension) { - return supportedFormatForExtension(uriExtension); + const uriFormat = supportedFormatForExtension(uriExtension); + if (uriExtension && !uriFormat) { + return null; } return ( + supportedFormatForMimeType(asset.mimeType) ?? supportedFormatForExtension(extensionFromPath(asset.fileName)) ?? - supportedFormatForMimeType(asset.mimeType) + uriFormat ); } diff --git a/packages/app/src/hooks/use-image-attachment-picker.ts b/packages/app/src/hooks/use-image-attachment-picker.ts index 30790bc09..4f8b3fd48 100644 --- a/packages/app/src/hooks/use-image-attachment-picker.ts +++ b/packages/app/src/hooks/use-image-attachment-picker.ts @@ -5,7 +5,7 @@ import { useTranslation } from "react-i18next"; import { getDesktopHost, isElectronRuntime } from "@/desktop/host"; import { normalizePickedImageAssets, - openImagePathsWithDesktopDialog, + pickImagesWithDesktopDialog, type PickedImageAttachmentInput, } from "@/hooks/image-attachment-picker"; import { isWeb } from "@/constants/platform"; @@ -51,15 +51,11 @@ export function useImageAttachmentPicker(): UseImageAttachmentPickerResult { try { if (isWeb && isElectronRuntime()) { - const selectedPaths = await openImagePathsWithDesktopDialog(getDesktopHost()?.dialog); - if (selectedPaths.length === 0) { + const selectedImages = await pickImagesWithDesktopDialog(getDesktopHost()?.dialog); + if (selectedImages.length === 0) { return null; } - return selectedPaths.map((path) => ({ - source: { kind: "file_uri" as const, uri: path }, - mimeType: null, - fileName: null, - })); + return selectedImages; } const hasPermission = await ensurePermission(); diff --git a/packages/app/src/utils/image-attachments-from-files.test.ts b/packages/app/src/utils/image-attachments-from-files.test.ts index d59dc1000..3dac77a83 100644 --- a/packages/app/src/utils/image-attachments-from-files.test.ts +++ b/packages/app/src/utils/image-attachments-from-files.test.ts @@ -92,7 +92,7 @@ describe("collectImageFilesFromClipboardData", () => { ], }); - expect(files).toEqual([imagePng]); + expect(files).toEqual([{ file: imagePng, mimeType: "image/png" }]); }); it("ignores SVG clipboard files", () => { @@ -127,7 +127,10 @@ describe("filesToImageAttachments", () => { type: "", }); - const attachments = await filesToImageAttachments([pngFile, typeLessFile]); + const attachments = await filesToImageAttachments([ + { file: pngFile, mimeType: "image/png" }, + { file: typeLessFile, mimeType: "image/png" }, + ]); expect(attachments).toEqual([ { @@ -141,7 +144,7 @@ describe("filesToImageAttachments", () => { }, { id: "att-2", - mimeType: "image/jpeg", + mimeType: "image/png", storageType: "web-indexeddb", storageKey: "att-2", fileName: "second", @@ -156,7 +159,7 @@ describe("filesToImageAttachments", () => { type: "image/png", }); - const [attachment] = await filesToImageAttachments([large]); + const [attachment] = await filesToImageAttachments([{ file: large, mimeType: "image/png" }]); expect(attachment?.storageType).toBe("web-indexeddb"); expect(attachment?.byteSize).toBe(4 * 1024 * 1024); diff --git a/packages/app/src/utils/image-attachments-from-files.ts b/packages/app/src/utils/image-attachments-from-files.ts index 11483aba5..e44b1d23f 100644 --- a/packages/app/src/utils/image-attachments-from-files.ts +++ b/packages/app/src/utils/image-attachments-from-files.ts @@ -1,6 +1,6 @@ import type { AttachmentMetadata } from "@/attachments/types"; import { persistAttachmentFromBlob } from "@/attachments/service"; -import { isRasterImageMimeType } from "@/attachments/file-types"; +import { resolveRasterImageMimeType } from "@/attachments/file-types"; export interface ClipboardItemLike { kind?: string; @@ -14,40 +14,46 @@ export interface ClipboardDataLike { export type ImageAttachmentFromFile = AttachmentMetadata; +export interface ClipboardImageFile { + file: File; + mimeType: string; +} + export function collectImageFilesFromClipboardData( clipboardData?: ClipboardDataLike | null, -): File[] { +): ClipboardImageFile[] { if (!clipboardData?.items) { return []; } - const files: File[] = []; + const files: ClipboardImageFile[] = []; for (const item of Array.from(clipboardData.items)) { if (item?.kind !== "file") { continue; } - if (!isRasterImageMimeType(item.type)) { + const mimeType = resolveRasterImageMimeType({ mimeType: item.type }); + if (!mimeType) { continue; } const file = item.getAsFile?.(); if (!file) { continue; } - files.push(file); + files.push({ file, mimeType }); } return files; } export async function filesToImageAttachments( - files: readonly File[], + files: readonly ClipboardImageFile[], ): Promise { const attachments = await Promise.all( - files.map(async (file) => { + files.map(async ({ file, mimeType }) => { try { return await persistAttachmentFromBlob({ blob: file, - mimeType: file.type || "image/jpeg", + mimeType, fileName: file.name, }); } catch (error) {