Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

more robust cookies #460

Merged
merged 1 commit into from
Oct 2, 2018
Merged

more robust cookies #460

merged 1 commit into from
Oct 2, 2018

Conversation

Rich-Harris
Copy link
Member

Man, cookies suck. Took me a long time to figure the following out:

  • The cookies that you send from server to client via a Set-Cookie header contain the key and value (foo=bar), but also a number of attributes (which could have values, like Max-Age=3600, but needn't, like HttpOnly). These are separated by a semi-colon. The order of attributes doesn't matter, but the key/value must go first
  • You can set multiple cookies in a single response, by having multiple Set-Cookie headers (i.e. you can't set multiple cookies in a single header)
  • You can't call response.setHeader('Set-Cookie', '...') multiple times. Instead you must pass an array as the second argument (response.setHeader('Set-Cookie', ['a=1', 'b=2'])). No error or warning will be given if you get this wrong
  • If you need to get the header back later (which Sapper does), res.getHeader('Set-Content') could therefore return either a string or an array of strings
  • The cookies that the client then sends back to the server only include the key/value. In a masterstroke of totally, 100% intuitive design, they come in the form of a single semi-colon-separated string, which looks ldentical to the Set-Cookie header except that it means something completely different (the semi-colon now separates cookies, not attributes)
  • If you're using the cookie library, cookie.parse works with the cookies the client sends to the server but not the other way around

Anyway, Sapper was behaving incorrectly because I didn't understand all that until now. This PR fixes it.

@Rich-Harris Rich-Harris merged commit 954bcba into master Oct 2, 2018
@Rich-Harris Rich-Harris deleted the cookies branch October 2, 2018 02:58
@maxmilton
Copy link

Thank you for sharing all this! ❤️

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

Successfully merging this pull request may close these issues.

2 participants