diff --git a/.changeset/slimy-oranges-argue.md b/.changeset/slimy-oranges-argue.md new file mode 100644 index 000000000000..f28ff49e1a1f --- /dev/null +++ b/.changeset/slimy-oranges-argue.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fixes a bug where responses can be returned before session data is saved diff --git a/.changeset/tough-cars-yawn.md b/.changeset/tough-cars-yawn.md new file mode 100644 index 000000000000..534018e45f05 --- /dev/null +++ b/.changeset/tough-cars-yawn.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fixes a bug where session data could be corrupted if it is changed after calling .set() diff --git a/.gitignore b/.gitignore index d6a28ec1b19a..8626a30a0ec1 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,7 @@ dist/ .vercel .netlify _site/ +.astro/ scripts/smoke/*-main/ scripts/memory/project/src/pages/ benchmark/projects/ diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts index 7d33ef1d0fba..b232ebbff518 100644 --- a/packages/astro/src/core/app/index.ts +++ b/packages/astro/src/core/app/index.ts @@ -298,7 +298,7 @@ export class App { this.#logger.error(null, err.stack || err.message || String(err)); return this.#renderError(request, { locals, status: 500, error: err, clientAddress }); } finally { - session?.[PERSIST_SYMBOL](); + await session?.[PERSIST_SYMBOL](); } if ( @@ -412,7 +412,7 @@ export class App { }); } } finally { - session?.[PERSIST_SYMBOL](); + await session?.[PERSIST_SYMBOL](); } } diff --git a/packages/astro/src/core/session.ts b/packages/astro/src/core/session.ts index 9b5e4eb9e3bb..ccb01ef6117d 100644 --- a/packages/astro/src/core/session.ts +++ b/packages/astro/src/core/session.ts @@ -1,4 +1,4 @@ -import { stringify, unflatten } from 'devalue'; +import { stringify as rawStringify, unflatten as rawUnflatten } from 'devalue'; import { type BuiltinDriverOptions, type Driver, @@ -26,6 +26,20 @@ interface SessionEntry { expires?: number; } +const unflatten: typeof rawUnflatten = (parsed, _) => { + // Revive URL objects + return rawUnflatten(parsed, { + URL: (href) => new URL(href), + }); +}; + +const stringify: typeof rawStringify = (data, _) => { + return rawStringify(data, { + // Support URL objects + URL: (val) => val instanceof URL && val.href, + }); +}; + export class AstroSession { // The cookies object. #cookies: AstroCookies; @@ -138,9 +152,12 @@ export class AstroSession { message: 'The session key was not provided.', }); } + // save a clone of the passed in object so later updates are not + // persisted into the store. Attempting to serialize also allows + // us to throw an error early if needed. + let cloned: T; try { - // Attempt to serialize the value so we can throw an error early if needed - stringify(value); + cloned = unflatten(JSON.parse(stringify(value))); } catch (err) { throw new AstroError( { @@ -160,7 +177,7 @@ export class AstroSession { // If ttl is numeric, it is the number of seconds until expiry. To get an expiry timestamp, we convert to milliseconds and add to the current time. const expires = typeof lifetime === 'number' ? Date.now() + lifetime * 1000 : lifetime; this.#data.set(key, { - data: value, + data: cloned, expires, }); this.#dirty = true; @@ -221,10 +238,7 @@ export class AstroSession { const key = this.#ensureSessionID(); let serialized; try { - serialized = stringify(data, { - // Support URL objects - URL: (val) => val instanceof URL && val.href, - }); + serialized = stringify(data); } catch (err) { throw new AstroError( { @@ -293,10 +307,7 @@ export class AstroSession { } try { - const storedMap = unflatten(raw, { - // Revive URL objects - URL: (href) => new URL(href), - }); + const storedMap = unflatten(raw); if (!(storedMap instanceof Map)) { await this.#destroySafe(); throw new AstroError({ diff --git a/packages/astro/test/fixtures/sessions/src/actions/index.ts b/packages/astro/test/fixtures/sessions/src/actions/index.ts index 33ac1cb653fb..856f68ba8fb1 100644 --- a/packages/astro/test/fixtures/sessions/src/actions/index.ts +++ b/packages/astro/test/fixtures/sessions/src/actions/index.ts @@ -21,7 +21,16 @@ export const server = { accept: 'json', handler: async (input, context) => { await context.session.set('cart', []); - return {cart: [], message: 'Cart cleared at ' + new Date().toTimeString() }; + return { cart: [], message: 'Cart cleared at ' + new Date().toTimeString() }; }, }), -}; + addUrl: defineAction({ + input: z.object({ favoriteUrl: z.string().url() }), + handler: async (input, context) => { + const previousFavoriteUrl = await context.session.get('favoriteUrl'); + const url = new URL(input.favoriteUrl); + context.session.set('favoriteUrl', url); + return { message: 'Favorite URL set to ' + url.href + ' from ' + (previousFavoriteUrl?.href ?? "nothing") }; + } + }) +} diff --git a/packages/astro/test/fixtures/sessions/src/pages/update.ts b/packages/astro/test/fixtures/sessions/src/pages/update.ts new file mode 100644 index 000000000000..71b058e75321 --- /dev/null +++ b/packages/astro/test/fixtures/sessions/src/pages/update.ts @@ -0,0 +1,10 @@ +import type { APIRoute } from 'astro'; + +export const GET: APIRoute = async (context) => { + const previousObject = await context.session.get("key") ?? { value: "none" }; + const previousValue = previousObject.value; + const sessionData = { value: "expected" }; + context.session.set("key", sessionData); + sessionData.value = "unexpected"; + return Response.json({previousValue}); +}; diff --git a/packages/astro/test/sessions.test.js b/packages/astro/test/sessions.test.js index 8490e78ba3e1..1dbc304bd295 100644 --- a/packages/astro/test/sessions.test.js +++ b/packages/astro/test/sessions.test.js @@ -1,5 +1,6 @@ import assert from 'node:assert/strict'; import { before, describe, it } from 'node:test'; +import * as devalue from 'devalue'; import testAdapter from './test-adapter.js'; import { loadFixture } from './test-utils.js'; @@ -43,5 +44,52 @@ describe('Astro.session', () => { const secondSessionId = secondHeaders[0].split(';')[0].split('=')[1]; assert.notEqual(firstSessionId, secondSessionId); }); + + it('can save session data by value', async () => { + const firstResponse = await fetchResponse('/update', { method: 'GET' }); + const firstValue = await firstResponse.json(); + assert.equal(firstValue.previousValue, 'none'); + + const firstHeaders = Array.from(app.setCookieHeaders(firstResponse)); + const firstSessionId = firstHeaders[0].split(';')[0].split('=')[1]; + const secondResponse = await fetchResponse('/update', { + method: 'GET', + headers: { + cookie: `astro-session=${firstSessionId}`, + }, + }); + const secondValue = await secondResponse.json(); + assert.equal(secondValue.previousValue, 'expected'); + }); + + it('can save and restore URLs in session data', async () => { + const firstResponse = await fetchResponse('/_actions/addUrl', { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + }, + body: JSON.stringify({ favoriteUrl: 'https://domain.invalid' }), + }); + + assert.equal(firstResponse.ok, true); + const firstHeaders = Array.from(app.setCookieHeaders(firstResponse)); + const firstSessionId = firstHeaders[0].split(';')[0].split('=')[1]; + + const data = devalue.parse(await firstResponse.text()); + assert.equal(data.message, 'Favorite URL set to https://domain.invalid/ from nothing'); + const secondResponse = await fetchResponse('/_actions/addUrl', { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + cookie: `astro-session=${firstSessionId}`, + }, + body: JSON.stringify({ favoriteUrl: 'https://example.com' }), + }); + const secondData = devalue.parse(await secondResponse.text()); + assert.equal( + secondData.message, + 'Favorite URL set to https://example.com/ from https://domain.invalid/', + ); + }); }); });