Skip to content

Commit 2baea2e

Browse files
authored
fix: prevent req.file leak between sequential duplicate() calls on upload collections (#15620)
### What? Prevents `req.file` from leaking between sequential `payload.duplicate()` calls when a shared `req` object is used for upload collections. ### Why? When calling `payload.duplicate()` multiple times on the same `req` object (e.g. in batch operations within a transaction), only the first document's file was used for all subsequent duplications. `generateFileData` writes the processed file back to `req.file`, and on the next call `let file = req.file` was truthy, so the block that fetches the correct source file from storage was skipped entirely. ### How? When `isDuplicating` is true, `req.file` is now ignored so the source file is always fetched fresh from storage for each duplicate operation. This is a one-line change in `generateFileData.ts`: ```diff - let file = req.file + let file = isDuplicating ? undefined : req.file ``` An integration test was added that creates two upload documents with different files, duplicates both using a shared `req`, and asserts each duplicate derives its filename from the correct source document. Fixes #15619
1 parent 45bd2f1 commit 2baea2e

2 files changed

Lines changed: 50 additions & 2 deletions

File tree

packages/payload/src/uploads/generateFileData.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ export const generateFileData = async <T>({
8484

8585
const { serverURL, sharp } = req.payload.config
8686

87-
let file = req.file
87+
let file = isDuplicating ? undefined : req.file
8888

8989
const uploadEdits = parseUploadEditsFromReqOrIncomingData({
9090
data,

test/uploads/int.spec.ts

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { AddressInfo } from 'net'
2-
import type { CollectionSlug, Payload } from 'payload'
2+
import type { CollectionSlug, Payload, PayloadRequest } from 'payload'
33

44
import { randomUUID } from 'crypto'
55
import fs from 'fs'
@@ -1376,6 +1376,54 @@ describe('Collections - Uploads', () => {
13761376

13771377
expect(await fileExists(path.join(expectedPath, duplicatedDoc.filename))).toBe(true)
13781378
})
1379+
1380+
it('should not leak req.file between sequential duplicate() calls on a shared req', async () => {
1381+
const filePath1 = path.resolve(dirname, './image.png')
1382+
const file1 = await getFileByPath(filePath1)
1383+
file1.name = 'alpha-leak-test.png'
1384+
1385+
const filePath2 = path.resolve(dirname, './small.png')
1386+
const file2 = await getFileByPath(filePath2)
1387+
file2.name = 'bravo-leak-test.png'
1388+
1389+
const doc1 = await payload.create({
1390+
collection: mediaSlug,
1391+
data: {},
1392+
file: file1,
1393+
})
1394+
1395+
const doc2 = await payload.create({
1396+
collection: mediaSlug,
1397+
data: {},
1398+
file: file2,
1399+
})
1400+
1401+
// Use a shared req object to simulate batch operations within a transaction
1402+
const req = {} as PayloadRequest
1403+
1404+
const dup1 = await payload.duplicate({
1405+
collection: mediaSlug,
1406+
id: doc1.id,
1407+
req,
1408+
})
1409+
1410+
const dup2 = await payload.duplicate({
1411+
collection: mediaSlug,
1412+
id: doc2.id,
1413+
req,
1414+
})
1415+
1416+
// dup1 should derive from alpha-leak-test.png
1417+
expect(dup1.filename).toContain('alpha-leak-test')
1418+
// dup2 should derive from bravo-leak-test.png, NOT alpha-leak-test.png
1419+
expect(dup2.filename).toContain('bravo-leak-test')
1420+
1421+
// Clean up created docs
1422+
await payload.delete({ collection: mediaSlug, id: doc1.id })
1423+
await payload.delete({ collection: mediaSlug, id: doc2.id })
1424+
await payload.delete({ collection: mediaSlug, id: dup1.id })
1425+
await payload.delete({ collection: mediaSlug, id: dup2.id })
1426+
})
13791427
})
13801428

13811429
describe('serverURL handling', () => {

0 commit comments

Comments
 (0)