Bug 1407675 - Fix cookie creation via Marionette using IP address r?ato, whimboo
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
--- a/testing/marionette/cookie.js
+++ b/testing/marionette/cookie.js
@@ -1,15 +1,15 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */
"use strict";
-const {interfaces: Ci, utils: Cu} = Components;
+const {interfaces: Ci, utils: Cu, results: Cr} = Components;
Cu.import("resource://gre/modules/Services.jsm");
Cu.import("chrome://marionette/content/assert.js");
const {
InvalidCookieDomainError,
pprint,
} = Cu.import("chrome://marionette/content/error.js", {});
@@ -55,22 +55,17 @@ 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;
+ 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");
}
if (typeof json.secure != "undefined") {
newCookie.secure = assert.boolean(json.secure, "Cookie secure flag must be boolean");
}
if (typeof json.httpOnly != "undefined") {
@@ -99,35 +94,60 @@ cookie.fromJSON = function(json) {
* not present and of the correct type.
* @throws {InvalidCookieDomainError}
* If <var>restrictToHost</var> is set and <var>newCookie</var>'s
* domain does not match.
*/
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") {
newCookie.path = "/";
}
if (typeof newCookie.expiry == "undefined") {
// 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;
}
+ 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})`);
}
}
// 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/driver.js
+++ b/testing/marionette/driver.js
@@ -2626,19 +2626,16 @@ GeckoDriver.prototype.addCookie = functi
let {protocol, hostname} = this.currentURL;
const networkSchemes = ["ftp:", "http:", "https:"];
if (!networkSchemes.includes(protocol)) {
throw new InvalidCookieDomainError("Document is cookie-averse");
}
let newCookie = cookie.fromJSON(cmd.parameters.cookie);
- if (typeof newCookie.domain == "undefined") {
- newCookie.domain = hostname;
- }
cookie.add(newCookie, {restrictToHost: hostname});
};
/**
* Get all the cookies for the current domain.
*
* This is the equivalent of calling <code>document.cookie</code> and
--- a/testing/marionette/test_cookie.js
+++ b/testing/marionette/test_cookie.js
@@ -32,17 +32,18 @@ cookie.manager = {
candidate.path === path) {
return this.cookies.splice(i, 1);
}
}
return false;
},
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 {
hasMoreElements () {
return nextIndex < hostCookies.length;
},
getNext () {
@@ -78,17 +79,17 @@ add_test(function test_fromJSON() {
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");
+ equal(parsedCookie.domain, "domain");
// path
for (let invalidType of [42, true, [], {}, null]) {
let test = {
name: "foo",
value: "bar",
path: invalidType,
};
@@ -184,17 +185,17 @@ add_test(function test_add() {
cookie.add({
name: "name",
value: "value",
domain: "domain",
});
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);
cookie.add({
name: "name2",
value: "value2",
domain: "domain2",
});
@@ -205,17 +206,17 @@ add_test(function test_add() {
cookie.add(biscuit, {restrictToHost: "other domain"});
}, /Cookies may only be set for the current domain/);
cookie.add({
name: "name4",
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",
value: "value5",
domain: "domain5",
path: "/foo/bar",
});
equal("/foo/bar", cookie.manager.cookies[3].path);
@@ -242,18 +243,16 @@ add_test(function test_remove() {
equal(undefined, cookie.manager.cookies[0]);
run_next_test();
});
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();
});
--- a/testing/web-platform/meta/MANIFEST.json
+++ b/testing/web-platform/meta/MANIFEST.json
@@ -409311,16 +409311,22 @@
]
],
"webdriver/tests/contexts/resizing_and_positioning.py": [
[
"/webdriver/tests/contexts/resizing_and_positioning.py",
{}
]
],
+ "webdriver/tests/cookies/add_cookie.py": [
+ [
+ "/webdriver/tests/cookies/add_cookie.py",
+ {}
+ ]
+ ],
"webdriver/tests/cookies/delete_cookie.py": [
[
"/webdriver/tests/cookies/delete_cookie.py",
{}
]
],
"webdriver/tests/cookies/get_named_cookie.py": [
[
new file mode 100644
--- /dev/null
+++ b/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"
--- a/testing/web-platform/tests/webdriver/tests/cookies/get_named_cookie.py
+++ b/testing/web-platform/tests/webdriver/tests/cookies/get_named_cookie.py
@@ -60,45 +60,8 @@ def test_duplicated_cookie(session, url)
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"
-