diff --git a/packages/void/src/pages/protocol.ts b/packages/void/src/pages/protocol.ts index e1a72e701..ba12f1994 100644 --- a/packages/void/src/pages/protocol.ts +++ b/packages/void/src/pages/protocol.ts @@ -109,6 +109,12 @@ function appendVary(headers: Headers, name: string): void { } } +function varyPagesResponse(c: CloudContext): void { + const headers = new Headers(c.res.headers); + appendVary(headers, 'X-VoidPages'); + c.header('Vary', headers.get('Vary')!); +} + function escapeHtmlAttr(value: string): string { return value.replaceAll('&', '&').replaceAll('"', '"').replaceAll('<', '<'); } @@ -464,6 +470,9 @@ export async function handlePageGet( // Redirect/error responses from the loader are returned before rendering return runLoader(handler, c, async (props) => { + if (renderMode !== 'island') { + varyPagesResponse(c); + } // Read at render time so per-route middleware can set them before next() const shared = (c.get('shared') as Record | undefined) ?? {}; const pageHead = headFn ? headFn(c, props) : undefined; @@ -538,9 +547,6 @@ export async function handlePageGet( } // Stream NDJSON: first line is PageObject, subsequent lines are deferred groups c.header('X-VoidPages', 'true'); - const headers = new Headers(c.res.headers); - appendVary(headers, 'X-VoidPages'); - c.header('Vary', headers.get('Vary')!); return streamDeferredResponse( JSON.stringify(pageObj) + '\n', deferredMap, @@ -652,6 +658,7 @@ export async function handlePageMutation( async function respondWithPage(errors: Record | null): Promise { try { return await runLoader(getHandler, c, async (props) => { + varyPagesResponse(c); const pageObj = buildMutationPageObj(stripDeferred(props), errors); if (isPagesRequest(c)) { return pageJsonResponse(c, pageObj); diff --git a/packages/void/test/unit/pages-protocol.test.ts b/packages/void/test/unit/pages-protocol.test.ts index e4d259002..e7ed05d26 100644 --- a/packages/void/test/unit/pages-protocol.test.ts +++ b/packages/void/test/unit/pages-protocol.test.ts @@ -67,26 +67,64 @@ const mockRenderFn = vi.fn(async (pageObj: RenderPage) => `${pageObj.compo const mockGetAssetTags = vi.fn(async () => ({ css: '', preloads: '', body: '' })); describe('handlePageGet', () => { - it.each([false, true])( - 'preserves loader Vary headers on Pages JSON (deferred=%s)', - async (deferred) => { - const c = await createTestContext('GET', '/users', { headers: { 'x-voidpages': 'true' } }); - const response = await handlePageGet( - (ctx) => { - ctx.header('Vary', 'Accept-Language'); - return deferred ? { users: defer(async () => []) } : { users: [] }; - }, - undefined, - c, - 'users', - mockRenderFn, - mockGetAssetTags, - ); - expect(response.headers.get('X-VoidPages')).toBe('true'); - expect(response.headers.get('Vary')).toBe('Accept-Language, X-VoidPages'); - await response.text(); - }, - ); + describe.each([ + { name: 'HTML', spa: false, deferred: false, contentType: 'text/html; charset=utf-8' }, + { name: 'JSON', spa: true, deferred: false, contentType: 'application/json' }, + { name: 'streamed HTML', spa: false, deferred: true, contentType: 'text/html; charset=utf-8' }, + { name: 'streamed NDJSON', spa: true, deferred: true, contentType: 'application/x-ndjson' }, + ])('$name cache negotiation', ({ spa, deferred, contentType }) => { + it.each([ + { existing: undefined, expected: 'X-VoidPages' }, + { existing: 'Accept-Language', expected: 'Accept-Language, X-VoidPages' }, + { existing: 'Accept-Language, x-voidpages', expected: 'Accept-Language, x-voidpages' }, + { existing: '*', expected: '*' }, + ])( + 'sets Vary without losing or duplicating existing values ($existing)', + async ({ existing, expected }) => { + const app = new Hono(); + app.get('/users', (c) => + handlePageGet( + (ctx) => { + if (existing) { + ctx.header('Vary', existing); + } + return deferred ? { users: defer(async () => []) } : { users: [] }; + }, + undefined, + c, + 'users', + mockRenderFn, + mockGetAssetTags, + ), + ); + const response = await app.request('/users', { + headers: spa ? { 'x-voidpages': 'true' } : {}, + }); + // Consume streams even if a header assertion fails. + await response.text(); + expect(response.headers.get('Content-Type')).toBe(contentType); + expect(response.headers.get('X-VoidPages')).toBe(spa ? 'true' : null); + expect(response.headers.get('Vary')).toBe(expected); + }, + ); + }); + + it.each([false, true])('does not vary HTML-only island pages (deferred=%s)', async (deferred) => { + const c = await createTestContext('GET', '/users', { headers: { 'x-voidpages': 'true' } }); + const response = await handlePageGet( + () => (deferred ? { users: defer(async () => []) } : { users: [] }), + undefined, + c, + 'users', + mockRenderFn, + mockGetAssetTags, + undefined, + { renderMode: 'island' }, + ); + await response.text(); + expect(response.headers.get('Content-Type')).toBe('text/html; charset=utf-8'); + expect(response.headers.get('Vary')).toBeNull(); + }); it('returns JSON page object when X-VoidPages header present', async () => { const c = await createTestContext('GET', '/users', { @@ -523,6 +561,26 @@ describe('handlePageGet', () => { }); describe('handlePageMutation', () => { + it.each([false, true])('varies re-rendered action pages by X-VoidPages (spa=%s)', async (spa) => { + const c = await createTestContext('POST', '/users', { + headers: spa ? { 'x-voidpages': 'true' } : {}, + }); + const response = await handlePageMutation( + () => undefined, + (ctx) => { + ctx.header('Vary', 'Accept-Language'); + return { users: [] }; + }, + undefined, + c, + 'users', + mockRenderFn, + mockGetAssetTags, + ); + await response.text(); + expect(response.headers.get('Vary')).toBe('Accept-Language, X-VoidPages'); + }); + it.each>([ { origin: 'https://attacker.test' }, { origin: 'http://sibling.localhost', 'sec-fetch-site': 'same-site' },