From 3d736e3368baea8b463e3967d482d0dcc058239b Mon Sep 17 00:00:00 2001 From: Peter Major Date: Fri, 29 Sep 2017 15:22:50 +0100 Subject: [PATCH] Bug 1402978 - Add cookie domain field to WebDriver:AddCookie r=ato 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 --HG-- extra : rebase_source : c72ba077ef1b1a1f308e4c9a1d2093c18f7483ce --- testing/marionette/cookie.js | 18 ++++- testing/marionette/test_cookie.js | 21 ++++- .../tests/webdriver/tests/cookies/cookies.py | 76 +++++++++++++++++++ .../tests/webdriver/tests/support/fixtures.py | 5 ++ 4 files changed, 115 insertions(+), 5 deletions(-) diff --git a/testing/marionette/cookie.js b/testing/marionette/cookie.js index 492f5129f031..bb32d2f2ceb2 100644 --- a/testing/marionette/cookie.js +++ b/testing/marionette/cookie.js @@ -37,8 +37,9 @@ this.cookie = { * * @param {Object.} json * Cookie to be deserialised. name and value - * are required fields which must be strings. The path - * field is optional, but must be a string if provided. + * are required fields which must be strings. The path and + * domain fields are optional, but must be a string if + * provided. * The secure, httpOnly, and * sessionfields are similarly optional, but must be * booleans. Likewise, the expiry field is optional but @@ -58,6 +59,14 @@ cookie.fromJSON = function(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"); } @@ -110,10 +119,11 @@ cookie.add = function(newCookie, {restrictToHost = null} = {}) { } 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})`); } } diff --git a/testing/marionette/test_cookie.js b/testing/marionette/test_cookie.js index 962ac9690cf3..2f25ab1246fb 100644 --- a/testing/marionette/test_cookie.js +++ b/testing/marionette/test_cookie.js @@ -68,6 +68,23 @@ add_test(function test_fromJSON() { 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 = { @@ -130,6 +147,7 @@ add_test(function test_fromJSON() { let full = cookie.fromJSON({ name: "name", value: "value", + domain: ".domain", path: "path", secure: true, httpOnly: true, @@ -138,6 +156,7 @@ add_test(function test_fromJSON() { }); equal("name", full.name); equal("value", full.value); + equal(".domain", full.domain); equal("path", full.path); equal(true, full.secure); equal(true, full.httpOnly); @@ -162,7 +181,7 @@ add_test(function test_add() { "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({ diff --git a/testing/web-platform/tests/webdriver/tests/cookies/cookies.py b/testing/web-platform/tests/webdriver/tests/cookies/cookies.py index 42a8c4480f99..915223d2b94a 100644 --- a/testing/web-platform/tests/webdriver/tests/cookies/cookies.py +++ b/testing/web-platform/tests/webdriver/tests/cookies/cookies.py @@ -1,3 +1,6 @@ +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'") @@ -26,3 +29,76 @@ def test_get_named_cookie(session, url): 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("") + 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" + diff --git a/testing/web-platform/tests/webdriver/tests/support/fixtures.py b/testing/web-platform/tests/webdriver/tests/support/fixtures.py index f51c498274fa..d93f862578a2 100644 --- a/testing/web-platform/tests/webdriver/tests/support/fixtures.py +++ b/testing/web-platform/tests/webdriver/tests/support/fixtures.py @@ -248,3 +248,8 @@ def create_dialog(session): {"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) +