Skip to content

Don't send SameSite=None to incompatible browsers

We set SameSite=None in GitLab 12.10 via !28205 (merged) because Chrome v80, rolled out in March 2020, treats any cookies without the SameSite directive set as though they are SameSite=Lax (https://www.chromestatus.com/feature/5088147346030592). This is a breaking change from the previous default behavior, which was to treat those cookies as SameSite=None.

However, older browsers (e.g. MacOS 10.14 on Safari 13.0.3) may interpret the None as Strict, which causes users that click on gitlab.com links from third-party sites (e.g. Gmail, Slack) to log in again.

https://www.chromium.org/updates/same-site/incompatible-clients recommends a set of regular expressions to determine whether to send this. This change implements most of the logic but skips one case since this doesn't seem common: macOS 10.14 with an embedded WebKit browser. This is also what https://rubygems.org/gems/rails_same_site_cookie does.

I considered using that gem (!40663 (closed)), but I didn't like how it added another dependency (user_agent_parser) that loads a large YAML database (https://github.com/ua-parser/uap-ruby#the-pattern-database). Plus, its performance is likely to be significantly slower than the implementation using RE2 as we saw with the browser gem.

Performance:

Warming up --------------------------------------
            no check    46.338k i/100ms
               check    21.325k i/100ms
Calculating -------------------------------------
            no check    446.012k (± 5.5%) i/s -      2.224M in   5.003135s
               check    210.757k (± 2.8%) i/s -      1.066M in   5.062987s

Comparison:
            no check:   446012.3 i/s
               check:   210756.9 i/s - 2.12x  (± 0.00) slower

Benchmark script

https://gitlab.com/-/snippets/2009886

ruby-prof output

Gathered after running a check 1000 times:

image

Relates to #241785 (closed)

Edited by Stan Hu

Merge request reports

Loading