Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[cloudflare adapter] Can't set multiple cookies #2305

Closed
half2me opened this issue Aug 27, 2021 · 7 comments · Fixed by #2313
Closed

[cloudflare adapter] Can't set multiple cookies #2305

half2me opened this issue Aug 27, 2021 · 7 comments · Fixed by #2313

Comments

@half2me
Copy link
Contributor

half2me commented Aug 27, 2021

Describe the bug

When setting multiple cookies from hooks in the handle function, I have the following code:

return {
  // rest of response
  headers: {
    'set-cookie': ['cookie1', 'cookie2'],
  },
}

When using the node adapter, it correctly uses separate entries in the returned headers for each cookie.

set-cookie: cookie1
set-cookie: cookie2

However when running on cloudflare workers, with the cloudflare adapter, the cookies are merged into a single entry:

set-cookie: cookie1,cookie2

This results in only one cookie being set.

Reproduction

Return multiple cookies and use the @sveltejs/adapter-cloudflare-workers adapter

Logs

No response

System Info

System:
    OS: macOS 11.5.2
    CPU: (12) x64 Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
    Memory: 407.40 MB / 16.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 16.7.0 - /usr/local/bin/node
    npm: 7.21.0 - /usr/local/bin/npm
  Browsers:
    Chrome: 92.0.4515.159
    Safari: 14.1.2
  npmPackages:
    @sveltejs/adapter-cloudflare-workers: ^1.0.0-next.19 => 1.0.0-next.19 
    @sveltejs/adapter-node: ^1.0.0-next.43 => 1.0.0-next.43 
    @sveltejs/kit: next => 1.0.0-next.158 
    svelte: ^3.34.0 => 3.42.3

Severity

blocking all usage of SvelteKit

Additional Information

No response

@half2me half2me changed the title [cloudflare adapter] [cloudflare adapter] Can't set multiple cookies Aug 27, 2021
@benmccann
Copy link
Member

I'm not sure that the Cloudflare adapter changes this. If it did, then it'd probably happen here:

headers: Object.fromEntries(request.headers),

I guess Cloudflare itself does it and the solution would be to make SvelteKit split apart the header into two when setting cookies

@JeanJPNM
Copy link
Contributor

Maybe this can work:

/**
 * @param {Record<string, string | string[]>} headers
 */
function makeHeaders(headers) {
	const result = new Headers();
	for (const header in headers) {
		const value = headers[header];
		if (typeof value === 'string') {
			result.set(header, value);
			continue;
		}
		for (const sub of value) {
			result.append(header, sub);
		}
	}
	return result;
}

@half2me
Copy link
Contributor Author

half2me commented Aug 28, 2021

Thanks @JeanJPNM this works perfectly. I'll be using this as a workaround for now. Reckon this behaviour could be made default?

@JeanJPNM
Copy link
Contributor

Ok, I will make a pull request

@benmccann
Copy link
Member

Won't result.append(header, sub); make it so that there's just a single header for Set-Cookie? I thought that's what @half2me was complaining about in the issue description

@JeanJPNM
Copy link
Contributor

Yes, it technically does the same thing, but for some reason it works

@benmccann
Copy link
Member

I guess it's because headers takes a bytestring and not a string[]: https://developer.mozilla.org/en-US/docs/Web/API/Headers/Headers#parameters

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants