search

ring-clojure / ring

Clojure HTTP server abstraction
MIT License
3.75k stars 519 forks source link

Add :none as possible :same-site value #374

Closed Rooke closed 5 years ago

Rooke commented 5 years ago

According to https://tools.ietf.org/html/draft-ietf-httpbis-rfc6265bis-03#section-4.1 Google is proposing a change to the cookie spec that will add the value "None" to the same-site HTTP header. This adds :none as an acceptable key for the :same-site header value.

Implements issue #373:

ring-clojure/ring#373

Rooke commented 5 years ago

The Travis CI failure seems unrelated to my change (the test that I modified passed). Perhaps something to do with the recent update to Jetty?

weavejester commented 5 years ago

If you rebase against the master branch, Travis CI should be fixed now.

weavejester commented 5 years ago

Thanks for the update. Github doesn't send me a notification for changes without comments, so when you update a commit it's usually a good idea to add a comment as well 😃

This looks fine, but can you neaten up the end of the commit message, and wrap at 72 characters for the description? So something like:

Add :none as possible :same-site value

Google is proposing a change to the cookie spec that will add the value
"None" to the same-site HTTP header. This commit adds :none as an
acceptable key for the :same-site header value.

See: https://tools.ietf.org/html/draft-ietf-httpbis-rfc6265bis-03#section-4.1

Fixes #373.
Rooke commented 5 years ago

No problem, there's no rush. Note that https://www.chromestatus.com/feature/5088147346030592 says "The Stable version of Chrome 80 is targeted for enabling this feature by default". According to https://www.chromestatus.com/features/schedule Chrome 79 isn't even scheduled yet, so this wont be the default in Chrome until at least 2020.