Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: prefer browser recognized mime type over file extension lookup #991

Merged
merged 10 commits into from
Oct 7, 2024
6 changes: 6 additions & 0 deletions .changeset/tiny-rice-draw.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@uploadthing/shared": patch
"uploadthing": patch
---

fix: prefer browser recognized mime type over file extension lookup when matching file's type to router config
81 changes: 41 additions & 40 deletions packages/shared/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export interface FileProperties {
name: string;
size: number;
type: string;
lastModified?: number;
lastModified?: number | undefined;
}

export type ExtractHashPartsFn = (
Expand Down Expand Up @@ -129,45 +129,46 @@ type ImageProperties = {

type AdditionalProperties<T> = Record<string, unknown> & T;

type RouteConfig<TAdditionalProperties extends Record<string, unknown>> = {
/**
* Human-readable file size limit
* @example "1MB"
* @default https://docs.uploadthing.com/api-reference/server#defaults
*/
maxFileSize: FileSize;
/**
* Maximum number of files allowed to be uploaded of this type
* @example 10
* @default https://docs.uploadthing.com/api-reference/server#defaults
*/
maxFileCount: number;
/**
* Minimum number of files allowed to be uploaded of this type
* @remarks Must be <= maxFileCount
* @example 2
* @default 1
*/
minFileCount: number;
/**
* Specify the [content disposition](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Disposition) of the uploaded file
* @example "attachment"
* @default "inline"
*/
contentDisposition: ContentDisposition;
/**
* Specify the [access control list](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Origin) of the uploaded file
* @remarks This must be enabled for your app. See https://docs.uploadthing.com/regions-and-acl#access-controls.
* @example "private"
* @default "public-read"
*/
acl?: ACL;
/**
* Additional properties to be passed to the client-side `useRouteConfig` hook
* @remarks These properties are not validated on the server on upload
*/
additionalProperties?: AdditionalProperties<TAdditionalProperties>;
};
export type RouteConfig<TAdditionalProperties extends Record<string, unknown>> =
{
/**
* Human-readable file size limit
* @example "1MB"
* @default https://docs.uploadthing.com/api-reference/server#defaults
*/
maxFileSize: FileSize;
/**
* Maximum number of files allowed to be uploaded of this type
* @example 10
* @default https://docs.uploadthing.com/api-reference/server#defaults
*/
maxFileCount: number;
/**
* Minimum number of files allowed to be uploaded of this type
* @remarks Must be <= maxFileCount
* @example 2
* @default 1
*/
minFileCount: number;
/**
* Specify the [content disposition](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Disposition) of the uploaded file
* @example "attachment"
* @default "inline"
*/
contentDisposition: ContentDisposition;
/**
* Specify the [access control list](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Origin) of the uploaded file
* @remarks This must be enabled for your app. See https://docs.uploadthing.com/regions-and-acl#access-controls.
* @example "private"
* @default "public-read"
*/
acl?: ACL;
/**
* Additional properties to be passed to the client-side `useRouteConfig` hook
* @remarks These properties are not validated on the server on upload
*/
additionalProperties?: AdditionalProperties<TAdditionalProperties>;
};

/**
* Shared config options for an entire route not bound to any specific file type
Expand Down
46 changes: 25 additions & 21 deletions packages/shared/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,12 @@ import {
import type {
ContentDisposition,
ExpandedRouteConfig,
FileProperties,
FileRouterInputConfig,
FileRouterInputKey,
FileSize,
ResponseEsque,
RouteConfig,
Time,
TimeShort,
} from "./types";
Expand All @@ -38,6 +40,17 @@ export function getDefaultSizeForType(fileType: FileRouterInputKey): FileSize {
return "4MB";
}

export function getDefaultRouteConfigValues(
type: FileRouterInputKey,
): RouteConfig<Record<string, never>> {
return {
maxFileSize: getDefaultSizeForType(type),
maxFileCount: 1,
minFileCount: 1,
contentDisposition: "inline" as const,
};
}

/**
* This function takes in the user's input and "upscales" it to a full config
* Additionally, it replaces numbers with "safe" equivalents
Expand All @@ -55,13 +68,7 @@ export const fillInputRouteConfig = (
if (isRouteArray(routeConfig)) {
return Micro.succeed(
routeConfig.reduce<ExpandedRouteConfig>((acc, fileType) => {
acc[fileType] = {
// Apply defaults
maxFileSize: getDefaultSizeForType(fileType),
maxFileCount: 1,
minFileCount: 1,
contentDisposition: "inline" as const,
};
acc[fileType] = getDefaultRouteConfigValues(fileType);
return acc;
}, {}),
);
Expand All @@ -72,15 +79,7 @@ export const fillInputRouteConfig = (
for (const key of objectKeys(routeConfig)) {
const value = routeConfig[key];
if (!value) return Micro.fail(new InvalidRouteConfigError(key));

const defaultValues = {
maxFileSize: getDefaultSizeForType(key),
maxFileCount: 1,
minFileCount: 1,
contentDisposition: "inline" as const,
};

newConfig[key] = { ...defaultValues, ...value };
newConfig[key] = { ...getDefaultRouteConfigValues(key), ...value };
}

// we know that the config is valid, so we can stringify it and parse it back
Expand All @@ -92,17 +91,22 @@ export const fillInputRouteConfig = (
);
};

export const getTypeFromFileName = (
fileName: string,
/**
* Match the file's type for a given allow list e.g. `image/png => image`
* Prefers the file's type, then falls back to a extension-based lookup
*/
export const matchFileType = (
file: FileProperties,
allowedTypes: FileRouterInputKey[],
): Micro.Micro<
FileRouterInputKey,
UnknownFileTypeError | InvalidFileTypeError
> => {
const mimeType = lookup(fileName);
// Type might be "" if the browser doesn't recognize the mime type
const mimeType = file.type || lookup(file.name);
if (!mimeType) {
if (allowedTypes.includes("blob")) return Micro.succeed("blob");
return Micro.fail(new UnknownFileTypeError(fileName));
return Micro.fail(new UnknownFileTypeError(file.name));
}

// If the user has specified a specific mime type, use that
Expand All @@ -124,7 +128,7 @@ export const getTypeFromFileName = (
if (allowedTypes.includes("blob")) {
return Micro.succeed("blob");
} else {
return Micro.fail(new InvalidFileTypeError(type, fileName));
return Micro.fail(new InvalidFileTypeError(type, file.name));
}
}

Expand Down
6 changes: 3 additions & 3 deletions packages/uploadthing/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
asArray,
FetchContext,
fileSizeToBytes,
getTypeFromFileName,
matchFileType,
objectKeys,
resolveMaybeUrlArg,
UploadAbortedError,
Expand Down Expand Up @@ -51,7 +51,7 @@ export const isValidFileType = (
routeConfig: ExpandedRouteConfig,
): boolean =>
Micro.runSync(
getTypeFromFileName(file.name, objectKeys(routeConfig)).pipe(
matchFileType(file, objectKeys(routeConfig)).pipe(
Micro.map((type) => file.type.includes(type)),
Micro.orElseSucceed(() => false),
),
Expand All @@ -66,7 +66,7 @@ export const isValidFileSize = (
routeConfig: ExpandedRouteConfig,
): boolean =>
Micro.runSync(
getTypeFromFileName(file.name, objectKeys(routeConfig)).pipe(
matchFileType(file, objectKeys(routeConfig)).pipe(
Micro.flatMap((type) => fileSizeToBytes(routeConfig[type]!.maxFileSize)),
Micro.map((maxFileSize) => file.size <= maxFileSize),
Micro.orElseSucceed(() => false),
Expand Down
25 changes: 11 additions & 14 deletions packages/uploadthing/src/internal/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
generateKey,
generateSignedURL,
getStatusCodeFromError,
getTypeFromFileName,
matchFileType,
objectKeys,
UploadThingError,
verifySignature,
Expand Down Expand Up @@ -529,19 +529,16 @@ const handleUploadAction = (opts: {
const fileUploadRequests = yield* Effect.forEach(
filesWithCustomIds,
(file) =>
Effect.map(
getTypeFromFileName(file.name, objectKeys(parsedConfig)),
(type) => ({
name: file.name,
size: file.size,
type: file.type,
lastModified: file.lastModified,
customId: file.customId,
contentDisposition:
parsedConfig[type]?.contentDisposition ?? "inline",
acl: parsedConfig[type]?.acl,
}),
),
Effect.map(matchFileType(file, objectKeys(parsedConfig)), (type) => ({
name: file.name,
size: file.size,
type: file.type || type,
lastModified: file.lastModified,
customId: file.customId,
contentDisposition:
parsedConfig[type]?.contentDisposition ?? "inline",
acl: parsedConfig[type]?.acl,
})),
).pipe(
Effect.catchTags({
/** Shouldn't happen since config is validated above so just dying is fine I think */
Expand Down
7 changes: 2 additions & 5 deletions packages/uploadthing/src/internal/route-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ import {
bytesToFileSize,
fileSizeToBytes,
fillInputRouteConfig,
getTypeFromFileName,
InvalidRouteConfigError,
matchFileType,
objectKeys,
UploadThingError,
} from "@uploadthing/shared";
Expand Down Expand Up @@ -71,10 +71,7 @@ export const assertFilesMeetConfig = (
const counts: Record<string, number> = {};

for (const file of files) {
const type = yield* getTypeFromFileName(
file.name,
objectKeys(routeConfig),
);
const type = yield* matchFileType(file, objectKeys(routeConfig));
counts[type] = (counts[type] ?? 0) + 1;

const sizeLimit = routeConfig[type]?.maxFileSize;
Expand Down
81 changes: 81 additions & 0 deletions packages/uploadthing/test/request-handler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,87 @@ describe("file route config", () => {
message: "Invalid config: FileCountMismatch",
});
});

it("uses file type to match mime type with router config", async ({ db }) => {
const res = await handler(
new Request(createApiUrl("imageUploader", "upload"), {
method: "POST",
headers: baseHeaders,
body: JSON.stringify({
files: [{ name: "foo", size: 48, type: "image/png" }],
}),
}),
);

expect(middlewareMock).toHaveBeenCalledWith(
expect.objectContaining({
files: [{ name: "foo", size: 48, type: "image/png" }],
}),
);

expect(res.status).toBe(200);
await expect(res.json()).resolves.toMatchObject([
{
name: "foo",
url: expect.stringContaining("x-ut-file-type=image"),
},
]);
});

it("prefers file.type over file.name extension", async ({ db }) => {
const res = await handler(
new Request(createApiUrl("imageUploader", "upload"), {
method: "POST",
headers: baseHeaders,
body: JSON.stringify({
files: [{ name: "foo.txt", size: 48, type: "image/png" }],
}),
}),
);

expect(middlewareMock).toHaveBeenCalledWith(
expect.objectContaining({
files: [{ name: "foo.txt", size: 48, type: "image/png" }],
}),
);

expect(res.status).toBe(200);
await expect(res.json()).resolves.toMatchObject([
{
name: "foo.txt",
url: expect.stringContaining("x-ut-file-type=image"),
},
]);
});

it("falls back to filename lookup type when there's no recognized mime type", async ({
db,
}) => {
const res = await handler(
new Request(createApiUrl("imageUploader", "upload"), {
method: "POST",
headers: baseHeaders,
body: JSON.stringify({
files: [{ name: "foo.png", size: 48, type: "" }], // mimic browser unrecognized type
}),
}),
);
expect(res.status).toBe(200);

expect(middlewareMock).toHaveBeenCalledWith(
expect.objectContaining({
files: [{ name: "foo.png", size: 48, type: "" }],
}),
);

expect(res.status).toBe(200);
await expect(res.json()).resolves.toMatchObject([
{
name: "foo.png",
url: expect.stringContaining("x-ut-file-type=image"),
},
]);
});
});

describe(".input()", () => {
Expand Down
Loading