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 426838 8a9c620351c9fbb759f4bbf78f42c9b4c41a44ef
parent 426837 ab3b47fde689d52670a25b5cdc3f4afb8d2c2839
child 426839 9cc67135b50e89ddf7368635de48eda97dc58796
push id97
push userfmarier@mozilla.com
push dateSat, 14 Oct 2017 01:12:59 +0000
reviewersato
bugs1402978
milestone58.0a1
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)
+