Bug 784790 - GCLI cookie command should use internal Cookie API rather than document.cookie; r=mratcliffe
authorJoe Walker <jwalker@mozilla.com>
Sat, 23 Mar 2013 09:53:10 +0000
changeset 126117 d5cdec6efff9a37f3c89dbd63a336d07fc72aa23
parent 126116 4ff44d1d475033d697946dea2df9fd4df7a7f11a
child 126118 bef19bca23f9aa2ae4c781ff7ad1b315b65938ad
push id25287
push useremorley@mozilla.com
push dateMon, 25 Mar 2013 17:02:43 +0000
treeherdermozilla-inbound@21657a1327cd [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmratcliffe
bugs784790
milestone22.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 784790 - GCLI cookie command should use internal Cookie API rather than document.cookie; r=mratcliffe
browser/devtools/commandline/BuiltinCommands.jsm
browser/devtools/commandline/commandline.css
browser/devtools/commandline/gcli.jsm
browser/devtools/commandline/test/browser_cmd_cookie.js
browser/locales/en-US/chrome/browser/devtools/gclicommands.properties
toolkit/devtools/Console.jsm
--- a/browser/devtools/commandline/BuiltinCommands.jsm
+++ b/browser/devtools/commandline/BuiltinCommands.jsm
@@ -775,119 +775,200 @@ XPCOMUtils.defineLazyModuleGetter(this, 
 }(this));
 
 /* CmdCookie --------------------------------------------------------------- */
 
 (function(module) {
   XPCOMUtils.defineLazyModuleGetter(this, "console",
                                     "resource://gre/modules/devtools/Console.jsm");
 
-  // We should really be using nsICookieManager so we can read more than just the
-  // key/value of cookies. The difficulty is filtering the cookies that are
-  // relevant to the current page. See
-  // https://github.com/firebug/firebug/blob/master/extension/content/firebug/cookies/cookieObserver.js#L123
-  // For details on how this is done with Firebug
-
-  /**
-  * 'cookie' command
-  */
-  gcli.addCommand({
-    name: "cookie",
-    description: gcli.lookup("cookieDesc"),
-    manual: gcli.lookup("cookieManual")
-  });
+  const cookieMgr = Cc["@mozilla.org/cookiemanager;1"]
+                      .getService(Ci.nsICookieManager2);
 
   /**
-  * The template for the 'cookie list' command.
-  */
-  var cookieListHtml = "" +
-    "<table>" +
-    "  <tr>" +
-    "    <th>" + gcli.lookup("cookieListOutKey") + "</th>" +
-    "    <th>" + gcli.lookup("cookieListOutValue") + "</th>" +
-    "    <th>" + gcli.lookup("cookieListOutActions") + "</th>" +
-    "  </tr>" +
-    "  <tr foreach='cookie in ${cookies}'>" +
-    "    <td>${cookie.key}</td>" +
-    "    <td>${cookie.value}</td>" +
-    "    <td>" +
-    "      <span class='gcli-out-shortcut' onclick='${onclick}'" +
-    "          data-command='cookie set ${cookie.key} '" +
-    "          >" + gcli.lookup("cookieListOutEdit") + "</span>" +
-    "      <span class='gcli-out-shortcut'" +
-    "          onclick='${onclick}' ondblclick='${ondblclick}'" +
-    "          data-command='cookie remove ${cookie.key}'" +
-    "          >" + gcli.lookup("cookieListOutRemove") + "</span>" +
-    "    </td>" +
-    "  </tr>" +
-    "</table>" +
+   * The template for the 'cookie list' command.
+   */
+  let cookieListHtml = "" +
+    "<ul class='gcli-cookielist-list'>" +
+    "  <li foreach='cookie in ${cookies}'>" +
+    "    <div>${cookie.name}=${cookie.value}</div>" +
+    "    <table class='gcli-cookielist-detail'>" +
+    "      <tr>" +
+    "        <td>" + gcli.lookup("cookieListOutHost") + "</td>" +
+    "        <td>${cookie.host}</td>" +
+    "      </tr>" +
+    "      <tr>" +
+    "        <td>" + gcli.lookup("cookieListOutPath") + "</td>" +
+    "        <td>${cookie.path}</td>" +
+    "      </tr>" +
+    "      <tr>" +
+    "        <td>" + gcli.lookup("cookieListOutExpires") + "</td>" +
+    "        <td>${cookie.expires}</td>" +
+    "      </tr>" +
+    "      <tr>" +
+    "        <td>" + gcli.lookup("cookieListOutAttributes") + "</td>" +
+    "        <td>${cookie.attrs}</td>" +
+    "      </tr>" +
+    "      <tr><td colspan='2'>" +
+    "        <span class='gcli-out-shortcut' onclick='${onclick}'" +
+    "            data-command='cookie set ${cookie.name} '" +
+    "            >" + gcli.lookup("cookieListOutEdit") + "</span>" +
+    "        <span class='gcli-out-shortcut'" +
+    "            onclick='${onclick}' ondblclick='${ondblclick}'" +
+    "            data-command='cookie remove ${cookie.name}'" +
+    "            >" + gcli.lookup("cookieListOutRemove") + "</span>" +
+    "      </td></tr>" +
+    "    </table>" +
+    "  </li>" +
+    "</ul>" +
     "";
 
-  /**
-  * 'cookie list' command
-  */
-  gcli.addCommand({
-    name: "cookie list",
-    description: gcli.lookup("cookieListDesc"),
-    manual: gcli.lookup("cookieListManual"),
-    returnType: "string",
-    exec: function Command_cookieList(args, context) {
-      // Parse out an array of { key:..., value:... } objects for each cookie
-      var doc = context.environment.contentDocument;
-      var cookies = doc.cookie.split("; ").map(function(cookieStr) {
-        var equalsPos = cookieStr.indexOf("=");
-        return {
-          key: cookieStr.substring(0, equalsPos),
-          value: cookieStr.substring(equalsPos + 1)
-        };
-      });
+  gcli.addConverter({
+    from: "cookies",
+    to: "view",
+    exec: function(cookies, context) {
+      if (cookies.length == 0) {
+        let host = context.environment.document.location.host;
+        let msg = gcli.lookupFormat("cookieListOutNoneHost", [ host ]);
+        return context.createView({ html: "<span>" + msg + "</span>" });
+      }
+
+      for (let cookie of cookies) {
+        cookie.expires = translateExpires(cookie.expires);
+
+        let noAttrs = !cookie.secure && !cookie.httpOnly && !cookie.sameDomain;
+        cookie.attrs = (cookie.secure ? 'secure' : ' ') +
+                       (cookie.httpOnly ? 'httpOnly' : ' ') +
+                       (cookie.sameDomain ? 'sameDomain' : ' ') +
+                       (noAttrs ? gcli.lookup("cookieListOutNone") : ' ');
+      }
 
       return context.createView({
         html: cookieListHtml,
         data: {
+          options: { allowEval: true },
           cookies: cookies,
           onclick: createUpdateHandler(context),
           ondblclick: createExecuteHandler(context),
         }
       });
     }
   });
 
   /**
-  * 'cookie remove' command
-  */
+   * The cookie 'expires' value needs converting into something more readable
+   */
+  function translateExpires(expires) {
+    if (expires == 0) {
+      return gcli.lookup("cookieListOutSession");
+    }
+    return new Date(expires).toLocaleString();
+  }
+
+  /**
+   * Check if a given cookie matches a given host
+   */
+  function isCookieAtHost(cookie, host) {
+    if (cookie.host == null) {
+      return host == null;
+    }
+    if (cookie.host.startsWith(".")) {
+      return cookie.host === "." + host;
+    }
+    else {
+      return cookie.host == host;
+    }
+  }
+
+  /**
+   * 'cookie' command
+   */
+  gcli.addCommand({
+    name: "cookie",
+    description: gcli.lookup("cookieDesc"),
+    manual: gcli.lookup("cookieManual")
+  });
+
+  /**
+   * 'cookie list' command
+   */
+  gcli.addCommand({
+    name: "cookie list",
+    description: gcli.lookup("cookieListDesc"),
+    manual: gcli.lookup("cookieListManual"),
+    returnType: "cookies",
+    exec: function Command_cookieList(args, context) {
+      let host = context.environment.document.location.host;
+      if (host == null) {
+        throw new Error(gcli.lookup("cookieListOutNonePage"));
+      }
+
+      let enm = cookieMgr.getCookiesFromHost(host);
+
+      let cookies = [];
+      while (enm.hasMoreElements()) {
+        let cookie = enm.getNext().QueryInterface(Ci.nsICookie);
+        if (isCookieAtHost(cookie, host)) {
+          cookies.push({
+            host: cookie.host,
+            name: cookie.name,
+            value: cookie.value,
+            path: cookie.path,
+            expires: cookie.expires,
+            secure: cookie.secure,
+            httpOnly: cookie.httpOnly,
+            sameDomain: cookie.sameDomain
+          });
+        }
+      }
+
+      return cookies;
+    }
+  });
+
+  /**
+   * 'cookie remove' command
+   */
   gcli.addCommand({
     name: "cookie remove",
     description: gcli.lookup("cookieRemoveDesc"),
     manual: gcli.lookup("cookieRemoveManual"),
     params: [
       {
-        name: "key",
+        name: "name",
         type: "string",
         description: gcli.lookup("cookieRemoveKeyDesc"),
       }
     ],
     exec: function Command_cookieRemove(args, context) {
-      let document = context.environment.contentDocument;
-      let expDate = new Date();
-      expDate.setDate(expDate.getDate() - 1);
-      document.cookie = escape(args.key) + "=; expires=" + expDate.toGMTString();
+      let host = context.environment.document.location.host;
+      let enm = cookieMgr.getCookiesFromHost(host);
+
+      let cookies = [];
+      while (enm.hasMoreElements()) {
+        let cookie = enm.getNext().QueryInterface(Components.interfaces.nsICookie);
+        if (isCookieAtHost(cookie, host)) {
+          if (cookie.name == args.name) {
+            cookieMgr.remove(cookie.host, cookie.name, cookie.path, false);
+          }
+        }
+      }
     }
   });
 
   /**
-  * 'cookie set' command
-  */
+   * 'cookie set' command
+   */
   gcli.addCommand({
     name: "cookie set",
     description: gcli.lookup("cookieSetDesc"),
     manual: gcli.lookup("cookieSetManual"),
     params: [
       {
-        name: "key",
+        name: "name",
         type: "string",
         description: gcli.lookup("cookieSetKeyDesc")
       },
       {
         name: "value",
         type: "string",
         description: gcli.lookup("cookieSetValueDesc")
       },
@@ -905,69 +986,90 @@ XPCOMUtils.defineLazyModuleGetter(this, 
             type: "string",
             defaultValue: null,
             description: gcli.lookup("cookieSetDomainDesc")
           },
           {
             name: "secure",
             type: "boolean",
             description: gcli.lookup("cookieSetSecureDesc")
-          }
+          },
+          {
+            name: "httpOnly",
+            type: "boolean",
+            description: gcli.lookup("cookieSetHttpOnlyDesc")
+          },
+          {
+            name: "session",
+            type: "boolean",
+            description: gcli.lookup("cookieSetSessionDesc")
+          },
+          {
+            name: "expires",
+            type: "string",
+            defaultValue: "Jan 17, 2038",
+            description: gcli.lookup("cookieSetExpiresDesc")
+          },
         ]
       }
     ],
     exec: function Command_cookieSet(args, context) {
-      let document = context.environment.contentDocument;
+      let host = context.environment.document.location.host;
+      let time = Date.parse(args.expires) / 1000;
 
-      document.cookie = escape(args.key) + "=" + escape(args.value) +
-              (args.domain ? "; domain=" + args.domain : "") +
-              (args.path ? "; path=" + args.path : "") +
-              (args.secure ? "; secure" : ""); 
+      cookieMgr.add(args.domain ? "." + args.domain : host,
+                    args.path ? args.path : "/",
+                    args.name,
+                    args.value,
+                    args.secure,
+                    args.httpOnly,
+                    args.session,
+                    time);
     }
   });
 
   /**
-  * Helper to find the 'data-command' attribute and call some action on it.
-  * @see |updateCommand()| and |executeCommand()|
-  */
+   * Helper to find the 'data-command' attribute and call some action on it.
+   * @see |updateCommand()| and |executeCommand()|
+   */
   function withCommand(element, action) {
-    var command = element.getAttribute("data-command");
+    let command = element.getAttribute("data-command");
     if (!command) {
       command = element.querySelector("*[data-command]")
               .getAttribute("data-command");
     }
 
     if (command) {
       action(command);
     }
     else {
       console.warn("Missing data-command for " + util.findCssSelector(element));
     }
   }
 
   /**
-  * Create a handler to update the requisition to contain the text held in the
-  * first matching data-command attribute under the currentTarget of the event.
-  * @param context Either a Requisition or an ExecutionContext or another object
-  * that contains an |update()| function that follows a similar contract.
-  */
+   * Create a handler to update the requisition to contain the text held in the
+   * first matching data-command attribute under the currentTarget of the event.
+   * @param context Either a Requisition or an ExecutionContext or another object
+   * that contains an |update()| function that follows a similar contract.
+   */
   function createUpdateHandler(context) {
     return function(ev) {
       withCommand(ev.currentTarget, function(command) {
         context.update(command);
       });
     }
   }
 
   /**
-  * Create a handler to execute the text held in the data-command attribute
-  * under the currentTarget of the event.
-  * @param context Either a Requisition or an ExecutionContext or another object
-  * that contains an |update()| function that follows a similar contract.
-  */
+   * Create a handler to execute the text held in the data-command attribute
+   * under the currentTarget of the event.
+   * @param context Either a Requisition or an ExecutionContext or another object
+   * that contains an |update()| function that follows a similar contract.
+   */
   function createExecuteHandler(context) {
     return function(ev) {
       withCommand(ev.currentTarget, function(command) {
         context.exec({
           visible: true,
           typed: command
         });
       });
--- a/browser/devtools/commandline/commandline.css
+++ b/browser/devtools/commandline/commandline.css
@@ -29,11 +29,21 @@
 }
 
 .gcli-menu-name,
 .gcli-out-shortcut,
 .gcli-help-synopsis {
   direction: ltr;
 }
 
+.gcli-cookielist-list {
+  list-style-type: none;
+  padding-left: 0;
+}
+
+.gcli-cookielist-detail {
+  padding-left: 20px;
+  padding-bottom: 10px;
+}
+
 .gcli-row-out .nowrap {
   white-space: nowrap;
 }
--- a/browser/devtools/commandline/gcli.jsm
+++ b/browser/devtools/commandline/gcli.jsm
@@ -6655,18 +6655,27 @@ Requisition.prototype.exec = function(op
     args: args,
     typed: typed,
     canonical: this.toCanonicalString(),
     hidden: hidden
   });
 
   this.commandOutputManager.onOutput({ output: output });
 
-  var onDone = function(data) { output.complete(data, false); };
-  var onError = function(error) { output.complete(error, true); };
+  var onDone = function(data) {
+    output.complete(data, false);
+  };
+
+  var onError = function(ex) {
+    output.complete({
+      isTypedData: true,
+      data: ex,
+      type: "exception"
+    }, true);
+  };
 
   try {
     var reply = command.exec(args, this.context);
 
     this._then(reply, onDone, onError);
   }
   catch (ex) {
     console.error(ex);
@@ -9544,16 +9553,30 @@ var stringDomConverter = {
   exec: function(data, context) {
     var node = util.createElement(context.document, 'p');
     node.textContent = data;
     return node;
   }
 };
 
 /**
+ * Convert a string to a DOM element
+ */
+var exceptionDomConverter = {
+  from: 'exception',
+  to: 'dom',
+  exec: function(ex, context) {
+    var node = util.createElement(context.document, 'p');
+    node.className = "gcli-exception";
+    node.textContent = ex;
+    return node;
+  }
+};
+
+/**
  * Create a new converter by using 2 converters, one after the other
  */
 function getChainConverter(first, second) {
   if (first.to !== second.from) {
     throw new Error('Chain convert impossible: ' + first.to + '!=' + second.from);
   }
   return {
     from: first.from,
@@ -9650,16 +9673,17 @@ exports.convert = function(data, from, t
     return data;
   }
   return Promise.resolve(getConverter(from, to).exec(data, context));
 };
 
 exports.addConverter(viewDomConverter);
 exports.addConverter(terminalDomConverter);
 exports.addConverter(stringDomConverter);
+exports.addConverter(exceptionDomConverter);
 
 
 });
 define("text!gcli/commands/pref_set_check.html", [], "<div>\n" +
   "  <p><strong>${l10n.prefSetCheckHeading}</strong></p>\n" +
   "  <p>${l10n.prefSetCheckBody}</p>\n" +
   "  <button onclick=\"${activate}\">${l10n.prefSetCheckGo}</button>\n" +
   "</div>\n" +
--- a/browser/devtools/commandline/test/browser_cmd_cookie.js
+++ b/browser/devtools/commandline/test/browser_cmd_cookie.js
@@ -34,26 +34,26 @@ function test() {
           markup: 'VVVVVVVVVVV',
           status: 'VALID'
         },
       },
       {
         setup: 'cookie remove',
         check: {
           input:  'cookie remove',
-          hints:               ' <key>',
+          hints:               ' <name>',
           markup: 'VVVVVVVVVVVVV',
           status: 'ERROR'
         },
       },
       {
         setup: 'cookie set',
         check: {
           input:  'cookie set',
-          hints:            ' <key> <value> [options]',
+          hints:            ' <name> <value> [options]',
           markup: 'VVVVVVVVVV',
           status: 'ERROR'
         },
       },
       {
         setup: 'cookie set fruit',
         check: {
           input:  'cookie set fruit',
@@ -65,46 +65,58 @@ function test() {
       {
         setup: 'cookie set fruit ban',
         check: {
           input:  'cookie set fruit ban',
           hints:                      ' [options]',
           markup: 'VVVVVVVVVVVVVVVVVVVV',
           status: 'VALID',
           args: {
-            key: { value: 'fruit' },
+            name: { value: 'fruit' },
             value: { value: 'ban' },
             secure: { value: false },
           }
         },
       },
       {
+        setup: "cookie list",
+        exec: {
+          output: 'No cookies found for host'
+        }
+      },
+      {
         setup: "cookie set fruit banana",
         check: {
           args: {
-            key: { value: 'fruit' },
+            name: { value: 'fruit' },
             value: { value: 'banana' },
           }
         },
         exec: {
           output: ""
         }
       },
       {
         setup: "cookie list",
         exec: {
-          // output: /Key/
+          output: [ /fruit=banana/, /Expires:/, /Edit/ ]
         }
       },
       {
         setup: "cookie remove fruit",
         check: {
           args: {
-            key: { value: 'fruit' },
+            name: { value: 'fruit' },
           }
         },
         exec: {
           output: ""
         }
       },
+      {
+        setup: "cookie list",
+        exec: {
+          output: 'No cookies found for host'
+        }
+      },
     ]);
   }).then(finish);
 }
--- a/browser/locales/en-US/chrome/browser/devtools/gclicommands.properties
+++ b/browser/locales/en-US/chrome/browser/devtools/gclicommands.properties
@@ -731,27 +731,42 @@ cookieManual=Commands to list, create, d
 # 'cookie list' command. This string is designed to be shown in a menu
 # alongside the command name, which is why it should be as short as possible.
 cookieListDesc=Display cookies
 
 # LOCALIZATION NOTE (cookieListManual) A fuller description of the 'cookie list'
 # command, displayed when the user asks for help on what it does.
 cookieListManual=Display a list of the cookies relevant to the current page.
 
-# LOCALIZATION NOTE (cookieListOutKey) A heading used in the output from the
-# 'cookie list' command above a list of cookie keys
-cookieListOutKey=Key
+# LOCALIZATION NOTE (cookieListOutHost,cookieListOutPath,cookieListOutExpires,cookieListOutAttributes):
+# The 'cookie list' command has a number of headings for cookie properties.
+# Particular care should be taken in translating these strings as they have
+# references to names in the cookies spec.
+cookieListOutHost=Host:
+cookieListOutPath=Path:
+cookieListOutExpires=Expires:
+cookieListOutAttributes=Attributes:
+
+# LOCALIZATION NOTE (cookieListOutNone) The output of the 'cookie list' command
+# uses this string when no cookie attributes (like httpOnly, secure, etc) apply
+cookieListOutNone=None
 
-# LOCALIZATION NOTE (cookieListOutValue) A heading used in the output from the
-# 'cookie list' command above a list of cookie values
-cookieListOutValue=Value
+# LOCALIZATION NOTE (cookieListOutSession) The output of the 'cookie list'
+# command uses this string to describe a cookie with an expiry value of '0'
+# that is to say it is a session cookie
+cookieListOutSession=At browser exit (session)
 
-# LOCALIZATION NOTE (cookieListOutActions) A heading used in the output from the
-# 'cookie list' command above a list of actions to take on cookies
-cookieListOutActions=Actions
+# LOCALIZATION NOTE (cookieListOutNonePage) The output of the 'cookie list'
+# command uses this string for pages like 'about:blank' which can't contain
+# cookies
+cookieListOutNonePage=No cookies found for this page
+
+# LOCALIZATION NOTE (cookieListOutNoneHost) The output of the 'cookie list'
+# command uses this string when there are no cookies on a given web page
+cookieListOutNoneHost=No cookies found for host %1$S
 
 # LOCALIZATION NOTE (cookieListOutEdit) A title used in the output from the
 # 'cookie list' command on a button which can be used to edit cookie values
 cookieListOutEdit=Edit
 
 # LOCALIZATION NOTE (cookieListOutRemove) A title used in the output from the
 # 'cookie list' command on a button which can be used to remove cookies
 cookieListOutRemove=Remove
@@ -803,16 +818,31 @@ cookieSetPathDesc=The path of the cookie
 # when the user is using this command.
 cookieSetDomainDesc=The domain of the cookie to set
 
 # LOCALIZATION NOTE (cookieSetSecureDesc) A very short string to describe the
 # 'secure' parameter to the 'cookie set' command, which is displayed in a dialog
 # when the user is using this command.
 cookieSetSecureDesc=Only transmitted over https
 
+# LOCALIZATION NOTE (cookieSetHttpOnlyDesc) A very short string to describe the
+# 'httpOnly' parameter to the 'cookie set' command, which is displayed in a dialog
+# when the user is using this command.
+cookieSetHttpOnlyDesc=Not accessible from client side script
+
+# LOCALIZATION NOTE (cookieSetSessionDesc) A very short string to describe the
+# 'session' parameter to the 'cookie set' command, which is displayed in a dialog
+# when the user is using this command.
+cookieSetSessionDesc=Only valid for the lifetime of the browser session
+
+# LOCALIZATION NOTE (cookieSetExpiresDesc) A very short string to describe the
+# 'expires' parameter to the 'cookie set' command, which is displayed in a dialog
+# when the user is using this command.
+cookieSetExpiresDesc=The expiry date of the cookie (quoted RFC2822 or ISO 8601 date)
+
 # LOCALIZATION NOTE (jsbDesc) A very short description of the
 # 'jsb' command. This string is designed to be shown in a menu
 # alongside the command name, which is why it should be as short as possible.
 jsbDesc=JavaScript beautifier
 
 # LOCALIZATION NOTE (jsbUrlDesc) A very short description of the
 # 'jsb <url>' parameter. This string is designed to be shown in a menu
 # alongside the command name, which is why it should be as short as possible.
--- a/toolkit/devtools/Console.jsm
+++ b/toolkit/devtools/Console.jsm
@@ -69,16 +69,22 @@ function fmt(aStr, aMaxLen, aMinLen, aOp
  * Object.toString gives: "[object ?????]"; we want the "?????".
  *
  * @param {object} aObj
  *        The object from which to extract the constructor name
  * @return {string}
  *        The constructor name
  */
 function getCtorName(aObj) {
+  if (aObj === null) {
+    return "null";
+  }
+  if (aObj === undefined) {
+    return "undefined";
+  }
   if (aObj.constructor && aObj.constructor.name) {
     return aObj.constructor.name;
   }
   // If that fails, use Objects toString which sometimes gives something
   // better than 'Object', and at least defaults to Object if nothing better
   return Object.prototype.toString.call(aObj).slice(8, -1);
 }