Bug 1407675 - Fix cookie creation via Marionette using IP address r=ato

The issue here was that the cookie domain was always prepended with
'.' character. To resolve this edge-case Marionette now first checks
whether the cookie domain is in fact an IP address.

MozReview-Commit-ID: 4xBd4rscXxx

--HG--
extra : rebase_source : 92bf20ceb43c05f4610e3e0a5411027300586784

testing/marionette/cookie.js

@@ -4,7 +4,7 @@
 
 "use strict";
 
-const {interfaces: Ci, utils: Cu} = Components;
+const {interfaces: Ci, utils: Cu, results: Cr} = Components;
 
 Cu.import("resource://gre/modules/Services.jsm");
 
@@ -60,12 +60,7 @@ cookie.fromJSON = function(json) {
   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;
+    newCookie.domain = assert.string(json.domain, "Cookie domain must be string");
   }
   if (typeof json.path != "undefined") {
     newCookie.path = assert.string(json.path, "Cookie path must be string");
@@ -104,6 +99,12 @@ cookie.fromJSON = function(json) {
 cookie.add = function(newCookie, {restrictToHost = null} = {}) {
   assert.string(newCookie.name, "Cookie name must be string");
   assert.string(newCookie.value, "Cookie value must be string");
+
+  let hostOnly = false;
+  if (typeof newCookie.domain == "undefined") {
+    hostOnly = true;
+    newCookie.domain = restrictToHost;
+  }
   assert.string(newCookie.domain, "Cookie domain must be string");
 
   if (typeof newCookie.path == "undefined") {
@@ -118,11 +119,30 @@ cookie.add = function(newCookie, {restrictToHost = null} = {}) {
     newCookie.expiry = date.getTime() / 1000;
   }
 
+  let isIpAddress = false;
+  try {
+    Services.eTLD.getPublicSuffixFromHost(newCookie.domain);
+  } catch (e) {
+    switch (e.result) {
+      case Cr.NS_ERROR_HOST_IS_IP_ADDRESS:
+        isIpAddress = true;
+        break;
+      default:
+        throw new InvalidCookieDomainError(newCookie.domain);
+    }
+  }
+
+  if (!hostOnly && !isIpAddress) {
+    // only store this as a domain cookie if the domain was specified in the
+    // request and it wasn't an IP address.
+    newCookie.domain = "." + newCookie.domain;
+  }
+
   if (restrictToHost) {
     if (!restrictToHost.endsWith(newCookie.domain) &&
-        ("." + restrictToHost) !== newCookie.domain) {
-      throw new InvalidCookieDomainError(
-          `Cookies may only be set ` +
+        ("." + restrictToHost) !== newCookie.domain &&
+        restrictToHost !== newCookie.domain) {
+      throw new InvalidCookieDomainError(`Cookies may only be set ` +
           `for the current domain (${restrictToHost})`);
     }
   }

testing/marionette/driver.js

@@ -2633,9 +2633,6 @@ GeckoDriver.prototype.addCookie = function(cmd) {
   }
 
   let newCookie = cookie.fromJSON(cmd.parameters.cookie);
-  if (typeof newCookie.domain == "undefined") {
-    newCookie.domain = hostname;
-  }
 
   cookie.add(newCookie, {restrictToHost: hostname});
 };

testing/marionette/test_cookie.js

@@ -37,7 +37,8 @@ cookie.manager = {
   },
 
   getCookiesFromHost(host, originAttributes = {}) {
-    let hostCookies = this.cookies.filter(cookie => cookie.host === host);
+    let hostCookies = this.cookies.filter(cookie => cookie.host === host ||
+       cookie.host === "." + host);
     let nextIndex = 0;
 
     return {
@@ -83,7 +84,7 @@ add_test(function test_fromJSON() {
     domain: "domain"
   };
   let parsedCookie = cookie.fromJSON(test);
-  equal(parsedCookie.domain, ".domain");
+  equal(parsedCookie.domain, "domain");
 
   // path
   for (let invalidType of [42, true, [], {}, null]) {
@@ -189,7 +190,7 @@ add_test(function test_add() {
   equal(1, cookie.manager.cookies.length);
   equal("name", cookie.manager.cookies[0].name);
   equal("value", cookie.manager.cookies[0].value);
-  equal("domain", cookie.manager.cookies[0].host);
+  equal(".domain", cookie.manager.cookies[0].host);
   equal("/", cookie.manager.cookies[0].path);
   ok(cookie.manager.cookies[0].expiry > new Date(Date.now()).getTime() / 1000);
 
@@ -210,7 +211,7 @@ add_test(function test_add() {
     value: "value4",
     domain: "my.domain:1234",
   });
-  equal("my.domain", cookie.manager.cookies[2].host);
+  equal(".my.domain", cookie.manager.cookies[2].host);
 
   cookie.add({
     name: "name5",
@@ -247,13 +248,11 @@ add_test(function test_remove() {
 add_test(function test_iter() {
   cookie.manager.cookies = [];
 
-  cookie.add({name: "0", value: "", domain: "example.com"});
-  cookie.add({name: "1", value: "", domain: "foo.example.com"});
-  cookie.add({name: "2", value: "", domain: "bar.example.com"});
-
+  cookie.add({name: "0", value: "", domain: "foo.example.com"});
+  cookie.add({name: "1", value: "", domain: "bar.example.com"});
   let fooCookies = [...cookie.iter("foo.example.com")];
   equal(1, fooCookies.length);
-  equal("foo.example.com", fooCookies[0].domain);
+  equal(".foo.example.com", fooCookies[0].domain);
 
   run_next_test();
 });

testing/web-platform/meta/MANIFEST.json

@@ -409321,6 +409321,12 @@
      {}
     ]
    ],
+   "webdriver/tests/cookies/add_cookie.py": [
+    [
+     "/webdriver/tests/cookies/add_cookie.py",
+     {}
+    ]
+   ],
    "webdriver/tests/cookies/delete_cookie.py": [
     [
      "/webdriver/tests/cookies/delete_cookie.py",

testing/web-platform/tests/webdriver/tests/cookies/add_cookie.py

@@ -0,0 +1,76 @@
+from tests.support.fixtures import clear_all_cookies
+from tests.support.fixtures import server_config
+
+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"
+
+def test_add_cookie_for_ip(session, url, server_config):
+    session.url = "http://127.0.0.1:%s/404" % (server_config["ports"]["http"][0])
+    clear_all_cookies(session)
+    create_cookie_request = {
+        "cookie": {
+            "name": "hello",
+            "value": "world",
+            "domain": "127.0.0.1",
+            "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"] == "127.0.0.1"

testing/web-platform/tests/webdriver/tests/cookies/get_named_cookie.py

@@ -65,40 +65,3 @@ def test_duplicated_cookie(session, url):
     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"
-