can no longer return multiple `set-cookie` headers from endpoints #3460

kamholz · 2022-01-20T18:38:04Z

Describe the bug

In a recent update, probably #3384, the ability to return multiple set-cookie headers from an endpoint seems to have broken. Instead of returning multiple set-cookie headers, it returns just one with cookie values comma-separated (which browsers cannot interpret correctly).

Reproduction

This endpoint:

export function get() {
  return {
    headers: {
      'set-cookie': [
        'accesstoken=; Path=/; Expires=Thu, 01 Jan 1970 00:00:00 GMT; HttpOnly; Secure; SameSite=Strict',
        'refreshtoken=; Path=/; Expires=Thu, 01 Jan 1970 00:00:00 GMT; HttpOnly; Secure; SameSite=Strict',
      ]
    }
  }
}

produces the following headers:

set-cookie: accesstoken=; Path=/; Expires=Thu, 01 Jan 1970 00:00:00 GMT; HttpOnly; Secure; SameSite=Strict, refreshtoken=; Path=/; Expires=Thu, 01 Jan 1970 00:00:00 GMT; HttpOnly; Secure; SameSite=Strict

but it should produce:

set-cookie: accesstoken=; Path=/; Expires=Thu, 01 Jan 1970 00:00:00 GMT; HttpOnly; Secure; SameSite=Strict
set-cookie: refreshtoken=; Path=/; Expires=Thu, 01 Jan 1970 00:00:00 GMT; HttpOnly; Secure; SameSite=Strict

Logs

No response

System Info

System:
    OS: macOS 12.1
    CPU: (8) arm64 Apple M1
    Memory: 1.71 GB / 16.00 GB
    Shell: 3.2.57 - /bin/bash
  Binaries:
    Node: 17.4.0 - /opt/homebrew/bin/node
    npm: 8.3.1 - /opt/homebrew/bin/npm
  Browsers:
    Chrome: 97.0.4692.99
    Safari: 15.2
  npmPackages:
    @sveltejs/adapter-auto: next => 1.0.0-next.13 
    @sveltejs/kit: next => 1.0.0-next.235 
    svelte: ^3.44.0 => 3.46.2

Severity

blocking an upgrade

Additional Information

No response

The text was updated successfully, but these errors were encountered:

kamholz · 2022-01-20T20:23:49Z

It looks like this issue goes all the way to the Headers object in the Fetch API: https://stackoverflow.com/questions/63204093/how-to-get-set-multiple-set-cookie-headers-using-fetch-api.

Is there any prospect for fixing this relatively soon, or is it going to be difficult to find a workaround? It breaks the JWT auth flow I've been using (and that I suspect various other people are), so I have to hold off updating my app until there's a fix.

dodiameer · 2022-01-20T21:01:21Z

I haven't tested this, but I believe you can use this as a workaround:

const headers = new Headers()
headers.append("Set-Cookie", cookie1)
headers.append("Set-Cookie", cookie2)
return {
  status: 200,
  headers
}

Headers.append() will add a value to a header instead of overwriting it according to MDN. Because you can return a Headers object in endpoints now with #3384 this should work

kamholz · 2022-01-20T21:04:12Z

No, that doesn't work because of the issue with the Headers API. The linked Stack Overflow issue explains it.

kamholz · 2022-01-21T04:24:11Z

There should be a way to fix this server-side by using the headers.raw() method from node-fetch(which is what provides Headers on the server). Something like this:

if (headers.has('set-cookie')) {
  const setCookieHeaders = headers.raw()['set-cookie'];
  ...
}

isaacfranco · 2022-01-21T05:46:39Z

There should be a way to fix this server-side by using the headers.raw() method from node-fetch(which is what provides Headers on the server). Something like this:
if (headers.has('set-cookie')) {
  const setCookieHeaders = headers.raw()['set-cookie'];
  ...
}

It should be.

I tracked down the problem on the kit source code, and found that the 'set-cookit' header (when is a Array) is flattened to a single string inside the setResponse function (packages/kit/src/node.js):

res.writeHead(response.status, Object.fromEntries(response.headers));

That Object.fromEntries(response.headers) creates a single 'set-cookies', with all cookies together, separated by a comma.

What I do not understand yet, is that .raw() is not available on the response object inside that function. That would be a quick fix.

That's the first time that I opened SvelteKit source, so i'm sorry to be a little confuse....

I will try to fix that as a exercise :)

kamholz · 2022-01-21T06:44:08Z

This seems to get the job done:

res.writeHead(response.status, getHeaders(response.headers));

function getHeaders(headers) {
  const obj = Object.fromEntries(headers);
  if (headers.has('set-cookie')) {
    obj['set-cookie'] = headers.raw()['set-cookie'];
  }
  return obj;
}

If we want to not overwrite the set-cookie key as above, which feels ugly, then it could be something like this (a lot more code though):

function getHeaders(headers) {
  if (headers.has('set-cookie')) {
    const obj = {};
    for (const [header, value] of headers.entries()) {
      obj[header] = header === 'set-cookie'
        ? headers.raw()[header]
        : value;
    }
    return obj;
  } else {
    return Object.fromEntries(headers);
  }
}

We can't simply replace Object.fromEntries(headers) with headers.raw() because we only want to return array values for Set-Cookie, not for Accept or other headers where it's possible to merge multiple values into a single string.

isaacfranco · 2022-01-21T07:06:59Z

I almost did the same thing...

/** @type {import('@sveltejs/kit/node').GetHeaders} */
export function getHeaders(response) {

	let headers;
	if (response.headers.has('set-cookie')) {
		const cookieArray = response.headers.get('set-cookie')?.split(', ') || [];
		headers = Object.assign(Object.fromEntries(response.headers), {'set-cookie': cookieArray});
	} else {
		headers = Object.fromEntries(response.headers);
	}	
	return headers;
}

.....

/** @type {import('@sveltejs/kit/node').SetResponse} */
export async function setResponse(res, response) {
	
	res.writeHead(response.status, getHeaders(response));

	if (response.body instanceof Readable) {
		response.body.pipe(res);
	} else {
		if (response.body) {
			res.write(await response.arrayBuffer());
		}

		res.end();
	}
}

And... on packagets/kit/src/core/preview/index.js:

import { getRawBody, getHeaders } from '../../node.js';
.....
res.writeHead(rendered.status, getHeaders(rendered));

isaacfranco · 2022-01-21T07:12:24Z

This seems to get the job done:
res.writeHead(response.status, getHeaders(response.headers));

function getHeaders(headers) {
  const obj = Object.fromEntries(headers);
  if (headers.has('set-cookie')) {
    obj['set-cookie'] = headers.raw()['set-cookie'];
  }
  return obj;
}
If we want to not overwrite the set-cookie key as above, which feels ugly, then it could be something like this (a lot more code though):
function getHeaders(headers) {
  if (headers.has('set-cookie')) {
    const obj = {};
    for (const [header, value] of headers.entries()) {
      obj[header] = header === 'set-cookie'
        ? headers.raw()[header]
        : value;
    }
    return obj;
  } else {
    return Object.fromEntries(headers);
  }
}
We can't simply replace Object.fromEntries(headers) with headers.raw() because we only want to return array values for Set-Cookie, not for Accept or other headers where it's possible to merge multiple values into a single string.

The problem that i had here with that approach, was that .raw() don´t exist on the response.header object....

export async function setResponse(res, response) {
	// testing raw()...
	response.headers.raw()['set-cookie']
ERROR => 
Property 'raw' does not exist on type 'Headers'.ts(2339)

Thats why I had to use split... ugly as hell.

isaacfranco · 2022-01-21T07:15:31Z

It worked, anyway... but I feel that there is a much cleaner way to solve this bug.

JeanJPNM · 2022-01-21T12:47:39Z

Using split doesn't work if one of the cookie strings contains a colon inside itself.

isaacfranco · 2022-01-21T18:01:15Z

Using split doesn't work if one of the cookie strings contains a colon inside itself.

Definitely. Using split is not just ugly, it's not correct. Using raw() worked now. I confess that I don't remember what I was doing wrong.

I created a helper function on utils/http.js (don´t know if it´s the best place)

/** @param {Partial<import('@sveltejs/kit/install-fetch').Response>} response */
export function to_out_headers(response) {
	if (!response?.headers) {
		return new Headers();
	};
	
	if (response.headers.has('set-cookie')) {
		return Object.assign(Object.fromEntries(response.headers), {'set-cookie': response.headers.raw()['set-cookie']});
	} else {
		return Object.fromEntries(response.headers);
	}
}

There are two places with this bug, as far as I know, dev and preview. I'm using this helper on both and now it looks like it's gone.

Any coments on this @JeanJPNM @kamholz?

JeanJPNM · 2022-01-21T18:09:15Z

Looks good to me.

Rich-Harris · 2022-01-21T18:32:10Z

Just recapping parallel discussion happening in Discord:

it looks like anything that uses __fetch_polyfill (which includes dev, preview, adapter-node, adapter-netlify, adapter-vercel) will need to use the .raw() trick
other adapters can't use that solution. but they presumably solve it in other ways. For example Cloudflare Workers implements a non-standard Headers.prototype.getAll method, so those environments should be unaffected
there has been discussion around standardising this behaviour, but the people responsible for those standards don't consider non-browser use cases worthwhile

kamholz · 2022-01-21T18:48:16Z

Thanks for the update @Rich-Harris and glad that the team is thinking through this! It's weird that getAll was removed from the spec given that it would solve this in a standard way for everyone, but I guess they just weren't concerned with use cases outside the browser. node-fetch actually implements getAll, but it isn't documented -- you're supposed to use .raw().

I wonder if this is the only complication you'll run into with using the Fetch API?

Rich-Harris · 2022-01-21T19:22:53Z

Fingers crossed. At the very least we'd be in good company, since more platforms and frameworks are moving in this direction

…veltejs#3460 * [fix] can no longer return multiple set-cookie headers from endpoints

* [fix] can no longer return multiple set-cookie headers from endpoints #3460 * [fix] can no longer return multiple set-cookie headers from endpoints * adding changeset * simplify * lint * simplify * tidy up and simplify test * tweak changeset Co-authored-by: Rich Harris <[email protected]>

chosecz · 2022-01-24T12:05:24Z

After that fix multiple cookies works only in dev (npm run dev) or preview mode (npm run preview), but when I run it from build with node adapter (node build/index.js), then it is again just one long string with comma separated.

"@sveltejs/kit": "^1.0.0-next.240"
"@sveltejs/adapter-node": "^1.0.0-next.66"
Node version 16.13.2

Rich-Harris · 2022-01-24T16:05:11Z

I think that might be because I forgot to add changesets for adapter-node/netlify, meaning they didn't get rebuilt with the fix. Releasing new versions now

Rich-Harris · 2022-01-27T04:03:26Z

closing this as it appears to be fixed

akiarostami · 2022-01-31T22:27:35Z

This works in endpoints, but unfortunately it still doesn't work for in the hooks.

export const handle = async ({ event, resolve }) => {
	let response = await resolve(event);
	response.headers.set('set-cookie', setCookies); // setCookies is a multi-element array
	return response;
}

Only the first item in the setCookies array gets assigned.

Should we re-open this issue?

JeanJPNM · 2022-01-31T22:41:52Z

Correct me if I'm wrong, but as far as I know you are not supposed to pass an array directly into Headers.set, in your case the correct approach would be to iterate over the items of the array and append them to the set-cookie header like this:

for(const cookie of setCookies) {
  response.headers.append("set-cookie", cookie)
}

akiarostami · 2022-01-31T22:46:59Z

Thank you @JeanJPNM! I appreciate you pointing this out.

@kamholz earlier said "that doesn't work because of the issue with the Headers API." though. So can we use multiple appends with headers?

fabienc974 · 2022-02-01T13:16:36Z

Hi, I have the same issue with multiple set-cookie. Before last framework update (234), everything worked well.
I've created a post on stackoverflow.
Even if I append the cookies in the headers, only the first one is added to the headers.

Here is the link on stack : Link on stack

I'm blocked and can't move on anymore on my App.

Thanks for your help.

Fabien

isaacfranco · 2022-02-01T14:43:13Z

Thank you @JeanJPNM! I appreciate you pointing this out.

@kamholz earlier said "that doesn't work because of the issue with the Headers API." though. So can we use multiple appends with headers?

Yes. You could use multiple appends.

version: 1.0.0-next.252

export async function handle({ event, resolve }) {
    const response = await resolve(event);

    response.headers.append('Set-Cookie', 'handleFoo=foo');
    response.headers.append('Set-Cookie', 'handleBar=bar; HttpOnly=true');

    return response;
}

fabienc974 · 2022-02-03T23:22:45Z

Hi,
You are adding the multiple set-cookie from within the handle function in the hooks.js file.
I want to set multiple cookies in my login.json.js file and use the handle function to set some locals.
I've tried passing my values in the body (in the login.json.js response), but when I try to access the body datas in the handle function, it says error [...] body = await request.json() [...]
Request is not available anymore.
How can I access the body datas ?
I think if I can access these datas, I can set the multiple cookies from the handle function.
Thanks for your help.

JeanJPNM · 2022-02-03T23:33:08Z

Hi, could you please send the error logs? They give important data about the error and make the resolution process faster.

fabienc974 · 2022-02-04T16:59:15Z

Hi,
If I add const body =. await request.json() in the handle function, I get an infinite loop with this message
__vite_ssr_import_1__.default.json is not a function TypeError: __vite_ssr_import_1__.default.json is not a function at Object.handle (//Users/fab/Documents/MyProject/sites/svelte-frontend/src/hooks:78:29) at respond (file:///Users/fab/Documents/MyProject/sites/svelte-frontend/.svelte-kit/runtime/server/index.js:2497:40) at fetch (file:///Users/fab/Documents/MyProject/sites/svelte-frontend/.svelte-kit/runtime/server/index.js:1657:23) at load (__layout.svelte:3:27) at load_node (file:///Users/fab/Documents/MyProject/sites/svelte-frontend/.svelte-kit/runtime/server/index.js:1778:30) at respond_with_error (file:///Users/fab/Documents/MyProject/sites/svelte-frontend/.svelte-kit/runtime/server/index.js:1965:10) at async respond (file:///Users/fab/Documents/MyProject/sites/svelte-frontend/.svelte-kit/runtime/server/index.js:2645:11) at async fetch (file:///Users/fab/Documents/MyProject/sites/svelte-frontend/.svelte-kit/runtime/server/index.js:1657:17) at async load (__layout.svelte:3:22) at async load_node (file:///Users/fab/Documents/MyProject/sites/svelte-frontend/.svelte-kit/runtime/server/index.js:1778:12)

Hope it can help !

fabienc974 · 2022-02-08T08:01:06Z

Hi,
Did the log help you ?
Thanks for your help.
Fabien

JeanJPNM · 2022-02-08T19:25:09Z

Sorry for taking so long, I cannot reproduce your issue, have you tried to reproduce the error inside another endpoint?

Noting that it is recommended to send a file with the relevant parts of the code related to the error when facing this kind of situation.

fabienc974 · 2022-02-09T16:06:11Z

No I did not tried to access it from another end point.
I can give you access to my repo if you need !

JeanJPNM · 2022-02-09T17:12:37Z

Yes, I would like to see the repo.

fabienc974 · 2022-02-10T11:11:22Z

Invite sent !
Please let me know if can't access it

edit:

You'll find the main svelte directory here : Lionjar / sites / svelte-frontend
And for the backend, I use strapi, Lionjar / backend-svelte-strapi

JeanJPNM · 2022-02-11T01:51:40Z

I think the issue happened because you were using the spread operator ({ ...obj, foo: 1}) on the response object. Svelte kit currently uses Response objects to represent responses so using the spread operator on them may cause undesired behavior.

To solve the issue I'd recommend modifying the response headers as bellow:

	if (user || loggingOut) {
		response.headers.append("Set-Cookie", setCookieValue)
		response.headers.append("Set-Cookie", setJwtCookieValue)
		response.headers.append("Set-Cookie", setCompIdCookieValue)
	}
	return response

benmccann added the bug Something isn't working label Jan 20, 2022

benmccann added this to the 1.0 milestone Jan 20, 2022

isaacfranco added a commit to isaacfranco/kit that referenced this issue Jan 22, 2022

[fix] can no longer return multiple set-cookie headers from endpoints s…

d5fde36

…veltejs#3460 * [fix] can no longer return multiple set-cookie headers from endpoints

isaacfranco mentioned this issue Jan 22, 2022

Multiple cookies are not set on endpoints #3502

Merged

5 tasks

Rich-Harris closed this as completed Jan 27, 2022

benmccann mentioned this issue Jan 29, 2022

multiple -set-cookie doesnt work sveltejs/realworld#102

Closed

ghalle mentioned this issue Sep 30, 2022

Setting multiple Set-Cookie headers is not handled properly solidjs/solid-start#293

Closed

simonpannek mentioned this issue Feb 26, 2023

Setting multiple Set-Cookie headers using the vercel adapter #9221

Open

anilmagzter mentioned this issue Apr 17, 2024

Setting multiple cookie in same request not working jill64/sveltekit-adapter-aws#604

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

can no longer return multiple `set-cookie` headers from endpoints #3460

can no longer return multiple `set-cookie` headers from endpoints #3460

kamholz commented Jan 20, 2022

kamholz commented Jan 20, 2022

dodiameer commented Jan 20, 2022

kamholz commented Jan 20, 2022

kamholz commented Jan 21, 2022

isaacfranco commented Jan 21, 2022 •

edited

Loading

kamholz commented Jan 21, 2022

isaacfranco commented Jan 21, 2022

isaacfranco commented Jan 21, 2022 •

edited

Loading

isaacfranco commented Jan 21, 2022 •

edited

Loading

JeanJPNM commented Jan 21, 2022

isaacfranco commented Jan 21, 2022

JeanJPNM commented Jan 21, 2022

Rich-Harris commented Jan 21, 2022 •

edited

Loading

kamholz commented Jan 21, 2022 •

edited

Loading

Rich-Harris commented Jan 21, 2022

chosecz commented Jan 24, 2022 •

edited

Loading

Rich-Harris commented Jan 24, 2022

Rich-Harris commented Jan 27, 2022

akiarostami commented Jan 31, 2022

JeanJPNM commented Jan 31, 2022

akiarostami commented Jan 31, 2022

fabienc974 commented Feb 1, 2022

isaacfranco commented Feb 1, 2022

fabienc974 commented Feb 3, 2022

JeanJPNM commented Feb 3, 2022 •

edited

Loading

fabienc974 commented Feb 4, 2022

fabienc974 commented Feb 8, 2022

JeanJPNM commented Feb 8, 2022 •

edited

Loading

fabienc974 commented Feb 9, 2022

JeanJPNM commented Feb 9, 2022

fabienc974 commented Feb 10, 2022 •

edited

Loading

JeanJPNM commented Feb 11, 2022

can no longer return multiple set-cookie headers from endpoints #3460

can no longer return multiple set-cookie headers from endpoints #3460

Comments

kamholz commented Jan 20, 2022

Describe the bug

Reproduction

Logs

System Info

Severity

Additional Information

kamholz commented Jan 20, 2022

dodiameer commented Jan 20, 2022

kamholz commented Jan 20, 2022

kamholz commented Jan 21, 2022

isaacfranco commented Jan 21, 2022 • edited Loading

kamholz commented Jan 21, 2022

isaacfranco commented Jan 21, 2022

isaacfranco commented Jan 21, 2022 • edited Loading

isaacfranco commented Jan 21, 2022 • edited Loading

JeanJPNM commented Jan 21, 2022

isaacfranco commented Jan 21, 2022

JeanJPNM commented Jan 21, 2022

Rich-Harris commented Jan 21, 2022 • edited Loading

kamholz commented Jan 21, 2022 • edited Loading

Rich-Harris commented Jan 21, 2022

chosecz commented Jan 24, 2022 • edited Loading

Rich-Harris commented Jan 24, 2022

Rich-Harris commented Jan 27, 2022

akiarostami commented Jan 31, 2022

JeanJPNM commented Jan 31, 2022

akiarostami commented Jan 31, 2022

fabienc974 commented Feb 1, 2022

isaacfranco commented Feb 1, 2022

fabienc974 commented Feb 3, 2022

JeanJPNM commented Feb 3, 2022 • edited Loading

fabienc974 commented Feb 4, 2022

fabienc974 commented Feb 8, 2022

JeanJPNM commented Feb 8, 2022 • edited Loading

fabienc974 commented Feb 9, 2022

JeanJPNM commented Feb 9, 2022

fabienc974 commented Feb 10, 2022 • edited Loading

JeanJPNM commented Feb 11, 2022

can no longer return multiple `set-cookie` headers from endpoints #3460

can no longer return multiple `set-cookie` headers from endpoints #3460

isaacfranco commented Jan 21, 2022 •

edited

Loading

isaacfranco commented Jan 21, 2022 •

edited

Loading

isaacfranco commented Jan 21, 2022 •

edited

Loading

Rich-Harris commented Jan 21, 2022 •

edited

Loading

kamholz commented Jan 21, 2022 •

edited

Loading

chosecz commented Jan 24, 2022 •

edited

Loading

JeanJPNM commented Feb 3, 2022 •

edited

Loading

JeanJPNM commented Feb 8, 2022 •

edited

Loading

fabienc974 commented Feb 10, 2022 •

edited

Loading