Skip to content

Commit 4f36c23

Browse files
committed
fix: ResponseCache.add() must not evict entries revalidatable via Last-Modified
add()'s expiry cleanup deleted any cache entry without an ETag as soon as it was expired, ignoring Last-Modified entirely - unlike get(), which keeps an expired entry with a Last-Modified date around as "stale" so it can be revalidated with a conditional If-Modified-Since request. This mismatch meant a response with a very small/zero max-age plus a Last-Modified header (a common "always revalidate" pattern) was purged from the cache immediately instead of being kept for revalidation, causing the next request to be an ordinary uncached GET instead of a conditional one. This also caused the intermittent SyncFetch.test.ts cache-revalidation failures on master. Adds deterministic regression tests in Fetch.test.ts and SyncFetch.test.ts: an already-expired Last-Modified response with no ETag or Cache-Control must stay cached so the next request is a conditional If-Modified-Since GET.
1 parent eac5a38 commit 4f36c23

3 files changed

Lines changed: 132 additions & 2 deletions

File tree

packages/happy-dom/src/fetch/cache/response/ResponseCache.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -201,8 +201,13 @@ export default class ResponseCache implements IResponseCache {
201201
}
202202
}
203203

204-
// Cache is invalid if it has expired and doesn't have an ETag.
205-
if (!cachedResponse.etag && (!cachedResponse.expires || cachedResponse.expires < Date.now())) {
204+
// Cache is invalid if it has expired and doesn't have an ETag
205+
// or a Last-Modified date to revalidate against.
206+
if (
207+
!cachedResponse.etag &&
208+
!cachedResponse.lastModified &&
209+
(!cachedResponse.expires || cachedResponse.expires < Date.now())
210+
) {
206211
const entries = this.#entries.get(url);
207212
if (entries) {
208213
const index = entries.indexOf(cachedResponse);

packages/happy-dom/test/fetch/Fetch.test.ts

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3856,6 +3856,76 @@ describe('Fetch', () => {
38563856
]);
38573857
});
38583858

3859+
it('Keeps an already-expired "Last-Modified" response in the cache so the next request revalidates it with "If-Modified-Since" (no "ETag", no "max-age").', async () => {
3860+
const window = new Window({ url: 'https://localhost:8080/' });
3861+
const url = 'https://localhost:8080/some/path';
3862+
const responseText = 'some text';
3863+
const requestArgs: Array<{
3864+
url: string;
3865+
options: { method: string; headers: { [k: string]: string } };
3866+
}> = [];
3867+
3868+
mockModule('https', {
3869+
request: (url, options) => {
3870+
requestArgs.push({ url, options });
3871+
3872+
return {
3873+
end: () => {},
3874+
on: (event: string, callback: (response: HTTP.IncomingMessage) => void) => {
3875+
if (event === 'response') {
3876+
if (options.headers['If-Modified-Since']) {
3877+
const response = <HTTP.IncomingMessage>Stream.Readable.from([]);
3878+
3879+
response.statusCode = 304;
3880+
response.statusMessage = 'Not Modified';
3881+
response.headers = {};
3882+
response.rawHeaders = ['last-modified', 'Mon, 11 Dec 2023 02:00:00 GMT'];
3883+
3884+
callback(response);
3885+
} else {
3886+
async function* generate(): AsyncGenerator<string> {
3887+
yield responseText;
3888+
}
3889+
3890+
const response = <HTTP.IncomingMessage>Stream.Readable.from(generate());
3891+
3892+
response.statusCode = 200;
3893+
response.statusMessage = 'OK';
3894+
response.headers = {};
3895+
// Already stale (Expires in the past) but revalidatable via Last-Modified, no ETag, no Cache-Control.
3896+
response.rawHeaders = [
3897+
'content-type',
3898+
'text/html',
3899+
'content-length',
3900+
String(responseText.length),
3901+
'last-modified',
3902+
'Mon, 11 Dec 2023 01:00:00 GMT',
3903+
'expires',
3904+
'Wed, 21 Oct 2015 07:28:00 GMT'
3905+
];
3906+
3907+
callback(response);
3908+
}
3909+
}
3910+
},
3911+
setTimeout: () => {}
3912+
};
3913+
}
3914+
});
3915+
3916+
await (await window.fetch(url)).text();
3917+
3918+
const response2 = await window.fetch(url);
3919+
await response2.text();
3920+
3921+
expect(requestArgs.length).toBe(2);
3922+
expect(requestArgs[1].options.headers['If-Modified-Since']).toBe(
3923+
'Mon, 11 Dec 2023 01:00:00 GMT'
3924+
);
3925+
expect(response2.status).toBe(200);
3926+
expect(response2.headers.get('Last-Modified')).toBe('Mon, 11 Dec 2023 02:00:00 GMT');
3927+
});
3928+
38593929
it('Updates cache after a failed revalidation with a "If-Modified-Since" request for a GET response with "Cache-Control" set to a "max-age".', async () => {
38603930
const window = new Window({ url: 'https://localhost:8080/' });
38613931
const url = '/some/path';

packages/happy-dom/test/fetch/SyncFetch.test.ts

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3056,6 +3056,61 @@ describe('SyncFetch', () => {
30563056
]);
30573057
});
30583058

3059+
it('Keeps an already-expired "Last-Modified" response in the cache so the next request revalidates it with "If-Modified-Since" (no "ETag", no "max-age").', async () => {
3060+
browserFrame.url = 'https://localhost:8080/';
3061+
3062+
const url = 'https://localhost:8080/some/path';
3063+
const responseText = 'some text';
3064+
const requestArgs: string[] = [];
3065+
3066+
mockModule('child_process', {
3067+
execFileSync: (_command: string, args: string[]) => {
3068+
requestArgs.push(args[1]);
3069+
3070+
if (args[1].includes('If-Modified-Since')) {
3071+
return JSON.stringify({
3072+
error: null,
3073+
incomingMessage: {
3074+
statusCode: 304,
3075+
statusMessage: 'Not Modified',
3076+
rawHeaders: ['last-modified', 'Mon, 11 Dec 2023 02:00:00 GMT'],
3077+
data: ''
3078+
}
3079+
});
3080+
}
3081+
return JSON.stringify({
3082+
error: null,
3083+
incomingMessage: {
3084+
statusCode: 200,
3085+
statusMessage: 'OK',
3086+
// Already stale (Expires in the past) but revalidatable via Last-Modified, no ETag, no Cache-Control.
3087+
rawHeaders: [
3088+
'content-type',
3089+
'text/html',
3090+
'content-length',
3091+
String(responseText.length),
3092+
'last-modified',
3093+
'Mon, 11 Dec 2023 01:00:00 GMT',
3094+
'expires',
3095+
'Wed, 21 Oct 2015 07:28:00 GMT'
3096+
],
3097+
data: Buffer.from(responseText).toString('base64')
3098+
}
3099+
});
3100+
}
3101+
});
3102+
3103+
new SyncFetch({ browserFrame, window, url }).send();
3104+
3105+
const response2 = new SyncFetch({ browserFrame, window, url }).send();
3106+
3107+
expect(requestArgs.length).toBe(2);
3108+
expect(requestArgs[1]).toContain('If-Modified-Since');
3109+
expect(requestArgs[1]).toContain('Mon, 11 Dec 2023 01:00:00 GMT');
3110+
expect(response2.status).toBe(200);
3111+
expect(response2.headers.get('Last-Modified')).toBe('Mon, 11 Dec 2023 02:00:00 GMT');
3112+
});
3113+
30593114
it('Updates cache after a failed revalidation with a "If-Modified-Since" request for a GET response with "Cache-Control" set to a "max-age".', async () => {
30603115
browserFrame.url = 'https://localhost:8080/';
30613116

0 commit comments

Comments
 (0)