From a6c901128bd6a128a7f85891f4ee6cd757b49d12 Mon Sep 17 00:00:00 2001 From: oliverlynch <59594611+oliverlynch@users.noreply.github.com> Date: Mon, 16 Mar 2026 20:03:30 +0900 Subject: [PATCH 1/5] fix(assets): allow 304 status to bypass redirect check while revalidating fixes #15920 --- packages/astro/src/assets/build/remote.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/astro/src/assets/build/remote.ts b/packages/astro/src/assets/build/remote.ts index 12f6ed8ae75a..4d5309915720 100644 --- a/packages/astro/src/assets/build/remote.ts +++ b/packages/astro/src/assets/build/remote.ts @@ -53,12 +53,13 @@ export async function revalidateRemoteImage( const req = new Request(src, { headers, cache: 'no-cache' }); const res = await fetch(req, { redirect: 'manual' }); - if (res.status >= 300 && res.status < 400) { - throw new Error(`Failed to revalidate cached remote image ${src}. The request was redirected.`); - } - - // Asset not modified: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/304 + // Allow 304 Not Modified: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/304 if (!res.ok && res.status !== 304) { + if (res.status >= 300 && res.status < 400) { + throw new Error( + `Failed to revalidate cached remote image ${src}. The request was redirected.`, + ); + } throw new Error( `Failed to revalidate cached remote image ${src}. The request did not return a 200 OK / 304 NOT MODIFIED response. (received ${res.status} ${res.statusText})`, ); From 53cc6b94e583e35550facef2ac40323d10efa011 Mon Sep 17 00:00:00 2001 From: oliverlynch <59594611+oliverlynch@users.noreply.github.com> Date: Mon, 16 Mar 2026 20:28:36 +0900 Subject: [PATCH 2/5] refactor(assets): improve cache validation code - replace double awaits with Promise.all - explicitly return null data from revalidateRemoteImage when there is no change - explicitly write cache file in utf-8, as we are reading it in utf-8 --- .changeset/ten-jobs-bathe.md | 5 +++ packages/astro/src/assets/build/generate.ts | 41 ++++++++++----------- packages/astro/src/assets/build/remote.ts | 6 +-- 3 files changed, 27 insertions(+), 25 deletions(-) create mode 100644 .changeset/ten-jobs-bathe.md diff --git a/.changeset/ten-jobs-bathe.md b/.changeset/ten-jobs-bathe.md new file mode 100644 index 000000000000..b81a7d44d692 --- /dev/null +++ b/.changeset/ten-jobs-bathe.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Asset cache revalidation refactor diff --git a/packages/astro/src/assets/build/generate.ts b/packages/astro/src/assets/build/generate.ts index eacbe4ff0c79..82717a397a15 100644 --- a/packages/astro/src/assets/build/generate.ts +++ b/packages/astro/src/assets/build/generate.ts @@ -176,13 +176,11 @@ export async function generateImagesForPath( } else { const JSONData = JSON.parse(readFileSync(cachedMetaFileURL, 'utf-8')) as RemoteCacheEntry; - if (!JSONData.expires) { - try { - await fs.promises.unlink(cachedFileURL); - } catch { - /* Old caches may not have a separate image binary, no-op */ - } - await fs.promises.unlink(cachedMetaFileURL); + if (typeof JSONData.expires !== 'number') { + await Promise.allSettled([ + fs.promises.unlink(cachedFileURL), + fs.promises.unlink(cachedMetaFileURL), + ]); throw new Error( `Malformed cache entry for ${filepath}, cache will be regenerated for this file.`, @@ -203,9 +201,7 @@ export async function generateImagesForPath( if (JSONData.expires > Date.now()) { await fs.promises.copyFile(cachedFileURL, finalFileURL, fs.constants.COPYFILE_FICLONE); - return { - cached: 'hit', - }; + return { cached: 'hit' }; } // Try to revalidate the cache @@ -216,18 +212,16 @@ export async function generateImagesForPath( lastModified: JSONData.lastModified, }); - if (revalidatedData.data.length) { + if (revalidatedData.data !== null) { // Image cache was stale, update original image to avoid redownload - originalImage = revalidatedData; + originalImage = revalidatedData as ImageData; } else { - // Freshen cache on disk - await writeCacheMetaFile(cachedMetaFileURL, revalidatedData, env); - - await fs.promises.copyFile( - cachedFileURL, - finalFileURL, - fs.constants.COPYFILE_FICLONE, - ); + // Freshen cache on disk and output cached image + await Promise.all([ + writeCacheMetaFile(cachedMetaFileURL, revalidatedData, env), + fs.promises.copyFile(cachedFileURL, finalFileURL, fs.constants.COPYFILE_FICLONE), + ]); + return { cached: 'revalidated' }; } } catch (e) { @@ -242,8 +236,10 @@ export async function generateImagesForPath( } } - await fs.promises.unlink(cachedFileURL); - await fs.promises.unlink(cachedMetaFileURL); + await Promise.allSettled([ + fs.promises.unlink(cachedFileURL), + fs.promises.unlink(cachedMetaFileURL), + ]); } } catch (e: any) { if (e.code !== 'ENOENT') { @@ -339,6 +335,7 @@ async function writeCacheMetaFile( etag: resultData.etag, lastModified: resultData.lastModified, }), + 'utf-8', ); } catch (e) { env.logger.warn( diff --git a/packages/astro/src/assets/build/remote.ts b/packages/astro/src/assets/build/remote.ts index 4d5309915720..8fa8d49085c7 100644 --- a/packages/astro/src/assets/build/remote.ts +++ b/packages/astro/src/assets/build/remote.ts @@ -40,7 +40,7 @@ export async function loadRemoteImage(src: string) { * The remote server may respond that the cached asset is still up-to-date if the entity-tag or modification time matches (304 Not Modified), or respond with an updated asset (200 OK) * @param src - url to remote asset * @param revalidationData - an object containing the stored Entity-Tag of the cached asset and/or the Last Modified time - * @returns An ImageData object containing the asset data, a new expiry time, and the asset's etag. The data buffer will be empty if the asset was not modified. + * @returns An object containing the refreshed expiry time and cache headers. `data` will be a `Buffer` of the new image if the asset was modified (200 OK), or `null` if the cached version is still valid (304 Not Modified). */ export async function revalidateRemoteImage( src: string, @@ -77,12 +77,12 @@ export async function revalidateRemoteImage( webToCachePolicyRequest(req), webToCachePolicyResponse( res.ok ? res : new Response(null, { status: 200, headers: res.headers }), - ), // 304 responses themselves are not cacheable, so just pretend to get the refreshed TTL + ), // 304 responses are not cacheable, so just use its headers to get the refreshed TTL ); const expires = policy.storable() ? policy.timeToLive() : 0; return { - data, + data: res.ok ? data : null, expires: Date.now() + expires, // While servers should respond with the same headers as a 200 response, if they don't we should reuse the stored value etag: res.headers.get('Etag') ?? (res.ok ? undefined : revalidationData.etag), From 27ff03df22435890a6df3d4ff48f0c26910aae13 Mon Sep 17 00:00:00 2001 From: oliverlynch <59594611+oliverlynch@users.noreply.github.com> Date: Mon, 16 Mar 2026 21:01:30 +0900 Subject: [PATCH 3/5] chore: reword changeset to be more descriptive --- .changeset/ten-jobs-bathe.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/ten-jobs-bathe.md b/.changeset/ten-jobs-bathe.md index b81a7d44d692..db0b1c5891d8 100644 --- a/.changeset/ten-jobs-bathe.md +++ b/.changeset/ten-jobs-bathe.md @@ -2,4 +2,4 @@ 'astro': patch --- -Asset cache revalidation refactor +Fix cached assets failing to revalidate due to redirect check mishandling Not Modified responses. From d5ccbccdaea5e1e94900a35df3bb613ef80bff8a Mon Sep 17 00:00:00 2001 From: Princesseuh <3019731+Princesseuh@users.noreply.github.com> Date: Wed, 25 Mar 2026 03:01:35 +0100 Subject: [PATCH 4/5] fix: add tests --- packages/astro/src/assets/build/remote.ts | 9 +- .../astro/test/units/assets/remote.test.js | 85 +++++++++++++++++++ 2 files changed, 90 insertions(+), 4 deletions(-) create mode 100644 packages/astro/test/units/assets/remote.test.js diff --git a/packages/astro/src/assets/build/remote.ts b/packages/astro/src/assets/build/remote.ts index 8fa8d49085c7..eae8e85f3a3b 100644 --- a/packages/astro/src/assets/build/remote.ts +++ b/packages/astro/src/assets/build/remote.ts @@ -7,9 +7,9 @@ export type RemoteCacheEntry = { lastModified?: string; }; -export async function loadRemoteImage(src: string) { +export async function loadRemoteImage(src: string, fetchFn: typeof fetch = globalThis.fetch) { const req = new Request(src); - const res = await fetch(req, { redirect: 'manual' }); + const res = await fetchFn(req, { redirect: 'manual' }); if (res.status >= 300 && res.status < 400) { throw new Error(`Failed to load remote image ${src}. The request was redirected.`); @@ -45,13 +45,14 @@ export async function loadRemoteImage(src: string) { export async function revalidateRemoteImage( src: string, revalidationData: { etag?: string; lastModified?: string }, + fetchFn: typeof fetch = globalThis.fetch, ) { const headers = { ...(revalidationData.etag && { 'If-None-Match': revalidationData.etag }), ...(revalidationData.lastModified && { 'If-Modified-Since': revalidationData.lastModified }), }; const req = new Request(src, { headers, cache: 'no-cache' }); - const res = await fetch(req, { redirect: 'manual' }); + const res = await fetchFn(req, { redirect: 'manual' }); // Allow 304 Not Modified: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/304 if (!res.ok && res.status !== 304) { @@ -69,7 +70,7 @@ export async function revalidateRemoteImage( if (res.ok && !data.length) { // Server did not include body but indicated cache was stale - return await loadRemoteImage(src); + return await loadRemoteImage(src, fetchFn); } // calculate an expiration date based on the response's TTL diff --git a/packages/astro/test/units/assets/remote.test.js b/packages/astro/test/units/assets/remote.test.js new file mode 100644 index 000000000000..0688a0abd140 --- /dev/null +++ b/packages/astro/test/units/assets/remote.test.js @@ -0,0 +1,85 @@ +// @ts-check +import assert from 'node:assert/strict'; +import { describe, it } from 'node:test'; +import { revalidateRemoteImage } from '../../../dist/assets/build/remote.js'; + +/** + * + * @param {number} status + * @param {Record} headerInit + * @param {ArrayBuffer} body + * @returns {() => Promise} + */ +function makeFetchMock(status, headerInit = {}, body = new ArrayBuffer(0)) { + const headers = new Headers(headerInit); + return async () => + /** @type {Response} */ ( + /** @type {unknown} */ ({ + status, + ok: status >= 200 && status < 300, + headers, + arrayBuffer: async () => body, + }) + ); +} + +describe('revalidateRemoteImage', () => { + it('returns null data on 304 Not Modified (cache still valid)', async () => { + const result = await revalidateRemoteImage( + 'https://example.com/img.jpg', + { etag: '"abc123"' }, + makeFetchMock(304, { 'Cache-Control': 'max-age=3600', Etag: '"abc123"' }), + ); + + // null signals "cache is still valid, use the file on disk" + assert.equal(result.data, null); + // etag should be preserved from the stored revalidation data + assert.equal(result.etag, '"abc123"'); + // expires should be a future timestamp + assert.ok(result.expires > Date.now()); + }); + + it('preserves lastModified from stored data on 304 when server omits the header', async () => { + const result = await revalidateRemoteImage( + 'https://example.com/img.jpg', + { lastModified: 'Wed, 01 Jan 2025 00:00:00 GMT' }, + makeFetchMock(304, { 'Cache-Control': 'max-age=600' }), + ); + + assert.equal(result.data, null); + assert.equal(result.lastModified, 'Wed, 01 Jan 2025 00:00:00 GMT'); + }); + + it('returns image data on 200 OK (cache was stale)', async () => { + const imageBytes = new Uint8Array([1, 2, 3, 4]).buffer; + const result = await revalidateRemoteImage( + 'https://example.com/img.jpg', + { etag: '"oldtag"' }, + makeFetchMock(200, { 'Cache-Control': 'max-age=3600', Etag: '"newetag"' }, imageBytes), + ); + + assert.ok(result.data !== null); + assert.ok(result.data.length > 0); + assert.equal(result.etag, '"newetag"'); + }); + + it('throws on redirect responses (e.g. 301)', async () => { + await assert.rejects( + () => + revalidateRemoteImage( + 'https://example.com/img.jpg', + { etag: '"abc"' }, + makeFetchMock(301, { Location: 'https://example.com/new.jpg' }), + ), + /redirected/, + ); + }); + + it('throws on server error responses (e.g. 500)', async () => { + await assert.rejects( + () => + revalidateRemoteImage('https://example.com/img.jpg', { etag: '"abc"' }, makeFetchMock(500)), + /500/, + ); + }); +}); From c2046e60aa07059bfc260bbbae50079b31e87c25 Mon Sep 17 00:00:00 2001 From: Erika <3019731+Princesseuh@users.noreply.github.com> Date: Wed, 25 Mar 2026 03:21:20 +0100 Subject: [PATCH 5/5] Apply suggestions from code review Co-authored-by: Erika <3019731+Princesseuh@users.noreply.github.com> --- .changeset/ten-jobs-bathe.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/ten-jobs-bathe.md b/.changeset/ten-jobs-bathe.md index db0b1c5891d8..a7956e09b4e6 100644 --- a/.changeset/ten-jobs-bathe.md +++ b/.changeset/ten-jobs-bathe.md @@ -2,4 +2,4 @@ 'astro': patch --- -Fix cached assets failing to revalidate due to redirect check mishandling Not Modified responses. +Fixes cached assets failing to revalidate due to redirect check mishandling Not Modified responses.