Setting multiple Set-Cookie headers using the vercel adapter

[simonpannek]

Describe the bug

It seems like it's not possible to set more than one cookie in an Vercel endpoint using the Set-Cookie header. Similar to the bug described in #3460, the cookies get concatenated, even though this will lead to most browsers only recognizing the first cookie set (According to MDN, multiple cookies should be set using multiple Set-Cookie headers instead of concatenating them in one).

The issue mentioned above already describes multiple workaround like creating a separate Headers or Response object and adding the different cookie headers using the append function, but this doesn't seem to fix this issue for the Vercel adapter. Deploying exactly the different code on other platforms like Netlify resolves this issue for me.

Reproduction

Deploying an endpoint on Vercel, and trying to set multiple cookies in the header:

	return new Response(null, {
		headers: {
			'Set-Cookie': [
				`cookie1=123; Path=/; HttpOnly; SameSite=Strict; Expires=${access_token_expires_in}}`,
				`cookie2=456; Path=/; HttpOnly; SameSite=Strict; Expires=${refresh_token_expires_in}`
			],
			Location: '/'
		},
		status: 302
	});

Logs

No response

System Info

Whatever system environment the serverless functions of Vercel are using.

Adapter version: `@sveltejs/[email protected]`

Severity

serious, but I can work around it

Additional Information

No response

[eltigerchino]

Not sure how to solve this one. The Fetch API doesn't allow setting multiple headers with the same name. They always get concatenated when using append.

[simonpannek]

Yes, I saw that as well. It's just weird to me that it's working for some platforms, and it's not for others. Maybe whatever workaround the Netlify adapter is using the Vercel adapter could implement as well, given that it's actually the adapters responsible for this behavior. I'm aware that we can't change platform internals :D

[eltigerchino]

Oh neat! Just saw how it’s done in the netlify adapter. Link
I wonder if this is something that can be implemented as part of kit / for each adapter (since the spec mentions this is the correct manner), or just as a fix for the Vercel adapter?

[Rich-Harris]

This 'works' on Vercel too, as long as you're using serverless functions, because we do this:

kit/packages/kit/src/exports/node/index.js

Lines 110 to 116 in 557422b

	if (response.headers.has('set-cookie')) {
	const header = /** @type {string} */ (response.headers.get('set-cookie'));
	const split = set_cookie_parser.splitCookiesString(header);
	// @ts-expect-error
	headers['set-cookie'] = split;
	}

(The same applies to any platform that uses the Node (req, res) => void signature. The Netlify code linked above is just the Lambda equivalent.)

Some background: Set-Cookie is an awkward header — it's the only one where you can't just concatenate multiple values (because you can't just split on ,, unlike e.g. cache-control: public, max-age=3600). Because of that, it's always needed special treatment (see the Node docs for example).

Unfortunately, the Headers API was only designed with browsers in mind, much to the dismay of people implementing web APIs in non-browser environments. In a browser, there's never a need to read Set-Cookie headers (they're removed from the response when you call fetch, for example), and so no accommodation was made for them until very recently when Headers.prototype.getSetCookie was added to the spec.

Back to the issue at hand. The behaviour we see on Netlify (and Vercel, when using serverless functions) isn't actually correct in this case. It's designed to handle this situation...

headers = new Headers();
headers.append('Set-Cookie', 'a=1');
headers.append('Set-Cookie', 'b=2');

...by splitting headers.get('Set-Cookie') back into an array. But by virtue of that, this incorrect code also 'works':

headers = new Headers();
headers.set('Set-Cookie', 'a=1, b=2');

And here's where we get to the crux of the matter: unfortunately, your code is wrong. If you're using TypeScript, it'll even tell you your code is wrong:

But web APIs are forgiving, and instead of throwing an error it will simply coerce the array to a string:

String(array) === array.join(',');

Happily, it's easy to fix your code — just create a new Headers() object and append the cookies.

So: the correct behaviour here isn't for this to work on Vercel edge functions, it's for it to fail on Vercel serverless functions and Netlify. We'll be able to make that change very soon, as undici already implemented getSetCookie; we just need to wait for a new release.

This is a semver quandary though — if people are relying on the buggy behaviour, is the fix a breaking change?