Bug 1408962 - Make add cookie command webdriver spec comformant r?ato, whimboo
* Changed positiveInteger to validate upper bounds
* Removed processing for the no longer existent 'session' field
* Added default values for absent fields
MozReview-Commit-ID: E4x6TyMwZQs
--- a/testing/marionette/assert.js
+++ b/testing/marionette/assert.js
@@ -228,17 +228,17 @@ assert.callable = function(obj, msg = ""
* @return {number}
* <var>obj</var> is returned unaltered.
*
* @throws {InvalidArgumentError}
* If <var>obj</var> is not an integer.
*/
assert.integer = function(obj, msg = "") {
msg = msg || pprint`Expected ${obj} to be an integer`;
- return assert.that(Number.isInteger, msg)(obj);
+ return assert.that(Number.isSafeInteger, msg)(obj);
};
/**
* Asserts that <var>obj</var> is a positive integer.
*
* @param {?} obj
* Value to test.
* @param {string=} msg
--- a/testing/marionette/cookie.js
+++ b/testing/marionette/cookie.js
@@ -4,17 +4,20 @@
"use strict";
const {interfaces: Ci, utils: Cu, results: Cr} = Components;
Cu.import("resource://gre/modules/Services.jsm");
Cu.import("chrome://marionette/content/assert.js");
-const {InvalidCookieDomainError} = Cu.import("chrome://marionette/content/error.js", {});
+const {
+ InvalidCookieDomainError,
+ UnableToSetCookieError,
+} = Cu.import("chrome://marionette/content/error.js", {});
const {pprint} = Cu.import("chrome://marionette/content/format.js", {});
this.EXPORTED_SYMBOLS = ["cookie"];
const IPV4_PORT_EXPR = /:\d+$/;
/** @namespace */
this.cookie = {
@@ -29,54 +32,50 @@ 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> and
+ * Cookie to be deserialised. <var>name</var> and <var>value</var>
+ * 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.
+ * The <var>secure</var>, and <var>httpOnly</var> 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.
*
* @throws {InvalidArgumentError}
* If any of the properties are invalid.
*/
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.path != "undefined") {
+ newCookie.path = assert.string(json.path, "Cookie path must be string");
+ }
if (typeof json.domain != "undefined") {
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") {
newCookie.httpOnly = assert.boolean(json.httpOnly, "Cookie httpOnly flag must be boolean");
}
- if (typeof json.session != "undefined") {
- newCookie.session = assert.boolean(json.session, "Cookie session flag must be boolean");
- }
if (typeof json.expiry != "undefined") {
newCookie.expiry = assert.positiveInteger(json.expiry, "Cookie expiry must be a positive integer");
}
return newCookie;
};
/**
@@ -88,38 +87,46 @@ cookie.fromJSON = function(json) {
* Perform test that <var>newCookie</var>'s domain matches this.
*
* @throws {TypeError}
* If <var>name</var>, <var>value</var>, or <var>domain</var> are
* not present and of the correct type.
* @throws {InvalidCookieDomainError}
* If <var>restrictToHost</var> is set and <var>newCookie</var>'s
* domain does not match.
+ * @throws {UnableToSetCookieError}
+ * If an error occurred while trying to save the cookie.
*/
cookie.add = function(newCookie, {restrictToHost = null} = {}) {
assert.string(newCookie.name, "Cookie name must be string");
assert.string(newCookie.value, "Cookie value must be string");
+ if (typeof newCookie.path == "undefined") {
+ newCookie.path = "/";
+ }
+
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.secure == "undefined") {
+ newCookie.secure = false;
}
-
+ if (typeof newCookie.httpOnly == "undefined") {
+ newCookie.httpOnly = false;
+ }
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;
+ // The XPCOM interface requires the expiry field even for session cookies.
+ newCookie.expiry = Number.MAX_SAFE_INTEGER;
+ newCookie.session = true;
+ } else {
+ newCookie.session = false;
}
let isIpAddress = false;
try {
Services.eTLD.getPublicSuffixFromHost(newCookie.domain);
} catch (e) {
switch (e.result) {
case Cr.NS_ERROR_HOST_IS_IP_ADDRESS:
@@ -145,26 +152,30 @@ cookie.add = function(newCookie, {restri
}
}
// remove port from domain, if present.
// unfortunately this catches IPv6 addresses by mistake
// TODO: Bug 814416
newCookie.domain = newCookie.domain.replace(IPV4_PORT_EXPR, "");
- cookie.manager.add(
- newCookie.domain,
- newCookie.path,
- newCookie.name,
- newCookie.value,
- newCookie.secure,
- newCookie.httpOnly,
- newCookie.session,
- newCookie.expiry,
- {} /* origin attributes */);
+ try {
+ cookie.manager.add(
+ newCookie.domain,
+ newCookie.path,
+ newCookie.name,
+ newCookie.value,
+ newCookie.secure,
+ newCookie.httpOnly,
+ newCookie.session,
+ newCookie.expiry,
+ {} /* origin attributes */);
+ } catch (e) {
+ throw new UnableToSetCookieError(e);
+ }
};
/**
* Remove cookie from the cookie store.
*
* @param {Cookie} toDelete
* Cookie to remove.
*/
--- a/testing/marionette/test_cookie.js
+++ b/testing/marionette/test_cookie.js
@@ -5,16 +5,19 @@
const {utils: Cu} = Components;
Cu.import("chrome://marionette/content/cookie.js");
cookie.manager = {
cookies: [],
add: function (domain, path, name, value, secure, httpOnly, session, expiry, originAttributes) {
+ if (name === "fail") {
+ throw new Error("An error occurred while adding cookie");
+ }
let newCookie = {
host: domain,
path: path,
name: name,
value: value,
isSecure: secure,
isHttpOnly: httpOnly,
isSession: session,
@@ -111,28 +114,18 @@ add_test(function test_fromJSON() {
let test = {
name: "foo",
value: "bar",
httpOnly: invalidType,
};
Assert.throws(() => cookie.fromJSON(test), /Cookie httpOnly flag must be boolean/);
}
- // session
- for (let invalidType of ["foo", 42, [], {}, null]) {
- let test = {
- name: "foo",
- value: "bar",
- session: invalidType,
- };
- Assert.throws(() => cookie.fromJSON(test), /Cookie session flag must be boolean/);
- }
-
// expiry
- for (let invalidType of ["foo", true, [], {}, null]) {
+ for (let invalidType of [-1, Number.MAX_SAFE_INTEGER + 1, "foo", true, [], {}, null]) {
let test = {
name: "foo",
value: "bar",
expiry: invalidType,
};
Assert.throws(() => cookie.fromJSON(test), /Cookie expiry must be a positive integer/);
}
@@ -147,26 +140,24 @@ add_test(function test_fromJSON() {
// 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();
});
add_test(function test_add() {
cookie.manager.cookies = [];
@@ -216,16 +207,20 @@ add_test(function test_add() {
cookie.add({
name: "name5",
value: "value5",
domain: "domain5",
path: "/foo/bar",
});
equal("/foo/bar", cookie.manager.cookies[3].path);
+ Assert.throws(() => {
+ cookie.add({name: "fail", value: "value6", domain: "domain6"})
+ }, /UnableToSetCookieError/);
+
run_next_test();
});
add_test(function test_remove() {
cookie.manager.cookies = [];
let crumble = {
name: "test_remove",
@@ -242,41 +237,38 @@ add_test(function test_remove() {
equal(0, cookie.manager.cookies.length);
equal(undefined, cookie.manager.cookies[0]);
run_next_test();
});
add_test(function test_iter() {
cookie.manager.cookies = [];
+ let tomorrow = new Date();
+ tomorrow.setHours(tomorrow.getHours() + 24);
cookie.add({
- session: false,
+ expiry: tomorrow,
name: "0",
value: "",
domain: "foo.example.com",
});
cookie.add({
- session: false,
+ expiry: tomorrow,
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(true, fooCookies[0].hasOwnProperty("expiry"));
- // here we're explicitly setting session to true as a workaround until
- // bug 1408962 has been fixed. when that bug has been fixed the cookie
- // will be created as session cookie simply by leaving out the 'expiry'
- // property.
cookie.add({
- session: true,
name: "aSessionCookie",
value: "",
domain: "session.com",
});
let sessionCookies = [...cookie.iter("session.com")];
equal(1, sessionCookies.length);
equal("aSessionCookie", sessionCookies[0].name);
--- a/testing/web-platform/tests/webdriver/tests/cookies/add_cookie.py
+++ b/testing/web-platform/tests/webdriver/tests/cookies/add_cookie.py
@@ -1,10 +1,11 @@
from tests.support.fixtures import clear_all_cookies
from tests.support.fixtures import server_config
+from datetime import datetime, timedelta
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",
@@ -69,8 +70,75 @@ def test_add_cookie_for_ip(session, url,
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"
+
+def test_add_non_session_cookie(session, url):
+ session.url = url("/common/blank.html")
+ clear_all_cookies(session)
+ a_year_from_now = int((datetime.utcnow() + timedelta(days=365)).strftime("%s"))
+ create_cookie_request = {
+ "cookie": {
+ "name": "hello",
+ "value": "world",
+ "expiry": a_year_from_now
+ }
+ }
+ 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 "expiry" in cookie
+ assert isinstance(cookie["expiry"], int)
+
+ assert cookie["name"] == "hello"
+ assert cookie["value"] == "world"
+ assert cookie["expiry"] == a_year_from_now
+
+def test_add_session_cookie(session, url):
+ session.url = url("/common/blank.html")
+ clear_all_cookies(session)
+ create_cookie_request = {
+ "cookie": {
+ "name": "hello",
+ "value": "world"
+ }
+ }
+ 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 "expiry" in cookie
+ assert cookie.get("expiry") is None
+
+ assert cookie["name"] == "hello"
+ assert cookie["value"] == "world"