Bug 1402978 - Add cookie domain field to WebDriver:AddCookie r=ato
authorPeter Major <peter.major@forgerock.com>
Fri, 29 Sep 2017 15:22:50 +0100
changeset 384152 8a9c620351c9fbb759f4bbf78f42c9b4c41a44ef
parent 384151 ab3b47fde689d52670a25b5cdc3f4afb8d2c2839
child 384153 9cc67135b50e89ddf7368635de48eda97dc58796
push id32623
push userkwierso@gmail.com
push dateTue, 03 Oct 2017 20:25:55 +0000
treeherdermozilla-central@65a5054a1f92 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersato
bugs1402978
milestone58.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
testing/marionette/cookie.js
testing/marionette/test_cookie.js
testing/web-platform/tests/webdriver/tests/cookies/cookies.py
testing/web-platform/tests/webdriver/tests/support/fixtures.py
--- 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)
+