Skip to content

Commit

Permalink
[fix] handle set-cookie in setHeaders (#6033)
Browse files Browse the repository at this point in the history
* [fix] handle set-cookie in setHeaders

Fixes #6032

* Update packages/kit/src/runtime/server/index.js

Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>

* Update packages/kit/src/runtime/server/index.js

* separate out cookie handling more explicitly

Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
Co-authored-by: Rich Harris <richard.a.harris@gmail.com>
Co-authored-by: Rich Harris <hello@rich-harris.dev>
  • Loading branch information
4 people authored Aug 18, 2022
1 parent e9b8c65 commit ca9f87d
Show file tree
Hide file tree
Showing 7 changed files with 53 additions and 16 deletions.
5 changes: 5 additions & 0 deletions .changeset/lemon-kids-double.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

handle `set-cookie` in `setHeaders`
45 changes: 29 additions & 16 deletions packages/kit/src/runtime/server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,9 @@ export async function respond(request, options, state) {
/** @type {import('types').ResponseHeaders} */
const headers = {};

/** @type {string[]} */
const cookies = [];

/** @type {import('types').RequestEvent} */
const event = {
get clientAddress() {
Expand All @@ -141,16 +144,26 @@ export async function respond(request, options, state) {
setHeaders: (new_headers) => {
for (const key in new_headers) {
const lower = key.toLowerCase();
const value = new_headers[key];

if (lower in headers) {
throw new Error(`"${key}" header is already set`);
}
if (lower === 'set-cookie') {
const new_cookies = /** @type {string[]} */ (Array.isArray(value) ? value : [value]);

// TODO apply these headers to the response
headers[lower] = new_headers[key];
for (const cookie of new_cookies) {
if (cookies.includes(cookie)) {
throw new Error(`"${key}" header already has cookie with same value`);
}

if (state.prerendering && lower === 'cache-control') {
state.prerendering.cache = /** @type {string} */ (new_headers[key]);
cookies.push(cookie);
}
} else if (lower in headers) {
throw new Error(`"${key}" header is already set`);
} else {
headers[lower] = value;

if (state.prerendering && lower === 'cache-control') {
state.prerendering.cache = /** @type {string} */ (value);
}
}
}
},
Expand Down Expand Up @@ -312,19 +325,19 @@ export async function respond(request, options, state) {
: await render_page(event, route, options, state, resolve_opts);
}

for (const key in headers) {
const value = headers[key];
if (key === 'set-cookie') {
for (const cookie of Array.isArray(value) ? value : [value]) {
response.headers.append(key, /** @type {string} */ (cookie));
}
} else if (!is_data_request) {
// we only want to set cookies on __data.json requests, we don't
// want to cache stuff erroneously etc
if (!is_data_request) {
// we only want to set cookies on __data.json requests, we don't
// want to cache stuff erroneously etc
for (const key in headers) {
const value = headers[key];
response.headers.set(key, /** @type {string} */ (value));
}
}

for (const cookie of cookies) {
response.headers.append('set-cookie', cookie);
}

// respond with 304 if etag matches
if (response.status === 200 && response.headers.has('etag')) {
let if_none_match_value = request.headers.get('if-none-match');
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export function load({ setHeaders }) {
setHeaders({
'set-cookie': 'cookie1=value1'
});
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<slot />
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export function load({ setHeaders }) {
setHeaders({
'set-cookie': 'cookie2=value2'
});
}
Empty file.
8 changes: 8 additions & 0 deletions packages/kit/test/apps/basics/test/server.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,14 @@ test.describe('Static files', () => {
});
});

test.describe('setHeaders', () => {
test('allows multiple set-cookie headers with different values', async ({ page }) => {
const response = await page.goto('/headers/set-cookie/sub');
const cookies = (await response?.allHeaders())['set-cookie'];
expect(cookies.includes('cookie1=value1') && cookies.includes('cookie2=value2')).toBe(true);
});
});

test.describe('Miscellaneous', () => {
test('does not serve version.json with an immutable cache header', async ({ request }) => {
// this isn't actually a great test, because caching behaviour is down to adapters.
Expand Down

0 comments on commit ca9f87d

Please sign in to comment.