Skip to content

Commit

Permalink
fix: distinguish between no samesite and samesite=none (#240)
Browse files Browse the repository at this point in the history
* Adding some initial tests
* fix: distinguish between no samesite and samesite=none
  • Loading branch information
colincasey authored Jul 27, 2022
1 parent b8d7511 commit aa4396d
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 15 deletions.
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;
break;
}
break;
Expand Down Expand Up @@ -807,7 +807,7 @@ const cookieDefaults = {
pathIsDefault: null,
creation: null,
lastAccessed: null,
sameSite: "none"
sameSite: undefined
};

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

0 comments on commit aa4396d

Please sign in to comment.