Skip to content

Commit 6139508

Browse files
JarrodMFleschJarrodMFlesch
authored
fix(storage-*): simplify key handling for signed urls and composite prefixes (#16291)
## Summary This PR primarily fixes inconsistent storage key and prefix behavior across adapters and client upload flows. The main goal is to make signed URL generation, object lookup/delete, and client upload path handling behave the same way for S3, GCS, Azure, R2, and Vercel Blob, especially when composite prefixes are enabled. ## Fixes This change fixes prefix/key mismatch issues by standardizing how adapters derive object keys, and resolves edge cases where client-uploaded files could diverge from server-side prefix/filename semantics. It also fixes GCS URL generation behavior by preserving canonical encoded public URLs instead of decoding them. ## Additional cleanup As part of the fix work, getFileKey was expanded to return normalized key components so adapters can consume one shared source of truth rather than rebuilding path logic in multiple places. This reduces duplication and makes future storage fixes safer. ## Validation Composite-prefix client upload coverage was added for Vercel Blob to protect the fixed behavior and prevent regressions in key/prefix handling.
1 parent c852d85 commit 6139508

41 files changed

Lines changed: 498 additions & 144 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎packages/plugin-cloud-storage/src/utilities/getFileKey.spec.ts‎

Lines changed: 76 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,12 @@ describe('getFileKey', () => {
1010
filename: 'test.png',
1111
useCompositePrefixes: false,
1212
})
13-
expect(result).toBe('document/test.png')
13+
expect(result).toEqual({
14+
fileKey: 'document/test.png',
15+
sanitizedCollectionPrefix: 'collection',
16+
sanitizedDocPrefix: 'document',
17+
sanitizedFilename: 'test.png',
18+
})
1419
})
1520

1621
it('should fallback to collectionPrefix when docPrefix is empty', () => {
@@ -20,7 +25,12 @@ describe('getFileKey', () => {
2025
filename: 'test.png',
2126
useCompositePrefixes: false,
2227
})
23-
expect(result).toBe('collection/test.png')
28+
expect(result).toEqual({
29+
fileKey: 'collection/test.png',
30+
sanitizedCollectionPrefix: 'collection',
31+
sanitizedDocPrefix: '',
32+
sanitizedFilename: 'test.png',
33+
})
2434
})
2535

2636
it('should fallback to collectionPrefix when docPrefix is undefined', () => {
@@ -29,15 +39,25 @@ describe('getFileKey', () => {
2939
filename: 'test.png',
3040
useCompositePrefixes: false,
3141
})
32-
expect(result).toBe('collection/test.png')
42+
expect(result).toEqual({
43+
fileKey: 'collection/test.png',
44+
sanitizedCollectionPrefix: 'collection',
45+
sanitizedDocPrefix: '',
46+
sanitizedFilename: 'test.png',
47+
})
3348
})
3449

3550
it('should return only filename when both prefixes are empty', () => {
3651
const result = getFileKey({
3752
filename: 'test.png',
3853
useCompositePrefixes: false,
3954
})
40-
expect(result).toBe('test.png')
55+
expect(result).toEqual({
56+
fileKey: 'test.png',
57+
sanitizedCollectionPrefix: '',
58+
sanitizedDocPrefix: '',
59+
sanitizedFilename: 'test.png',
60+
})
4161
})
4262
})
4363

@@ -49,7 +69,12 @@ describe('getFileKey', () => {
4969
filename: 'test.png',
5070
useCompositePrefixes: true,
5171
})
52-
expect(result).toBe('collection/document/test.png')
72+
expect(result).toEqual({
73+
fileKey: 'collection/document/test.png',
74+
sanitizedCollectionPrefix: 'collection',
75+
sanitizedDocPrefix: 'document',
76+
sanitizedFilename: 'test.png',
77+
})
5378
})
5479

5580
it('should work with only collectionPrefix', () => {
@@ -58,7 +83,12 @@ describe('getFileKey', () => {
5883
filename: 'test.png',
5984
useCompositePrefixes: true,
6085
})
61-
expect(result).toBe('collection/test.png')
86+
expect(result).toEqual({
87+
fileKey: 'collection/test.png',
88+
sanitizedCollectionPrefix: 'collection',
89+
sanitizedDocPrefix: '',
90+
sanitizedFilename: 'test.png',
91+
})
6292
})
6393

6494
it('should work with only docPrefix', () => {
@@ -67,15 +97,25 @@ describe('getFileKey', () => {
6797
filename: 'test.png',
6898
useCompositePrefixes: true,
6999
})
70-
expect(result).toBe('document/test.png')
100+
expect(result).toEqual({
101+
fileKey: 'document/test.png',
102+
sanitizedCollectionPrefix: '',
103+
sanitizedDocPrefix: 'document',
104+
sanitizedFilename: 'test.png',
105+
})
71106
})
72107

73108
it('should return only filename when both prefixes are empty', () => {
74109
const result = getFileKey({
75110
filename: 'test.png',
76111
useCompositePrefixes: true,
77112
})
78-
expect(result).toBe('test.png')
113+
expect(result).toEqual({
114+
fileKey: 'test.png',
115+
sanitizedCollectionPrefix: '',
116+
sanitizedDocPrefix: '',
117+
sanitizedFilename: 'test.png',
118+
})
79119
})
80120
})
81121

@@ -86,8 +126,13 @@ describe('getFileKey', () => {
86126
filename: 'test.png',
87127
useCompositePrefixes: false,
88128
})
89-
expect(result).toBe('etc/test.png')
90-
expect(result).not.toContain('..')
129+
expect(result).toEqual({
130+
fileKey: 'etc/test.png',
131+
sanitizedCollectionPrefix: 'etc',
132+
sanitizedDocPrefix: '',
133+
sanitizedFilename: 'test.png',
134+
})
135+
expect(result.fileKey).not.toContain('..')
91136
})
92137

93138
it('should remove path traversal segments from docPrefix', () => {
@@ -96,8 +141,13 @@ describe('getFileKey', () => {
96141
filename: 'test.png',
97142
useCompositePrefixes: false,
98143
})
99-
expect(result).toBe('a/outside/test.png')
100-
expect(result).not.toContain('..')
144+
expect(result).toEqual({
145+
fileKey: 'a/outside/test.png',
146+
sanitizedCollectionPrefix: '',
147+
sanitizedDocPrefix: 'a/outside',
148+
sanitizedFilename: 'test.png',
149+
})
150+
expect(result.fileKey).not.toContain('..')
101151
})
102152

103153
it('should remove control characters from prefixes', () => {
@@ -106,8 +156,13 @@ describe('getFileKey', () => {
106156
filename: 'test.png',
107157
useCompositePrefixes: false,
108158
})
109-
expect(result).toBe('testprefix/test.png')
110-
expect(result).not.toMatch(/[\x00-\x1f]/)
159+
expect(result).toEqual({
160+
fileKey: 'testprefix/test.png',
161+
sanitizedCollectionPrefix: 'testprefix',
162+
sanitizedDocPrefix: '',
163+
sanitizedFilename: 'test.png',
164+
})
165+
expect(result.fileKey).not.toMatch(/[\x00-\x1f]/)
111166
})
112167

113168
it('should sanitize both prefixes in composite mode', () => {
@@ -117,8 +172,13 @@ describe('getFileKey', () => {
117172
filename: 'test.png',
118173
useCompositePrefixes: true,
119174
})
120-
expect(result).toBe('collection/doc/test.png')
121-
expect(result).not.toContain('..')
175+
expect(result).toEqual({
176+
fileKey: 'collection/doc/test.png',
177+
sanitizedCollectionPrefix: 'collection',
178+
sanitizedDocPrefix: 'doc',
179+
sanitizedFilename: 'test.png',
180+
})
181+
expect(result.fileKey).not.toContain('..')
122182
})
123183
})
124184
})

‎packages/plugin-cloud-storage/src/utilities/getFileKey.ts‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import path from 'path'
2+
import { sanitizeFilename } from 'payload/shared'
23

34
import { sanitizePrefix } from './sanitizePrefix.js'
45

@@ -9,6 +10,13 @@ type GetFileKeyArgs = {
910
useCompositePrefixes?: boolean
1011
}
1112

13+
type GetFileKeyResult = {
14+
fileKey: string
15+
sanitizedCollectionPrefix: string
16+
sanitizedDocPrefix: string
17+
sanitizedFilename: string
18+
}
19+
1220
/**
1321
* Computes the file key (path) for storage.
1422
*
@@ -21,13 +29,19 @@ export function getFileKey({
2129
docPrefix,
2230
filename,
2331
useCompositePrefixes = false,
24-
}: GetFileKeyArgs): string {
32+
}: GetFileKeyArgs): GetFileKeyResult {
2533
const safeCollectionPrefix = sanitizePrefix(collectionPrefix || '')
2634
const safeDocPrefix = sanitizePrefix(docPrefix || '')
35+
const safeFilename = sanitizeFilename(filename)
2736

28-
if (useCompositePrefixes) {
29-
return path.posix.join(safeCollectionPrefix, safeDocPrefix, filename)
30-
}
37+
const fileKey = useCompositePrefixes
38+
? path.posix.join(safeCollectionPrefix, safeDocPrefix, safeFilename)
39+
: path.posix.join(safeDocPrefix || safeCollectionPrefix, safeFilename)
3140

32-
return path.posix.join(safeDocPrefix || safeCollectionPrefix, filename)
41+
return {
42+
fileKey,
43+
sanitizedCollectionPrefix: safeCollectionPrefix,
44+
sanitizedDocPrefix: safeDocPrefix,
45+
sanitizedFilename: safeFilename,
46+
}
3347
}

‎packages/plugin-cloud-storage/src/utilities/initClientUploads.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,23 +74,23 @@ export const initClientUploads = <ExtraProps extends Record<string, unknown>, T>
7474
for (const collectionSlug in collections) {
7575
const collection = collections[collectionSlug]
7676

77-
let prefix: string | undefined
77+
let collectionPrefix: string | undefined
7878

7979
if (
8080
collection &&
8181
typeof collection === 'object' &&
8282
'prefix' in collection &&
8383
typeof collection.prefix === 'string'
8484
) {
85-
prefix = collection.prefix
85+
collectionPrefix = collection.prefix
8686
}
8787

8888
config.admin.components.providers.push({
8989
clientProps: {
9090
collectionSlug,
9191
enabled,
9292
extra: extraClientHandlerProps ? extraClientHandlerProps(collection!) : undefined,
93-
prefix,
93+
prefix: collectionPrefix,
9494
serverHandlerPath,
9595
},
9696
path: clientHandler,

‎packages/storage-azure/src/client/AzureClientUploadHandler.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { createClientUploadHandler } from '@payloadcms/plugin-cloud-storage/clie
33
import { formatAdminURL } from 'payload/shared'
44

55
export const AzureClientUploadHandler = createClientUploadHandler({
6-
handler: async ({ apiRoute, collectionSlug, file, prefix, serverHandlerPath, serverURL }) => {
6+
handler: async ({ apiRoute, collectionSlug, docPrefix, file, serverHandlerPath, serverURL }) => {
77
const endpointRoute = formatAdminURL({
88
apiRoute,
99
path: serverHandlerPath,
@@ -12,14 +12,16 @@ export const AzureClientUploadHandler = createClientUploadHandler({
1212
const response = await fetch(endpointRoute, {
1313
body: JSON.stringify({
1414
collectionSlug,
15+
docPrefix,
1516
filename: file.name,
1617
mimeType: file.type,
1718
}),
1819
credentials: 'include',
1920
method: 'POST',
2021
})
2122

22-
const { url } = (await response.json()) as {
23+
const { docPrefix: sanitizedDocPrefix, url } = (await response.json()) as {
24+
docPrefix: string
2325
url: string
2426
}
2527

@@ -34,6 +36,6 @@ export const AzureClientUploadHandler = createClientUploadHandler({
3436
method: 'PUT',
3537
})
3638

37-
return { prefix }
39+
return { prefix: sanitizedDocPrefix }
3840
},
3941
})

‎packages/storage-azure/src/deleteFile.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ export async function deleteFile({
1717
filename,
1818
useCompositePrefixes = false,
1919
}: DeleteArgs): Promise<void> {
20-
const fileKey = getFileKey({
20+
const { fileKey } = getFileKey({
2121
collectionPrefix,
2222
docPrefix,
2323
filename,

‎packages/storage-azure/src/generateSignedURL.ts‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,8 @@ import type { ClientUploadsAccess } from '@payloadcms/plugin-cloud-storage/types
33
import type { PayloadHandler } from 'payload'
44

55
import { BlobSASPermissions, generateBlobSASQueryParameters } from '@azure/storage-blob'
6-
import path from 'path'
6+
import { getFileKey } from '@payloadcms/plugin-cloud-storage/utilities'
77
import { APIError, Forbidden } from 'payload'
8-
import { sanitizeFilename } from 'payload/shared'
98

109
import type { AzureStorageOptions } from './index.js'
1110

@@ -14,6 +13,7 @@ interface Args {
1413
collections: AzureStorageOptions['collections']
1514
containerName: string
1615
getStorageClient: () => ContainerClient
16+
useCompositePrefixes?: boolean
1717
}
1818

1919
const defaultAccess: Args['access'] = ({ req }) => !!req.user
@@ -23,31 +23,38 @@ export const getGenerateSignedURLHandler = ({
2323
collections,
2424
containerName,
2525
getStorageClient,
26+
useCompositePrefixes = false,
2627
}: Args): PayloadHandler => {
2728
return async (req) => {
2829
if (!req.json) {
2930
throw new APIError('Unreachable')
3031
}
3132

32-
const { collectionSlug, filename, mimeType } = (await req.json()) as {
33+
const { collectionSlug, docPrefix, filename, mimeType } = (await req.json()) as {
3334
collectionSlug: string
35+
docPrefix?: string
3436
filename: string
3537
mimeType: string
3638
}
3739

38-
const collectionS3Config = collections[collectionSlug]
39-
if (!collectionS3Config) {
40-
throw new APIError(`Collection ${collectionSlug} was not found in S3 options`)
40+
const collectionStorageConfig = collections[collectionSlug]
41+
if (!collectionStorageConfig) {
42+
throw new APIError(`Collection ${collectionSlug} was not found in Azure storage options`)
4143
}
4244

43-
const prefix = (typeof collectionS3Config === 'object' && collectionS3Config.prefix) || ''
45+
const collectionPrefix =
46+
(typeof collectionStorageConfig === 'object' && collectionStorageConfig.prefix) || ''
4447

4548
if (!(await access({ collectionSlug, req }))) {
4649
throw new Forbidden()
4750
}
4851

49-
const sanitizedFilename = sanitizeFilename(filename)
50-
const fileKey = path.posix.join(prefix, sanitizedFilename)
52+
const { fileKey, sanitizedDocPrefix } = getFileKey({
53+
collectionPrefix,
54+
docPrefix: docPrefix || '',
55+
filename,
56+
useCompositePrefixes,
57+
})
5158

5259
const blobClient = getStorageClient().getBlobClient(fileKey)
5360

@@ -63,6 +70,9 @@ export const getGenerateSignedURLHandler = ({
6370
getStorageClient().credential as StorageSharedKeyCredential,
6471
)
6572

66-
return Response.json({ url: `${blobClient.url}?${sasToken.toString()}` })
73+
return Response.json({
74+
docPrefix: sanitizedDocPrefix,
75+
url: `${blobClient.url}?${sasToken.toString()}`,
76+
})
6777
}
6878
}

‎packages/storage-azure/src/generateURL.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ export function generateURL({
1717
prefix,
1818
useCompositePrefixes = false,
1919
}: GenerateURLArgs): string {
20-
const fileKey = getFileKey({
20+
const { fileKey } = getFileKey({
2121
collectionPrefix,
2222
docPrefix: prefix,
2323
filename,

0 commit comments

Comments
 (0)