cookie stuff

[Rich-Harris]

#6648, plus changes that for some reason I couldn't push to the PR

[changeset-bot]

🦋 Changeset detected

Latest commit: 312c3b8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@sveltejs/kit	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[Rich-Harris]

@cdcarson what's the rationale for having a default path value? For httpOnly and secure the defaults add security, and sameSite makes sense because it's already a default, and specifying it explicitly squelches console warnings.

But defaulting path to / makes things less secure. Even though it's behaviour you often want, should it be the default?

[cdcarson]

Woops -- responded in the other PR: #6648 (comment). Pasted:

@Rich-Harris Pretty sure it is necessary. See #6650. Basically a cookie set in a request to /sign-in will have that path. A subsequent request to /profile won't send that cookie.

The only downside of having the default is if the site base path is /foo. We can probably address this in the docs.

[cdcarson]

But defaulting path to / makes things less secure. Even though it's behaviour you often want, should it be the default?

Ack. You're right.

We're sorta painted into a corner here. As I understand it, we need the cookies API because cookies may be written and read in the same request, in different functions/files, especially with actions. Even though we're using cookie underneath folks will post their cookie issues to SvelteKit. So, it makes some sense to provide defaults that take care of the most common cases.

I'm fine with removing the default path, but it should be documented in Kit.

Or, maybe we throw if path is not explicitly set.

[Rich-Harris]

As I understand it, we need the cookies API because cookies may be written and read in the same request, in different functions/files, especially with actions

We're not really any worse off because of cookies; the same is true if you add a set-cookie header any other way. The only difference is that we now have the opportunity to set a default, and it's sorely tempting to take it.

To tl;dr a conversation among the maintainers just now: 👍 to removing the default path. Throwing an error feels heavy-handed and would be a breaking change, but if people do end up getting confused by this stuff then a dev-time warning could be appropriate in future.

Agree re docs, will add something brief to this PR. We needed docs anyway because of the httpOnly default.

[cdcarson]

We're not really any worse off because of cookies; the same is true if you add a set-cookie header any other way.

I guess I was harking back to the old days, doing my own form enhancement. I don't remember having to read a cookie after writing it -- but I was probably doing a whole bunch of goofy inefficient stuff instead.

Agree throwing would be bad. A warning might be nice -- it's an easy mistake.