Bug 1402978 - Add cookie domain field to WebDriver:AddCookie r?ato, whimboo
There were two issues with the previous implementation:
* Domain cookies were created as host only cookies (due to lack of
leading '.' characters)
* The cookie domain included in the Marionette request was completely
ignored, which always resulted in host-only cookies
MozReview-Commit-ID: 2JLQ3vwNMrb
--- a/testing/marionette/cookie.js
+++ b/testing/marionette/cookie.js
@@ -32,18 +32,19 @@ this.cookie = {
/**
* Unmarshal a JSON Object to a cookie representation.
*
* Effectively this will run validation checks on |json|, which will
* produce the errors expected by WebDriver if the input is not valid.
*
* @param {Object.<string, (number|boolean|string)>} json
* Cookie to be deserialised. <var>name</var> and <var>value</var>
- * are required fields which must be strings. The <var>path</var>
- * field is optional, but must be a string if provided.
+ * are required fields which must be strings. The <var>path</var> and
+ * <var>domain</var> fields are optional, but must be a string if
+ * provided.
* The <var>secure</var>, <var>httpOnly</var>, and
* <var>session</var>fields are similarly optional, but must be
* booleans. Likewise, the <var>expiry</var> field is optional but
* must be unsigned integer.
*
* @return {Cookie}
* Valid cookie object.
*
@@ -53,16 +54,24 @@ this.cookie = {
cookie.fromJSON = function(json) {
let newCookie = {};
assert.object(json, pprint`Expected cookie object, got ${json}`);
newCookie.name = assert.string(json.name, "Cookie name must be string");
newCookie.value = assert.string(json.value, "Cookie value must be string");
+ if (typeof json.domain != "undefined") {
+ let domain = assert.string(json.domain, "Cookie domain must be string");
+ if (domain.substring(0, 1) !== ".") {
+ // make sure that this is stored as a domain cookie
+ domain = "." + domain;
+ }
+ newCookie.domain = domain;
+ }
if (typeof json.path != "undefined") {
newCookie.path = assert.string(json.path, "Cookie path must be string");
}
if (typeof json.secure != "undefined") {
newCookie.secure = assert.boolean(json.secure, "Cookie secure flag must be boolean");
}
if (typeof json.httpOnly != "undefined") {
newCookie.httpOnly = assert.boolean(json.httpOnly, "Cookie httpOnly flag must be boolean");
@@ -105,20 +114,21 @@ cookie.add = function(newCookie, {restri
// twenty years into the future
let date = new Date();
let now = new Date(Date.now());
date.setYear(now.getFullYear() + 20);
newCookie.expiry = date.getTime() / 1000;
}
if (restrictToHost) {
- if (newCookie.domain !== restrictToHost) {
+ if (!restrictToHost.endsWith(newCookie.domain) &&
+ ("." + restrictToHost) !== newCookie.domain) {
throw new InvalidCookieDomainError(
`Cookies may only be set ` +
- ` for the current domain (${restrictToHost})`);
+ `for the current domain (${restrictToHost})`);
}
}
// remove port from domain, if present.
// unfortunately this catches IPv6 addresses by mistake
// TODO: Bug 814416
newCookie.domain = newCookie.domain.replace(IPV4_PORT_EXPR, "");
--- a/testing/marionette/test_cookie.js
+++ b/testing/marionette/test_cookie.js
@@ -63,16 +63,33 @@ add_test(function test_fromJSON() {
}
// name and value
for (let invalidType of [42, true, [], {}, null, undefined]) {
Assert.throws(() => cookie.fromJSON({name: invalidType}), "Cookie name must be string");
Assert.throws(() => cookie.fromJSON({value: invalidType}), "Cookie value must be string");
}
+ // domain
+ for (let invalidType of [42, true, [], {}, null]) {
+ let test = {
+ name: "foo",
+ value: "bar",
+ domain: invalidType
+ };
+ Assert.throws(() => cookie.fromJSON(test), "Cookie domain must be string");
+ }
+ let test = {
+ name: "foo",
+ value: "bar",
+ domain: "domain"
+ };
+ let parsedCookie = cookie.fromJSON(test);
+ equal(parsedCookie.domain, ".domain");
+
// path
for (let invalidType of [42, true, [], {}, null]) {
let test = {
name: "foo",
value: "bar",
path: invalidType,
};
Assert.throws(() => cookie.fromJSON(test), "Cookie path must be string");
@@ -125,24 +142,26 @@ add_test(function test_fromJSON() {
for (let missing of ["path", "secure", "httpOnly", "session", "expiry"]) {
ok(!bare.hasOwnProperty(missing));
}
// everything
let full = cookie.fromJSON({
name: "name",
value: "value",
+ domain: ".domain",
path: "path",
secure: true,
httpOnly: true,
session: true,
expiry: 42,
});
equal("name", full.name);
equal("value", full.value);
+ equal(".domain", full.domain);
equal("path", full.path);
equal(true, full.secure);
equal(true, full.httpOnly);
equal(true, full.session);
equal(42, full.expiry);
run_next_test();
});
@@ -157,17 +176,17 @@ add_test(function test_add() {
Assert.throws(
() => cookie.add({name: invalidType}),
"Cookie must have string name");
Assert.throws(
() => cookie.add({name: "name", value: invalidType}),
"Cookie must have string value");
Assert.throws(
() => cookie.add({name: "name", value: "value", domain: invalidType}),
- "Cookie must have string value");
+ "Cookie must have string domain");
}
cookie.add({
name: "name",
value: "value",
domain: "domain",
});
equal(1, cookie.manager.cookies.length);
--- a/testing/web-platform/tests/webdriver/tests/cookies/cookies.py
+++ b/testing/web-platform/tests/webdriver/tests/cookies/cookies.py
@@ -1,8 +1,11 @@
+from tests.support.inline import inline
+from tests.support.fixtures import clear_all_cookies
+
def test_get_named_cookie(session, url):
session.url = url("/common/blank.html")
session.execute_script("document.cookie = 'foo=bar'")
result = session.transport.send("GET", "session/%s/cookie/foo" % session.session_id)
assert result.status == 200
assert isinstance(result.body["value"], dict)
@@ -21,8 +24,81 @@ def test_get_named_cookie(session, url):
assert isinstance(cookie["secure"], bool)
assert "httpOnly" in cookie
assert isinstance(cookie["httpOnly"], bool)
assert "expiry" in cookie
assert isinstance(cookie["expiry"], (int, long))
assert cookie["name"] == "foo"
assert cookie["value"] == "bar"
+
+def test_duplicated_cookie(session, url):
+ session.url = url("/common/blank.html")
+ clear_all_cookies(session)
+ create_cookie_request = {
+ "cookie": {
+ "name": "hello",
+ "value": "world",
+ "domain": "web-platform.test",
+ "path": "/",
+ "httpOnly": False,
+ "secure": False
+ }
+ }
+ result = session.transport.send("POST", "session/%s/cookie" % session.session_id, create_cookie_request)
+ assert result.status == 200
+ assert "value" in result.body
+ assert isinstance(result.body["value"], dict)
+
+ session.url = inline("<script>document.cookie = 'hello=newworld; domain=web-platform.test; path=/';</script>")
+ result = session.transport.send("GET", "session/%s/cookie" % session.session_id)
+ assert result.status == 200
+ assert "value" in result.body
+ assert isinstance(result.body["value"], list)
+ assert len(result.body["value"]) == 1
+ assert isinstance(result.body["value"][0], dict)
+
+ cookie = result.body["value"][0]
+ assert "name" in cookie
+ assert isinstance(cookie["name"], basestring)
+ assert "value" in cookie
+ assert isinstance(cookie["value"], basestring)
+
+ assert cookie["name"] == "hello"
+ assert cookie["value"] == "newworld"
+
+def test_add_domain_cookie(session, url):
+ session.url = url("/common/blank.html")
+ clear_all_cookies(session)
+ create_cookie_request = {
+ "cookie": {
+ "name": "hello",
+ "value": "world",
+ "domain": "web-platform.test",
+ "path": "/",
+ "httpOnly": False,
+ "secure": False
+ }
+ }
+ result = session.transport.send("POST", "session/%s/cookie" % session.session_id, create_cookie_request)
+ assert result.status == 200
+ assert "value" in result.body
+ assert isinstance(result.body["value"], dict)
+
+ result = session.transport.send("GET", "session/%s/cookie" % session.session_id)
+ assert result.status == 200
+ assert "value" in result.body
+ assert isinstance(result.body["value"], list)
+ assert len(result.body["value"]) == 1
+ assert isinstance(result.body["value"][0], dict)
+
+ cookie = result.body["value"][0]
+ assert "name" in cookie
+ assert isinstance(cookie["name"], basestring)
+ assert "value" in cookie
+ assert isinstance(cookie["value"], basestring)
+ assert "domain" in cookie
+ assert isinstance(cookie["domain"], basestring)
+
+ assert cookie["name"] == "hello"
+ assert cookie["value"] == "world"
+ assert cookie["domain"] == ".web-platform.test"
+
--- a/testing/web-platform/tests/webdriver/tests/support/fixtures.py
+++ b/testing/web-platform/tests/webdriver/tests/support/fixtures.py
@@ -243,8 +243,13 @@ def create_dialog(session):
}}, 0);
""".format(result_var, dialog_type, text)
session.send_session_command("POST",
"execute/async",
{"script": spawn, "args": []})
return create_dialog
+
+def clear_all_cookies(session):
+ """Removes all cookies associated with the current active document"""
+ session.transport.send("DELETE", "session/%s/cookie" % session.session_id)
+