Add a cookies interface to RequestEvent and LoadServerEvent

[Rich-Harris]

Describe the problem

In #5748, we introduced the setHeaders API:

export function load({ setHeaders }) {
  setHeaders({
    'set-cookie': 'answer=42'
  });
}

Recent explorations (see the 'cookie handling' section in particular) suggest the API falls short. We need a way to set a cookie inside an action that can be read during the subsequent load, and while we could make it work with setHeaders it would involve some very unexpected behaviour (we would basically need to intercept request.headers.get('cookie') and sub in a value that included the set-cookie, which is a bad amount of magic).

Describe the proposed solution

We add a new cookies interface:

// src/routes/login/+page.server.js
export const actions = {
  default: ({ fields, cookies }) => {
    const user = await db.getUserFromEmail(fields.get('email');

    // ...some logic happens...

    cookies.set('sessionid', await db.createSession(user), {
        maxAge: 30 * 24 * 60 * 60 * 1000 // stay logged in for 30 days
    });
  }
};

export const actions is described in #5875 (comment)

// src/routes/+layout.server.js
export async function load({ cookies }) {
  // this value could come from `request.headers.get('cookie')` or the previous `cookies.set` call
  const sessionid = cookies.get('sessionid');
  const user = await db.getUserFromSessionId(sessionid);
  return { user };
}

There is an experimental standard CookieStore interface, but I don't think we should use it. It's designed for use in browsers, which means all the methods are asynchronous — needlessly, in our case — and there's a change event that doesn't really make sense in our context. More important, it doesn't expose an httpOnly option (because that wouldn't make sense for client-side JavaScript), whereas I think httpOnly is the only sensible default.

The proposed API:

interface Cookies {
  delete(name: string): void;

  get(name: string, opts: {
    decode: (input: string) => string
  }): string;

  set(name: string, value: string, opts?: {
    expires?: Date;
    maxAge?: number;
    domain?: string;
    path?: string;
    secure?: boolean;
    httpOnly: boolean;
    sameSite: 'strict' | 'lax' | 'none'; // can only be `none` with `secure: true`,
    encode: (input: string) => string
  }); void;
}

If you're familiar with the cookie package, this API will seem very familiar — it's what we'd use under the hood.

Alternatives considered

Relying on people populating event.locals in actions, which feels unnecessarily restrictive.

Intercepting (and changing) request.headers.get('cookie'), which feels like too much magic.

Importance

would make my life easier

Additional Information

No response

[Rich-Harris]

Might be nice to throw in sign and unsign methods that work with WebCrypto (cookie-signature works with Node's built-in crypto and is therefore unsuitable for many SvelteKit apps)

[Rich-Harris]

Marking as a breaking change since we'd probably want to prevent setHeaders({ 'set-cookie': [...} }) if we did this

[dummdidumm]

I'm split on this. It adds a new API for one specific use case - I don't see others outside this. There was the idea in another issue to be able to pass data back into load, maybe something like this can be used here, too, so we can solve both these use cases with one API.
Regardless, I think making setHeaders throw on cookie feels a little weird - why restrict people there? Using it is fine outside of the actions use case.

[Rich-Harris]

It is a specific use case, but it's one that isn't currently served. load needs to be able to read cookies that were set in an action, and that's not currently possible.

setHeaders treating set-cookie differently to other headers wouldn't be all that weird — it already treats them differently:

set-cookie is a unique header in that its value always has to be treated by JavaScript as an array, which is a long-running source of confusion — see whatwg/fetch#973 and e.g. this section of the Node docs:

It's handled very differently at an implementation level. So I don't think we'd be going rogue by treating it differently at an API level (or rather, more differently than we already currently do).

The other argument in favour of this is that every non-trivial app is going to need to parse cookies at one point, and this...

// src/routes/+layout.server.js
export async function load({ cookies }) {
  const user = await db.getUserFromSessionId(cookies.get('sessionid'));
  return { user };
}

...just seems a lot nicer than this:

import * as cookie from 'cookie';

// src/routes/+layout.server.js
export async function load({ request }) {
  const cookies = cookie.parse(request.headers.get('cookie') ?? '');
  const user = await db.getUserFromSessionId(cookies.sessionid);
  return { user };
}

[cdcarson]

I like this. Thoughts:

Depending on the browser Secure cookies aren't always sent to localhost over http://. I think Safari is the only major malefactor at this point, but I know Chrome was behaving that way within living memory (maybe 8 months ago.)

Sign/Verify -- yes, please. It might require changes to some people's code (e.g. I use JWTs since they are easy to sign.) But that's fine if it means I don't have to think about whether or not to sign.

It'd be nice to have the API catch the case where a cookie is over the 4K limit after serialization encoding and signing. This error is relatively easy to do and hard to debug -- the cookie is sent but the browser (at least Chrome) silently ignores it

[Rich-Harris]

It'll just be set/get/delete for now, since we're focused on getting all the breaking changes out of the way, and sign/unsign and the 4k limit stuff can be added non-breakingly later. But yes to all of that

[sourcegr]

Regardless of whether this proposition passes or not, I believe someone should still be able to setHeaders({ 'set-cookie': [...} }).

We should not prevent the devs to set any headers they want.

Granted, it creates confusion and it would be a bad practise; so we could issue a warning on dev mode when a set-cookie key is used, and be done with the breaking change situation.