Make path a required option when setting/deleting cookies

[Rich-Harris]

Describe the problem

See discussion in #9130. I'm not sure there's ever a good reason not to specify a path — it's almost always an annoying mistake, that we may or may not catch in time.

Describe the proposed solution

Make path required.

Alternatives considered

No response

Importance

nice to have

Additional Information

No response

[dummdidumm]

Still not sure this is the right move. None of the other metaframeworks do this AFAIK, and the "default path" logic works without bugs now. If we were to introduce this, it means that people need to stringify the path they're currently on, and when it's not / then they're potentially introducing a bug when the route changes but they don't update the string in the cookie, etc etc.

[fehnomenal]

What about defaulting the path to ${base}/? This is probably the most common case which I have to set in all of my projects.

[dummdidumm]

It probably is, but changing it now feels too risky - people could rely on the current behavior (more scoped cookies) and upgrade, not reading the changelog carefully, introducing a security hole.

[dummdidumm]

We decided to do this with a convenience shortcut for the current default behavior, something like .. meaning "the current default path"

[Rich-Harris]

Trying to figure out how to indicate 'use the default path behaviour'. Our implementation...

kit/packages/kit/src/runtime/server/cookie.js

Lines 27 to 28 in 6818dd4

	// Emulate browser-behavior: if the cookie is set at '/foo/bar', its path is '/foo'
	const default_path = normalized_url.split('/').slice(0, -1).join('/') || '/';

...matches RFC 6265, but neither '' nor '.' nor '..' quite matches that:

normalized_url = '/foo/bar';

normalized_url.split('/').slice(0, -1).join('/') || '/'; // "/foo"
new URL('', 'https://example.com/foo/bar').pathname; // "/foo/bar"
new URL('.', 'https://example.com/foo/bar').pathname; // "/foo/"
new URL('..', 'https://example.com/foo/bar').pathname; // "/"

Having said that, I'm not sure the default behaviour is particularly useful. I think it's probably fine to just say 'you have to specify path but if you want to specify a relative path that's cool too'. Thoughts?

[dummdidumm]

What would it mean specifying a relative path? Relative to the current location, so that ./ means "the route I'm on"? In which way does it take trailing slashes into account?

[benmccann]

I'm not sure the default behaviour is particularly useful.

I would agree. I don't really understand the discussion around default path behavior because the default Set-Cookie behavior is that path is optional. It seems like for some reason we're talking about taking the default of a different browser-only cookie API and apply it to this server-only cookie API, which I don't really think we should be doing.

I think it's probably fine to just say 'you have to specify path but if you want to specify a relative path that's cool too'. Thoughts?

I think it's probably fine to say you have to specify the path. I think that the user can specify / to get the same behavior as omitting the path?

if you want to specify a relative path that's cool too

That could be okay, but I'd be careful to think through, test, and maybe document edge cases with trailing slashes. I know people have gotten tripped up on the browser when relative paths behave differently for things like /foo/bar and /foo/bar/ and so hopefully we don't make mistakes there or introduce an API people are likely to trip over

[dummdidumm]

Re "discussion about default path behavior": the idea was to provide a shortcut so that people get the same cookie path they would get today when omitting the path, like "pass in . to trigger the default path logic"

[benmccann]

Yeah, I just don't get why today's behavior ever existed

[Rich-Harris]

We're not talking about a browser API at all, we're talking about whether — in a world where we require you to provide a path, to avoid a common source of bugs (but without defaulting to path: '/' which is insecure) — it makes sense to provide a way to say 'I want the default browser behaviour you get when you return a Set-Cookie header without specifying a path'.

My vote is 'no', because the default browser behaviour seems dumb.

What would it mean specifying a relative path?

Specifying '' would mean 'the current pathname'; '.' would mean 'the current directory', and so on. In fact this is how it works today for '.' (if your Set-Cookie header includes Path=./relative then the browser will resolve it against the current path) — the only change is that '' is currently treated as 'unspecified', i.e. the default behaviour.

Trailing slashes work exactly as you'd expect:

new URL('', 'https://example.com/foo/').pathname; // '/foo/'
new URL('.', 'https://example.com/foo/').pathname; // '/foo/'
new URL('..', 'https://example.com/foo/').pathname; // '/'

new URL('', 'https://example.com/foo/bar').pathname; // '/foo/bar'
new URL('.', 'https://example.com/foo/bar').pathname; // '/foo/'
new URL('..', 'https://example.com/foo/bar').pathname; // '/'

[dummdidumm]

Because it's what the browser does if you don't set a path

[Rich-Harris]

(One thing to note: our resolve(base, path) internal helper currently misbehaves if path is the empty string. We need to fix that behaviour at the same time as we fix the cookies API. Working on it)

[dummdidumm]

One thing that speaks for having some string reserved for the current "omit path" behavior is that we can automigrate the user's code

[Rich-Harris]

Feels a bit tail-wagging-the-dog

[benmccann]

Yeah, I'd not make an effort to keep the current behavior. It's better to make the user do some work to migrate so that we all have nicer APIs

[dummdidumm]

But what if you do want the current default behavior? Sounds weird to not have an ergonomic option for that. Absolute paths are prone to get out of sync

[benmccann]

You can just use .. and strip the trailing slash (assuming you have the default trailingSlash setting).

If we really want to support the old behavior I suppose we could introduce an additional shortcut such as ... or something

[Rich-Harris]

The default behaviour isn't hard to implement yourself if you really need it. The question is, why would you ever need it? What's a scenario in which it's helpful?

[dummdidumm]

Ok asked differently: If I'm on admin/login and I want to set the cookie to the admin scope, how do I do that without hardcoding /admin which may get out of sync? How would a relative path look like to do this? We should explain this in the docs a little.

[benmccann]

I just saw #11237 was merged. One suggestion on that - I think we should add the relative path support in 1.x. Otherwise it's weird to deprecate the old behavior, but not yet have available the new functionality that we want users to migrate to

[Rich-Harris]

I want to set the cookie to the admin scope

'.' would set it to /admin/. To set it to /admin then yeah, you would need to hardcode it. But that's what I'd suggest anyway — otherwise if you move the login route to /admin/auth/login then you're just as hosed.

If we're worried about things getting out of date (or handling use cases like translated routes) then we should probably just build tools to accommodate those scenarios (like path: validate('/admin') where validate has a list of valid routes; ideally it would also be type safe)

I think we should add the relative path support in 1.x

It's already supported, except for '' (which is treated as 'unspecified') — that's just how cookies work

[Rich-Harris]

I suppose the other way to tackle this is to make path required but continue making '' mean 'the default behaviour'. I just think that behaviour is a bit confusing since it doesn't work the way path resolution works in any other context (and it makes things harder in the more likely case that you want to express 'the current path, and only the current path')

[Rich-Harris]

It's already supported

Oh actually there is one caveat here — it's support insofar as the cookie path will be correct in the browser, but cookies.get(...) within the same request as the cookie is set may get confused, because it checks whether the path matches, and the path isn't resolved in the internal lookup

[benmccann]

I agree making '' keep the old behavior would be confusing

[dummdidumm]

I just checked this again and setting the path to . is equivalent to the SvelteKit 1.x behavior in all cases.

[pilcrowOnPaper]

IMO, this is the wrong move. event.cookies.set() is the lowest available API to set cookies in SvelteKit (you don't have access to response headers in actions). Making the API as close to the HTTP spec is important, and it shouldn't be the place to introduce SvelteKit specific concepts (path is required, "" means current path, "." means current dir).

Also, this breaks compatibility with every other meta-framework (including Astro, Next.js, and Unjs/Nuxt), as well as the popular cookie package. That's disappointing from a library author perspective

[Rich-Harris]

I'll be honest, I'm very surprised at the number of 👍 reactions here. It would be useful to know how many of those are because of the compatibility-with-other-frameworks thing, and how many are because people genuinely want the confusing default behaviour. (Sound off if you're one of the 21 — so far — people who reacted!) Doing the default-HTTP-spec thing is good in the absence of an alternative, but when relying on the default behaviour is a frequent source of confusion and bugs, it's the framework's job to push people towards more reliable outcomes.

I'm sympathetic to the cross-framework issue! But I'm not sure it should be decisive — if anything, I'd love to see other frameworks (and libraries that support them) adopt the same constraint. I do think it's better for users in the long run.

[pilcrowOnPaper]

I just don't think it's the right place to put it. From the API/types, I'd expect it to work exactly like how HTTP cookies would. secure is a boolean because the flag exists or doesn't. I can omit the expiration to make it a session cookie. I don't set the path option to limit it to the current path. It's the same issue in Express - its maxAge option uses milliseconds instead of seconds. It's another thing I have to check with the docs. It's another thing inconsistent with the standard. Because, for better or for worse, the previous API can be considered the de-facto standard in JS and will likely continue to be so. I don't think deviating it from it and adding SvelteKit specific concepts to patch it is worth the safe guard it provides - especially since it's not a security issue.

[Rich-Harris]

I don't set the path option to limit it to the current path

Which perfectly encapsulates why we made this change — omitting path doesn't limit the cookie to the current path — if you omit path when setting a cookie on /foo/bar, then the cookie will apply to /foo/bar but also to /foo and /foo/baz. The cases where that outcome is desirable are very rare, in my experience. If the behaviour is confusing enough that the maintainer of an auth library misstates its effects, it's probably not a very good default!

And we already deviated from those defaults — we default httpOnly to true, and secure to true unless you're developing locally — because the defaults are bad. Having opinions about these things, and nudging people towards better outcomes, is the whole point of a framework. The only difference between path and httpOnly/secure is that there's no default we can set on your behalf, so it has to be a required option.

I strongly encourage you to consider this question from the opposite angle — rather than asking SvelteKit to undo a necessary change to facilitate compatibility with cross-framework libraries, what if cross-framework libraries made path required like we do? More people will benefit that way.

[pilcrowOnPaper]

Sorry I meant to say path directory there. Anyway, I guess it's just different design philosophy. I value standardized API that's consistent with the spec over potentially safer API, and I don't want/expect the framework to do everything for me. Even if other frameworks change their API, we still have a fractured ecosystem right now - and IMO for very little reason.

[ollema]

I'll be honest, I'm very surprised at the number of 👍 reactions here. It would be useful to know how many of those are because of the compatibility-with-other-frameworks thing, and how many are because people genuinely want the confusing default behaviour

it was linked from pilcrow's twitter, which has thousands of followers: https://twitter.com/pilcrowonpaper/status/1735468834843418738.
this might be one of the reasons why there are so many 👍 reactions.