Skip to content

Commit aa33b44

Browse files
authored
fix(cache): skip caching Astro cookie responses (#17781)
1 parent adc750f commit aa33b44

3 files changed

Lines changed: 96 additions & 1 deletion

File tree

.changeset/long-tips-care.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'astro': patch
3+
---
4+
5+
Fixes `memoryCache()` storing responses that set cookies through `Astro.cookies` or `Astro.session`

packages/astro/src/core/cache/memory-provider.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import picomatch from 'picomatch';
2+
import type { AstroCookies } from '../cookies/cookies.js';
3+
import { getCookiesFromResponse } from '../cookies/response.js';
24
import { AstroError } from '../errors/errors.js';
35
import { CacheQueryConfigConflict } from '../errors/errors-data.js';
46
import type { CacheProvider, CacheProviderFactory, InvalidateOptions } from './types.js';
@@ -255,8 +257,13 @@ function matchesVary(request: Request, entry: CachedEntry): boolean {
255257
return true;
256258
}
257259

260+
function hasAtLeastOneCookie(cookies: AstroCookies | undefined): boolean {
261+
return cookies ? !cookies.headers().next().done : false;
262+
}
263+
258264
function hasSetCookieHeader(response: Response): boolean {
259-
return response.headers.has('set-cookie');
265+
if (response.headers.has('set-cookie')) return true;
266+
return hasAtLeastOneCookie(getCookiesFromResponse(response));
260267
}
261268

262269
function warnSkippedSetCookie(url: URL): void {

packages/astro/test/units/cache/app-cache.test.ts

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import assert from 'node:assert/strict';
22
import { describe, it } from 'node:test';
3+
import sessionMemoryDriver from 'unstorage/drivers/memory';
34
import memoryProvider from '../../../dist/core/cache/memory-provider.js';
45
import { createComponent, render, renderHead } from '../../../dist/runtime/server/index.js';
56
import { createEndpoint, createPage, createTestApp } from '../mocks.ts';
@@ -12,6 +13,14 @@ function createCacheManifestOverrides() {
1213
};
1314
}
1415

16+
function createSessionCacheManifestOverrides() {
17+
return {
18+
...createCacheManifestOverrides(),
19+
sessionConfig: { driver: 'memory', cookie: 'astro-session' },
20+
sessionDriver: async () => ({ default: () => sessionMemoryDriver() }),
21+
};
22+
}
23+
1524
// #region Route factories
1625

1726
function cachedEndpoint() {
@@ -73,6 +82,33 @@ function withCookieEndpoint() {
7382
);
7483
}
7584

85+
function withAstroCookieEndpoint() {
86+
return createEndpoint(
87+
{
88+
GET: (ctx: APIContext) => {
89+
ctx.cache.set({ maxAge: 300, tags: ['astro-cookie'] });
90+
ctx.cookies.set('session', 'test', { path: '/', httpOnly: true });
91+
return Response.json({ nonce: Math.random() });
92+
},
93+
},
94+
{ route: '/with-astro-cookie' },
95+
);
96+
}
97+
98+
function withSessionEndpoint() {
99+
return createEndpoint(
100+
{
101+
GET: async (ctx: APIContext) => {
102+
ctx.cache.set({ maxAge: 300, tags: ['session'] });
103+
const count = ((await ctx.session!.get<number>('count')) ?? 0) + 1;
104+
ctx.session!.set('count', count);
105+
return Response.json({ count, nonce: Math.random() });
106+
},
107+
},
108+
{ route: '/with-session' },
109+
);
110+
}
111+
76112
function invalidateEndpoint() {
77113
return createEndpoint(
78114
{
@@ -269,6 +305,53 @@ describe('context.cache through App pipeline', () => {
269305
assert.notEqual(firstBody.nonce, secondBody.nonce);
270306
});
271307

308+
it('does not cache responses that set Astro cookies', async () => {
309+
const overrides = createCacheManifestOverrides();
310+
const app = createTestApp([withAstroCookieEndpoint()], overrides);
311+
312+
const first = await app.render(new Request('http://localhost/with-astro-cookie'), {
313+
addCookieHeader: true,
314+
});
315+
assert.equal(first.headers.get('X-Astro-Cache'), null);
316+
assert.ok(first.headers.get('Set-Cookie'));
317+
const firstBody = await first.json();
318+
319+
const second = await app.render(new Request('http://localhost/with-astro-cookie'), {
320+
addCookieHeader: true,
321+
});
322+
assert.equal(second.headers.get('X-Astro-Cache'), null);
323+
assert.ok(second.headers.get('Set-Cookie'));
324+
const secondBody = await second.json();
325+
326+
assert.notEqual(firstBody.nonce, secondBody.nonce);
327+
});
328+
329+
it('does not cache responses that set an Astro session', async () => {
330+
const overrides = createSessionCacheManifestOverrides();
331+
const app = createTestApp([withSessionEndpoint()], overrides);
332+
333+
const first = await app.render(new Request('http://localhost/with-session'), {
334+
addCookieHeader: true,
335+
});
336+
assert.equal(first.headers.get('X-Astro-Cache'), null);
337+
const cookie = first.headers.get('Set-Cookie');
338+
assert.ok(cookie);
339+
const firstBody = await first.json();
340+
assert.equal(firstBody.count, 1);
341+
342+
const second = await app.render(
343+
new Request('http://localhost/with-session', {
344+
headers: { Cookie: cookie.split(';', 1)[0] },
345+
}),
346+
{ addCookieHeader: true },
347+
);
348+
assert.equal(second.headers.get('X-Astro-Cache'), null);
349+
const secondBody = await second.json();
350+
351+
assert.equal(secondBody.count, 2);
352+
assert.notEqual(firstBody.nonce, secondBody.nonce);
353+
});
354+
272355
it('normalizes query parameter order (sorting)', async () => {
273356
const overrides = createCacheManifestOverrides();
274357
const app = createTestApp([cachedEndpoint()], overrides);

0 commit comments

Comments
 (0)