Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: distinguish between no samesite and samesite=none #240

Merged
merged 3 commits into from
Jul 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions lib/cookie.js
Original file line number Diff line number Diff line change
Expand Up @@ -619,11 +619,11 @@ function parse(str, options) {
case "lax":
c.sameSite = "lax";
break;
case "none":
c.sameSite = "none";
break;
default:
// RFC6265bis-02 S5.3.7 step 1:
// "If cookie-av's attribute-value is not a case-insensitive match
// for "Strict" or "Lax", ignore the "cookie-av"."
// This effectively sets it to 'none' from the prototype.
c.sameSite = undefined;
awaterma marked this conversation as resolved.
Show resolved Hide resolved
break;
}
break;
Expand Down Expand Up @@ -807,7 +807,7 @@ const cookieDefaults = {
pathIsDefault: null,
creation: null,
lastAccessed: null,
sameSite: "none"
sameSite: undefined
awaterma marked this conversation as resolved.
Show resolved Hide resolved
};

class Cookie {
Expand Down Expand Up @@ -1221,7 +1221,11 @@ class CookieJar {
}

// 6252bis-02 S5.4 Step 13 & 14:
if (cookie.sameSite !== "none" && sameSiteContext) {
if (
cookie.sameSite !== "none" &&
cookie.sameSite !== undefined &&
sameSiteContext
) {
// "If the cookie's "same-site-flag" is not "None", and the cookie
// is being set from a context whose "site for cookies" is not an
// exact match for request-uri's host's registered domain, then
Expand Down
38 changes: 33 additions & 5 deletions test/parsing_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,8 @@ vows
"has max-age": function(c) {
assert.equal(c.maxAge, 1234);
},
"has same-site 'none'": function(c) {
assert.equal(c.sameSite, "none");
"has same-site 'undefined'": function(c) {
assert.equal(c.sameSite, undefined);
},
"has extensions": function(c) {
assert.ok(c.extensions);
Expand Down Expand Up @@ -677,19 +677,47 @@ vows
assert.equal(c.extensions, null);
}
},
absent: {
none: {
topic: function() {
return Cookie.parse("abc=xyzzy; SameSite=example.com") || null;
return Cookie.parse("abc=xyz; SameSite=NoNe") || null;
},
parsed: function(c) {
assert.ok(c);
},
"is set to 'none' (by prototype)": function(c) {
"is none (lowercased)": function(c) {
assert.equal(c.sameSite, "none");
},
"no extensions": function(c) {
assert.equal(c.extensions, null);
}
},
bad: {
topic: function() {
return Cookie.parse("abc=xyzzy; SameSite=example.com") || null;
},
parsed: function(c) {
assert.ok(c);
},
"is set to 'undefined'": function(c) {
assert.equal(c.sameSite, undefined);
},
"no extensions": function(c) {
assert.equal(c.extensions, null);
}
},
absent: {
topic: function() {
return Cookie.parse("abc=xyzzy;") || null;
},
parsed: function(c) {
assert.ok(c);
},
"is set to 'undefined'": function(c) {
assert.equal(c.sameSite, undefined);
},
"no extensions": function(c) {
assert.equal(c.extensions, null);
}
}
},
"empty string": {
Expand Down
8 changes: 4 additions & 4 deletions test/same_site_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,9 @@ vows
topic: function(options) {
this.callSetCookie("garbage", options, this.callback);
},
"treated as 'none'": function(err, cookie) {
"treated as 'undefined'": function(err, cookie) {
assert.isNull(err);
assert.equal(cookie.sameSite, "none");
assert.equal(cookie.sameSite, undefined);
}
},
"for strict cookie": {
Expand All @@ -151,9 +151,9 @@ vows
topic: function(options) {
this.callSetCookie("normal", options, this.callback);
},
"treated as 'none'": function(err, cookie) {
"treated as 'undefined'": function(err, cookie) {
assert.isNull(err);
assert.equal(cookie.sameSite, "none");
assert.equal(cookie.sameSite, undefined);
}
}
},
Expand Down