Skip to content

Commit 7ccc3f0

Browse files
perf(storage-s3): avoid getObject call if ETag matches (#15295)
### What? Avoid unnecessary getObject call to S3 when the ETag matches. ### Why? Range support introduced an early headObject call, which already returns ETag and metadata; we can use that to short‑circuit with 304 and save a fetch. ### How? By using response headers from headObject and check ETag matching before invoking getObject. Added a test case for verifying 200 status when If-None-Match does not match, and 304 status with empty body when If-None-Match is included and matching. --------- Co-authored-by: Paul Popus <paul@payloadcms.com>
1 parent 43f0d20 commit 7ccc3f0

2 files changed

Lines changed: 50 additions & 18 deletions

File tree

packages/storage-s3/src/staticHandler.ts

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ export const getHandler = ({
111111
}
112112
}
113113

114-
// Get file size first for range validation
114+
// Get file size first for range validation and to set Content-Length header before streaming
115115
const headObject = await getStorageClient().headObject({
116116
Bucket: bucket,
117117
Key: key,
@@ -138,36 +138,25 @@ export const getHandler = ({
138138
? `bytes=${rangeResult.rangeStart}-${rangeResult.rangeEnd}`
139139
: undefined
140140

141-
object = await getStorageClient().getObject(
142-
{
143-
Bucket: bucket,
144-
Key: key,
145-
Range: rangeForS3,
146-
},
147-
{ abortSignal: abortController.signal },
148-
)
149-
150-
if (!object.Body) {
151-
return new Response(null, { status: 404, statusText: 'Not Found' })
152-
}
153-
154141
let headers = new Headers(incomingHeaders)
155142

156143
// Add range-related headers from the result
157144
for (const [key, value] of Object.entries(rangeResult.headers)) {
158145
headers.append(key, value)
159146
}
160147

161-
headers.append('Content-Type', String(object.ContentType))
162-
headers.append('ETag', String(object.ETag))
148+
headers.append('Content-Type', String(headObject.ContentType))
149+
if (headObject.ETag) {
150+
headers.append('ETag', headObject.ETag)
151+
}
163152

164153
// Add Content-Security-Policy header for SVG files to prevent executable code
165-
if (object.ContentType === 'image/svg+xml') {
154+
if (headObject.ContentType === 'image/svg+xml') {
166155
headers.append('Content-Security-Policy', "script-src 'none'")
167156
}
168157

169158
const etagFromHeaders = req.headers.get('etag') || req.headers.get('if-none-match')
170-
const objectEtag = object.ETag
159+
const objectEtag = headObject.ETag
171160

172161
if (
173162
collection.upload &&
@@ -184,6 +173,19 @@ export const getHandler = ({
184173
})
185174
}
186175

176+
object = await getStorageClient().getObject(
177+
{
178+
Bucket: bucket,
179+
Key: key,
180+
Range: rangeForS3,
181+
},
182+
{ abortSignal: abortController.signal },
183+
)
184+
185+
if (!object.Body) {
186+
return new Response(null, { status: 404, statusText: 'Not Found' })
187+
}
188+
187189
if (!isNodeReadableStream(object.Body)) {
188190
req.payload.logger.error({
189191
key,

test/storage-s3/int.spec.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,36 @@ describe('@payloadcms/storage-s3', () => {
135135
expect(response.status).toBe(404)
136136
})
137137

138+
it('should return 304 with empty body when the ETag matches', async () => {
139+
await payload.create({
140+
collection: mediaWithSignedDownloadsSlug,
141+
data: {},
142+
filePath: path.resolve(dirname, '../uploads/temp.png'),
143+
})
144+
145+
const response = await restClient.GET(`/${mediaWithSignedDownloadsSlug}/file/temp.png`, {
146+
headers: { 'X-Disable-Signed-URL': 'true', 'If-None-Match': 'invalid-etag-1234' },
147+
})
148+
expect(response.status).toBe(200)
149+
expect(response.headers.get('Content-Type')).toBe('image/png')
150+
151+
const etag = response.headers.get('ETag')
152+
expect(etag).toBeDefined()
153+
154+
const responseNotModified = await restClient.GET(
155+
`/${mediaWithSignedDownloadsSlug}/file/temp.png`,
156+
{
157+
headers: {
158+
'X-Disable-Signed-URL': 'true',
159+
'If-None-Match': etag!,
160+
},
161+
},
162+
)
163+
expect(responseNotModified.status).toBe(304)
164+
const body = await responseNotModified.text()
165+
expect(body).toBe('')
166+
})
167+
138168
describe('disablePayloadAccessControl', () => {
139169
it('should return direct S3 URL with encoded filename when uploading file with spaces', async () => {
140170
const upload = await payload.create({

0 commit comments

Comments
 (0)