Skip to content

Commit 4a6189e

Browse files
r1tsuuDanRibbens
andauthored
fix(storage-s3): respect upload limits with client uploads (#15176)
Fixes #12671 Relies on #15174 (merge this one first) In addition to checking the passed `filesize` from `S3ClientUploadHandler`, this PR also uses S3 signed headers to ensure that `ContentLength` does not exceed `upload.limits.fileSize`. --------- Co-authored-by: Dan Ribbens <dan.ribbens@gmail.com>
1 parent 6f4b272 commit 4a6189e

8 files changed

Lines changed: 207 additions & 11 deletions

File tree

packages/storage-s3/src/client/S3ClientUploadHandler.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
'use client'
22
import { createClientUploadHandler } from '@payloadcms/plugin-cloud-storage/client'
3+
import { toast } from '@payloadcms/ui'
34
import { formatAdminURL } from 'payload/shared'
45

56
export const S3ClientUploadHandler = createClientUploadHandler({
@@ -9,16 +10,26 @@ export const S3ClientUploadHandler = createClientUploadHandler({
910
path: serverHandlerPath,
1011
serverURL,
1112
})
13+
1214
const response = await fetch(endpointRoute, {
1315
body: JSON.stringify({
1416
collectionSlug,
1517
filename: file.name,
18+
filesize: file.size,
1619
mimeType: file.type,
1720
}),
1821
credentials: 'include',
1922
method: 'POST',
2023
})
2124

25+
if (!response.ok) {
26+
const { errors } = (await response.json()) as {
27+
errors: { message: string }[]
28+
}
29+
30+
throw new Error(errors.reduce((acc, err) => `${acc ? `${acc}, ` : ''}${err.message}`, ''))
31+
}
32+
2233
const { url } = (await response.json()) as {
2334
url: string
2435
}

packages/storage-s3/src/generateSignedURL.ts

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,14 @@ import type { PayloadHandler } from 'payload'
44
import * as AWS from '@aws-sdk/client-s3'
55
import { getSignedUrl } from '@aws-sdk/s3-request-presigner'
66
import path from 'path'
7-
import { APIError, Forbidden } from 'payload'
7+
import { APIError, Forbidden, ValidationError } from 'payload'
88

99
import type { S3StorageOptions } from './index.js'
1010

11+
const bytesToMB = (bytes: number) => {
12+
return bytes / 1024 / 1024
13+
}
14+
1115
interface Args {
1216
access?: ClientUploadsAccess
1317
acl?: 'private' | 'public-read'
@@ -30,9 +34,16 @@ export const getGenerateSignedURLHandler = ({
3034
throw new APIError('Content-Type expected to be application/json', 400)
3135
}
3236

33-
const { collectionSlug, filename, mimeType } = (await req.json()) as {
37+
let filesizeLimit = req.payload.config.upload.limits?.fileSize
38+
39+
if (filesizeLimit === Infinity) {
40+
filesizeLimit = undefined
41+
}
42+
43+
const { collectionSlug, filename, filesize, mimeType } = (await req.json()) as {
3444
collectionSlug: string
3545
filename: string
46+
filesize: number
3647
mimeType: string
3748
}
3849

@@ -49,12 +60,33 @@ export const getGenerateSignedURLHandler = ({
4960

5061
const fileKey = path.posix.join(prefix, filename)
5162

63+
const signableHeaders = new Set()
64+
65+
if (filesizeLimit) {
66+
if (filesize > filesizeLimit) {
67+
throw new APIError(
68+
`Exceeded file size limit. Limit: ${bytesToMB(filesizeLimit).toFixed(2)}MB, got: ${bytesToMB(filesize).toFixed(2)}MB`,
69+
400,
70+
)
71+
}
72+
73+
// Still force S3 to validate
74+
signableHeaders.add('content-length')
75+
}
76+
5277
const url = await getSignedUrl(
5378
// @ts-expect-error mismatch versions or something
5479
getStorageClient(),
55-
new AWS.PutObjectCommand({ ACL: acl, Bucket: bucket, ContentType: mimeType, Key: fileKey }),
80+
new AWS.PutObjectCommand({
81+
ACL: acl,
82+
Bucket: bucket,
83+
ContentLength: filesizeLimit ? Math.min(filesize, filesizeLimit) : undefined,
84+
ContentType: mimeType,
85+
Key: fileKey,
86+
}),
5687
{
5788
expiresIn: 600,
89+
signableHeaders,
5890
},
5991
)
6092

packages/ui/src/forms/Form/index.tsx

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -386,12 +386,12 @@ export const Form: React.FC<FormProps> = (props) => {
386386
return
387387
}
388388

389-
const formData = await contextRef.current.createFormData(overrides, {
390-
data,
391-
mergeOverrideData: Boolean(typeof overridesFromArgs !== 'function'),
392-
})
393-
394389
try {
390+
const formData = await contextRef.current.createFormData(overrides, {
391+
data,
392+
mergeOverrideData: Boolean(typeof overridesFromArgs !== 'function'),
393+
})
394+
395395
let res
396396

397397
if (typeof actionArg === 'string') {
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import type { CollectionConfig } from 'payload'
2+
3+
export const MediaWithClientUploads: CollectionConfig = {
4+
slug: 'media-with-client-uploads',
5+
fields: [
6+
{
7+
name: 'alt',
8+
type: 'text',
9+
label: 'Alt Text',
10+
},
11+
],
12+
upload: {
13+
disableLocalStorage: true,
14+
},
15+
}

test/storage-s3/config.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { buildConfigWithDefaults } from '../buildConfigWithDefaults.js'
77
import { devUser } from '../credentials.js'
88
import { Media } from './collections/Media.js'
99
import { MediaWithAlwaysInsertFields } from './collections/MediaWithAlwaysInsertFields.js'
10+
import { MediaWithClientUploads } from './collections/MediaWithClientUploads.js'
1011
import { MediaWithDirectAccess } from './collections/MediaWithDirectAccess.js'
1112
import { MediaWithDynamicPrefix } from './collections/MediaWithDynamicPrefix.js'
1213
import { MediaWithPrefix } from './collections/MediaWithPrefix.js'
@@ -15,6 +16,7 @@ import { Users } from './collections/Users.js'
1516
import {
1617
mediaSlug,
1718
mediaWithAlwaysInsertFieldsSlug,
19+
mediaWithClientUploadsSlug,
1820
mediaWithDirectAccessSlug,
1921
mediaWithDynamicPrefixSlug,
2022
mediaWithPrefixSlug,
@@ -24,8 +26,6 @@ import {
2426
const filename = fileURLToPath(import.meta.url)
2527
const dirname = path.dirname(filename)
2628

27-
let uploadOptions
28-
2929
// Load config to work with emulated services
3030
dotenv.config({
3131
path: path.resolve(dirname, '../plugin-cloud-storage/.env.emulated'),
@@ -40,6 +40,7 @@ export default buildConfigWithDefaults({
4040
collections: [
4141
Media,
4242
MediaWithAlwaysInsertFields,
43+
MediaWithClientUploads,
4344
MediaWithDirectAccess,
4445
MediaWithDynamicPrefix,
4546
MediaWithPrefix,
@@ -57,8 +58,10 @@ export default buildConfigWithDefaults({
5758
},
5859
plugins: [
5960
s3Storage({
61+
clientUploads: true,
6062
collections: {
6163
[mediaSlug]: true,
64+
[mediaWithClientUploadsSlug]: true,
6265
[mediaWithDirectAccessSlug]: {
6366
disablePayloadAccessControl: true,
6467
},
@@ -106,7 +109,11 @@ export default buildConfigWithDefaults({
106109
enabled: false,
107110
}),
108111
],
109-
upload: uploadOptions,
112+
upload: {
113+
limits: {
114+
fileSize: 1_000_000, // 1MB
115+
},
116+
},
110117
typescript: {
111118
outputFile: path.resolve(dirname, 'payload-types.ts'),
112119
},

test/storage-s3/int.spec.ts

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { initPayloadInt } from '../helpers/initPayloadInt.js'
1111
import {
1212
mediaSlug,
1313
mediaWithAlwaysInsertFieldsSlug,
14+
mediaWithClientUploadsSlug,
1415
mediaWithDirectAccessSlug,
1516
mediaWithDynamicPrefixSlug,
1617
mediaWithPrefixSlug,
@@ -187,6 +188,92 @@ describe('@payloadcms/storage-s3', () => {
187188
it.todo('can upload')
188189
})
189190

191+
describe('client uploads with size limits', () => {
192+
const signedURLEndpoint = '/storage-s3-generate-signed-url'
193+
194+
it('should generate signed URL for file within size limit', async () => {
195+
const filename = 'small-file.png'
196+
const filesize = 500_000 // 500KB (within 1MB limit)
197+
const mimeType = 'image/png'
198+
199+
const response = await restClient.POST(signedURLEndpoint, {
200+
body: JSON.stringify({
201+
collectionSlug: mediaWithClientUploadsSlug,
202+
filename,
203+
filesize,
204+
mimeType,
205+
}),
206+
})
207+
208+
expect(response.status).toBe(200)
209+
const { url } = (await response.json()) as any
210+
expect(url).toBeDefined()
211+
expect(url).toContain(process.env.S3_BUCKET)
212+
expect(url).toContain(filename)
213+
})
214+
215+
it('should reject file exceeding size limit', async () => {
216+
const filename = 'large-file.png'
217+
const filesize = 2_000_000 // 2MB (exceeds 1MB limit)
218+
const mimeType = 'image/png'
219+
220+
const response = await restClient.POST(signedURLEndpoint, {
221+
body: JSON.stringify({
222+
collectionSlug: mediaWithClientUploadsSlug,
223+
filename,
224+
filesize,
225+
mimeType,
226+
}),
227+
})
228+
229+
expect(response.status).toBe(400)
230+
const { errors } = (await response.json()) as any
231+
expect(errors).toBeDefined()
232+
expect(errors[0].message).toContain('Exceeded file size limit')
233+
expect(errors[0].message).toMatch(/Limit: 0\.9\dMB/) // 1,000,000 bytes ≈ 0.95MB
234+
expect(errors[0].message).toMatch(/got: 1\.9\dMB/) // 2,000,000 bytes ≈ 1.91MB
235+
})
236+
237+
it('should reject file exactly at limit boundary', async () => {
238+
const filename = 'boundary-file.png'
239+
const filesize = 1_000_001 // Just over 1MB limit
240+
const mimeType = 'image/png'
241+
242+
const response = await restClient.POST(signedURLEndpoint, {
243+
body: JSON.stringify({
244+
collectionSlug: mediaWithClientUploadsSlug,
245+
filename,
246+
filesize,
247+
mimeType,
248+
}),
249+
})
250+
251+
expect(response.status).toBe(400)
252+
const { errors } = (await response.json()) as any
253+
expect(errors).toBeDefined()
254+
expect(errors[0].message).toContain('Exceeded file size limit')
255+
})
256+
257+
it('should accept file exactly at limit', async () => {
258+
const filename = 'exact-limit.png'
259+
const filesize = 1_000_000 // Exactly 1MB
260+
const mimeType = 'image/png'
261+
262+
const response = await restClient.POST(signedURLEndpoint, {
263+
body: JSON.stringify({
264+
collectionSlug: mediaWithClientUploadsSlug,
265+
filename,
266+
filesize,
267+
mimeType,
268+
}),
269+
})
270+
271+
expect(response.status).toBe(200)
272+
const { url } = (await response.json()) as any
273+
expect(url).toBeDefined()
274+
})
275+
})
276+
190277
describe('prefix collision detection', () => {
191278
beforeEach(async () => {
192279
// Clear S3 bucket before each test

test/storage-s3/payload-types.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ export interface Config {
6969
collections: {
7070
media: Media;
7171
'media-with-always-insert-fields': MediaWithAlwaysInsertField;
72+
'media-with-client-uploads': MediaWithClientUpload;
7273
'media-with-direct-access': MediaWithDirectAccess;
7374
'media-with-dynamic-prefix': MediaWithDynamicPrefix;
7475
'media-with-prefix': MediaWithPrefix;
@@ -83,6 +84,7 @@ export interface Config {
8384
collectionsSelect: {
8485
media: MediaSelect<false> | MediaSelect<true>;
8586
'media-with-always-insert-fields': MediaWithAlwaysInsertFieldsSelect<false> | MediaWithAlwaysInsertFieldsSelect<true>;
87+
'media-with-client-uploads': MediaWithClientUploadsSelect<false> | MediaWithClientUploadsSelect<true>;
8688
'media-with-direct-access': MediaWithDirectAccessSelect<false> | MediaWithDirectAccessSelect<true>;
8789
'media-with-dynamic-prefix': MediaWithDynamicPrefixSelect<false> | MediaWithDynamicPrefixSelect<true>;
8890
'media-with-prefix': MediaWithPrefixSelect<false> | MediaWithPrefixSelect<true>;
@@ -183,6 +185,25 @@ export interface MediaWithAlwaysInsertField {
183185
focalX?: number | null;
184186
focalY?: number | null;
185187
}
188+
/**
189+
* This interface was referenced by `Config`'s JSON-Schema
190+
* via the `definition` "media-with-client-uploads".
191+
*/
192+
export interface MediaWithClientUpload {
193+
id: string;
194+
alt?: string | null;
195+
updatedAt: string;
196+
createdAt: string;
197+
url?: string | null;
198+
thumbnailURL?: string | null;
199+
filename?: string | null;
200+
mimeType?: string | null;
201+
filesize?: number | null;
202+
width?: number | null;
203+
height?: number | null;
204+
focalX?: number | null;
205+
focalY?: number | null;
206+
}
186207
/**
187208
* This interface was referenced by `Config`'s JSON-Schema
188209
* via the `definition` "media-with-direct-access".
@@ -315,6 +336,10 @@ export interface PayloadLockedDocument {
315336
relationTo: 'media-with-always-insert-fields';
316337
value: string | MediaWithAlwaysInsertField;
317338
} | null)
339+
| ({
340+
relationTo: 'media-with-client-uploads';
341+
value: string | MediaWithClientUpload;
342+
} | null)
318343
| ({
319344
relationTo: 'media-with-direct-access';
320345
value: string | MediaWithDirectAccess;
@@ -438,6 +463,24 @@ export interface MediaWithAlwaysInsertFieldsSelect<T extends boolean = true> {
438463
focalX?: T;
439464
focalY?: T;
440465
}
466+
/**
467+
* This interface was referenced by `Config`'s JSON-Schema
468+
* via the `definition` "media-with-client-uploads_select".
469+
*/
470+
export interface MediaWithClientUploadsSelect<T extends boolean = true> {
471+
alt?: T;
472+
updatedAt?: T;
473+
createdAt?: T;
474+
url?: T;
475+
thumbnailURL?: T;
476+
filename?: T;
477+
mimeType?: T;
478+
filesize?: T;
479+
width?: T;
480+
height?: T;
481+
focalX?: T;
482+
focalY?: T;
483+
}
441484
/**
442485
* This interface was referenced by `Config`'s JSON-Schema
443486
* via the `definition` "media-with-direct-access_select".

test/storage-s3/shared.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,4 @@ export const prefix = 'test-prefix'
66

77
export const mediaWithSignedDownloadsSlug = 'media-with-signed-downloads'
88
export const mediaWithDirectAccessSlug = 'media-with-direct-access'
9+
export const mediaWithClientUploadsSlug = 'media-with-client-uploads'

0 commit comments

Comments
 (0)