Bug 584842 - nsIContentPrefService remoting. r=myk a=blocking-fennec=2.0b1

dom/interfaces/base/nsIContentPrefService.idl

@@ -82,6 +82,9 @@ interface nsIContentPrefService : nsISupports
    * to NULL in the database, as well as undefined (nsIDataType::VTYPE_VOID),
    * which means there is no record for this pref in the database.
    *
+   * This method can be called from content processes in electrolysis builds.
+   * We have a whitelist of values that can be read in such a way.
+   *
    * @param    aGroup      the group for which to get the pref, as an nsIURI
    *                       from which the hostname will be used, a string
    *                       (typically in the format of a hostname), or null 
@@ -101,6 +104,9 @@ interface nsIContentPrefService : nsISupports
   /**
    * Set a pref.
    *
+   * This method can be called from content processes in electrolysis builds.
+   * We have a whitelist of values that can be set in such a way.
+   *
    * @param    aGroup      the group for which to set the pref, as an nsIURI
    *                       from which the hostname will be used, a string
    *                       (typically in the format of a hostname), or null

toolkit/components/contentprefs/src/nsContentPrefService.js

@@ -43,7 +43,88 @@ const Cu = Components.utils;
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
+/**
+ * Remotes the service. All the remoting/electrolysis code is in here,
+ * so the regular service code below remains uncluttered and maintainable.
+ */
+function electrolify(service) {
+  // FIXME: For now, use the wrappedJSObject hack, until bug
+  //        593407 which will clean that up.
+  //        Note that we also use this in the xpcshell tests, separately.
+  service.wrappedJSObject = service;
+
+  var appInfo = Cc["@mozilla.org/xre/app-info;1"];
+  if (!appInfo || appInfo.getService(Ci.nsIXULRuntime).processType ==
+      Ci.nsIXULRuntime.PROCESS_TYPE_DEFAULT) {
+    // Parent process
+
+    // Setup listener for child messages. We don't need to call
+    // addMessageListener as the wakeup service will do that for us.
+    service.receiveMessage = function(aMessage) {
+      var json = aMessage.json;
+      // We have a whitelist for getting/setting. This is because
+      // there are potential privacy issues with a compromised
+      // content process checking the user's content preferences
+      // and using that to discover all the websites visited, etc.
+      // Also there are both potential race conditions (if two processes
+      // set more than one value in succession, and the values
+      // only make sense together), as well as security issues, if
+      // a compromised content process can send arbitrary setPref
+      // messages. The whitelist contains only those settings that
+      // are not at risk for either.
+      // We currently whitelist saving/reading the last directory of file
+      // uploads, which is so far the only need we have identified.
+      const NAME_WHITELIST = ["browser.upload.lastDir"];
+      if (NAME_WHITELIST.indexOf(json.name) == -1)
+        return { succeeded: false };
+
+      switch (aMessage.name) {
+        case "ContentPref:getPref":
+          return { succeeded: true,
+                   value: service.getPref(json.group, json.name, json.value) };
+
+        case "ContentPref:setPref":
+          service.setPref(json.group, json.name, json.value);
+          return { succeeded: true };
+      }
+    };
+  } else {
+    // Child process
+
+    service._dbInit = function(){}; // No local DB
+
+    service.messageManager = Cc["@mozilla.org/childprocessmessagemanager;1"].
+                             getService(Ci.nsISyncMessageSender);
+
+    // Child method remoting
+    [
+      ['getPref', ['group', 'name'], ['_parseGroupParam']],
+      ['setPref', ['group', 'name', 'value'], ['_parseGroupParam']],
+    ].forEach(function(data) {
+      var method = data[0];
+      var params = data[1];
+      var parsers = data[2];
+      service[method] = function __remoted__() {
+        var json = {};
+        for (var i = 0; i < params.length; i++) {
+          if (params[i]) {
+            json[params[i]] = arguments[i];
+            if (parsers[i])
+              json[params[i]] = this[parsers[i]](json[params[i]]);
+          }
+        }
+        var ret = service.messageManager.sendSyncMessage('ContentPref:' + method, json)[0];
+        if (!ret.succeeded)
+          throw "ContentPrefs remoting failed to pass whitelist";
+        return ret.value;
+      };
+    });
+  }
+}
+
 function ContentPrefService() {
+  electrolify(this);
+
   // If this throws an exception, it causes the getService call to fail,
   // but the next time a consumer tries to retrieve the service, we'll try
   // to initialize the database again, which might work if the failure
@@ -59,7 +140,8 @@ ContentPrefService.prototype = {
   // XPCOM Plumbing
 
   classID:          Components.ID("{e6a3f533-4ffa-4615-8eb4-d4e72d883fa7}"),
-  QueryInterface:   XPCOMUtils.generateQI([Ci.nsIContentPrefService]),
+  QueryInterface:   XPCOMUtils.generateQI([Ci.nsIContentPrefService,
+                                           Ci.nsIFrameMessageListener]),
 
 
   //**************************************************************************//
@@ -137,15 +219,10 @@ ContentPrefService.prototype = {
       throw Components.Exception("aName cannot be null or an empty string",
                                  Cr.NS_ERROR_ILLEGAL_VALUE);
 
-    if (aGroup == null)
+    var group = this._parseGroupParam(aGroup);
+    if (group == null)
       return this._selectGlobalPref(aName, aCallback);
-    if (aGroup.constructor.name == "String")
-      return this._selectPref(aGroup.toString(), aName, aCallback);
-    if (aGroup instanceof Ci.nsIURI)
-      return this._selectPref(this.grouper.group(aGroup), aName, aCallback);
-
-    throw Components.Exception("aGroup is not a string, nsIURI or null",
-                               Cr.NS_ERROR_ILLEGAL_VALUE);
+    return this._selectPref(group, aName, aCallback);
   },
 
   setPref: function ContentPrefService_setPref(aGroup, aName, aValue) {
@@ -168,26 +245,15 @@ ContentPrefService.prototype = {
     }
 
     var settingID = this._selectSettingID(aName) || this._insertSetting(aName);
-    var group, groupID, prefID;
-    if (aGroup == null) {
-      group = null;
+    var group = this._parseGroupParam(aGroup);
+    var groupID, prefID;
+    if (group == null) {
       groupID = null;
       prefID = this._selectGlobalPrefID(settingID);
     }
-    else if (aGroup.constructor.name == "String") {
-      group = aGroup.toString();
-      groupID = this._selectGroupID(group) || this._insertGroup(group);
-      prefID = this._selectPrefID(groupID, settingID);
-    }
-    else if (aGroup instanceof Ci.nsIURI) {
-      group = this.grouper.group(aGroup);
-      groupID = this._selectGroupID(group) || this._insertGroup(group);
-      prefID = this._selectPrefID(groupID, settingID);
-    }
     else {
-      // Should never get here, due to earlier getPref call
-      throw Components.Exception("aGroup is not a string, nsIURI or null",
-                                 Cr.NS_ERROR_ILLEGAL_VALUE);
+      groupID = this._selectGroupID(group) || this._insertGroup(group);
+      prefID = this._selectPrefID(groupID, settingID);
     }
 
     // Update the existing record, if any, or create a new one.
@@ -217,27 +283,17 @@ ContentPrefService.prototype = {
     if (!this.hasPref(aGroup, aName))
       return;
 
+
     var settingID = this._selectSettingID(aName);
-    var group, groupID, prefID;
-    if (aGroup == null) {
-      group = null;
+    var group = this._parseGroupParam(aGroup);
+    var groupID, prefID;
+    if (group == null) {
       groupID = null;
       prefID = this._selectGlobalPrefID(settingID);
     }
-    else if (aGroup.constructor.name == "String") {
-      group = aGroup.toString();
-      groupID = this._selectGroupID(group);
-      prefID = this._selectPrefID(groupID, settingID);
-    }
-    else if (aGroup instanceof Ci.nsIURI) {
-      group = this.grouper.group(aGroup);
-      groupID = this._selectGroupID(group);
-      prefID = this._selectPrefID(groupID, settingID);
-    }
     else {
-      // Should never get here, due to earlier hasPref call
-      throw Components.Exception("aGroup is not a string, nsIURI or null",
-                                 Cr.NS_ERROR_ILLEGAL_VALUE);
+      groupID = this._selectGroupID(group);
+      prefID = this._selectPrefID(groupID, settingID);
     }
 
     this._deletePref(prefID);
@@ -312,18 +368,10 @@ ContentPrefService.prototype = {
   },
 
   getPrefs: function ContentPrefService_getPrefs(aGroup) {
-    if (aGroup == null)
+    var group = this._parseGroupParam(aGroup);
+    if (group == null)
       return this._selectGlobalPrefs();
-    if (aGroup.constructor.name == "String") {
-      group = aGroup.toString();
-      return this._selectPrefs(group);
-    }
-    if (aGroup instanceof Ci.nsIURI) {
-      var group = this.grouper.group(aGroup);
-      return this._selectPrefs(group);
-    }
-    throw Components.Exception("aGroup is not a string, nsIURI or null",
-                               Cr.NS_ERROR_ILLEGAL_VALUE);
+    return this._selectPrefs(group);
   },
 
   getPrefsByName: function ContentPrefService_getPrefsByName(aName) {
@@ -1011,8 +1059,19 @@ ContentPrefService.prototype = {
 
   _dbMigrate2To3: function ContentPrefService__dbMigrate2To3(aDBConnection) {
     this._dbCreateIndices(aDBConnection);
-  }
+  },
 
+  _parseGroupParam: function ContentPrefService__parseGroupParam(aGroup) {
+    if (aGroup == null)
+      return null;
+    if (aGroup.constructor.name == "String")
+      return aGroup.toString();
+    if (aGroup instanceof Ci.nsIURI)
+      return this.grouper.group(aGroup);
+
+    throw Components.Exception("aGroup is not a string, nsIURI or null",
+                               Cr.NS_ERROR_ILLEGAL_VALUE);
+  },
 };
 
 

toolkit/components/contentprefs/src/nsContentPrefService.manifest

@@ -2,3 +2,5 @@ component {e6a3f533-4ffa-4615-8eb4-d4e72d883fa7} nsContentPrefService.js
 contract @mozilla.org/content-pref/service;1 {e6a3f533-4ffa-4615-8eb4-d4e72d883fa7}
 component {8df290ae-dcaa-4c11-98a5-2429a4dc97bb} nsContentPrefService.js
 contract @mozilla.org/content-pref/hostname-grouper;1 {8df290ae-dcaa-4c11-98a5-2429a4dc97bb}
+category wakeup-request nsContentPrefService @mozilla.org/content-pref/service;1,nsIContentPrefService,getService,ContentPref:getPref,ContentPref:setPref
+

toolkit/components/contentprefs/tests/Makefile.in

@@ -48,6 +48,14 @@ MODULE		= test_toolkit_contentprefs
 
 ifdef MOZ_PHOENIX
 XPCSHELL_TESTS = unit
+
+ifdef MOZ_IPC
+# FIXME/bug 575918: out-of-process xpcshell is broken on OS X
+ifneq ($(OS_ARCH),Darwin)
+XPCSHELL_TESTS += unit_ipc
+endif
+endif
+
 endif
 
 include $(topsrcdir)/config/rules.mk

toolkit/components/contentprefs/tests/unit/head_contentPrefs.js

@@ -166,8 +166,21 @@ var ContentPrefTest = {
 
 ContentPrefTest.deleteDatabase();
 
+function inChildProcess() {
+  var appInfo = Cc["@mozilla.org/xre/app-info;1"];
+  if (!appInfo || appInfo.getService(Ci.nsIXULRuntime).processType ==
+      Ci.nsIXULRuntime.PROCESS_TYPE_DEFAULT) {
+    return false;
+  }
+  return true;
+}
+
 // Turn on logging for the content preferences service so we can troubleshoot
-// problems with the tests.
-var prefBranch = Cc["@mozilla.org/preferences-service;1"].
-                 getService(Ci.nsIPrefBranch);
-prefBranch.setBoolPref("browser.preferences.content.log", true);
+// problems with the tests. Note that we cannot do this in a child process
+// without crashing (but we don't need it anyhow)
+if (!inChildProcess()) {
+  var prefBranch = Cc["@mozilla.org/preferences-service;1"].
+                   getService(Ci.nsIPrefBranch);
+  prefBranch.setBoolPref("browser.preferences.content.log", true);
+}
+

toolkit/components/contentprefs/tests/unit_ipc/contentPrefs_childipc.js

@@ -0,0 +1,39 @@
+
+function run_test() {
+  do_check_true(inChildProcess(), "test harness should never call us directly");
+
+  var cps = Cc["@mozilla.org/content-pref/service;1"].
+            createInstance(Ci.nsIContentPrefService);
+
+  // Cannot get general values
+  try {
+    cps.getPref("group", "name")
+    do_check_false(true, "Must have thrown exception on getting general value");
+  }
+  catch(e) { }
+
+  // Cannot set general values
+  try {
+    cps.setPref("group", "name", "someValue2");
+    do_check_false(true, "Must have thrown exception on setting general value");
+  }
+  catch(e) { }
+
+  // Can set&get whitelisted values
+  cps.setPref("group", "browser.upload.lastDir", "childValue");
+  do_check_eq(cps.getPref("group", "browser.upload.lastDir"), "childValue");
+
+  // Test sending URI
+  var ioSvc = Cc["@mozilla.org/network/io-service;1"].
+              getService(Ci.nsIIOService);
+  var uri = ioSvc.newURI("http://mozilla.org", null, null);
+  cps.setPref(uri, "browser.upload.lastDir", "childValue2");
+  do_check_eq(cps.getPref(uri, "browser.upload.lastDir"), "childValue2");
+
+  // Previous value
+  do_check_eq(cps.getPref("group", "browser.upload.lastDir"), "childValue");
+
+  // Tell parent to finish and clean up
+  cps.wrappedJSObject.messageManager.sendSyncMessage('ContentPref:QUIT');
+}
+

toolkit/components/contentprefs/tests/unit_ipc/head_contentPrefs.js

@@ -0,0 +1,3 @@
+
+load("../unit/head_contentPrefs.js");
+

toolkit/components/contentprefs/tests/unit_ipc/tail_contentPrefs.js

@@ -0,0 +1,3 @@
+
+load("../unit/tail_contentPrefs.js");
+

toolkit/components/contentprefs/tests/unit_ipc/test_contentPrefs_parentipc.js

@@ -0,0 +1,88 @@
+
+function run_test() {
+  // Check received messages
+
+  var cps = Cc["@mozilla.org/content-pref/service;1"].
+            createInstance(Ci.nsIContentPrefService).
+            wrappedJSObject;
+
+  var messageHandler = cps;
+  // FIXME: For now, use the wrappedJSObject hack, until bug
+  //        593407 which will clean that up. After that, use
+  //        the commented out line below it.
+  messageHandler = cps.wrappedJSObject;
+  //messageHandler = cps.QueryInterface(Ci.nsIFrameMessageListener);
+
+  // Cannot get values
+  do_check_false(messageHandler.receiveMessage({
+    name: "ContentPref:getPref",
+    json: { group: 'group2', name: 'name' } }).succeeded);
+
+  // Cannot set general values
+  messageHandler.receiveMessage({ name: "ContentPref:setPref",
+    json: { group: 'group2', name: 'name', value: 'someValue' } });
+  do_check_eq(cps.getPref('group', 'name'), undefined);
+
+  // Can set whitelisted values
+  do_check_true(messageHandler.receiveMessage({ name: "ContentPref:setPref",
+    json: { group: 'group2', name: 'browser.upload.lastDir',
+            value: 'someValue' } }).succeeded);
+  do_check_eq(cps.getPref('group2', 'browser.upload.lastDir'), 'someValue');
+
+  // Prepare for child tests
+
+  // Manually listen to messages - the wakeup manager should do this
+  // for us, but it doesn't run in xpcshell tests.
+  var messageProxy = {
+    receiveMessage: function(aMessage) {
+      if (aMessage.name == 'ContentPref:QUIT') {
+        // Undo mock storage.
+        delete cps._mockStorage;
+        delete cps._messageProxy;
+        cps.setPref = cps.old_setPref;
+        cps.getPref = cps.old_getPref;
+        cps._dbInit = cps.old__dbInit;
+        // Unlisten to messages
+        mM.removeMessageListener("ContentPref:setPref", messageProxy);
+        mM.removeMessageListener("ContentPref:getPref", messageProxy);
+        mM.removeMessageListener("ContentPref:QUIT", messageProxy);
+        do_test_finished();
+        return true;
+      } else {
+        return messageHandler.receiveMessage(aMessage);
+      }
+    },
+  };
+
+  var mM = Cc["@mozilla.org/parentprocessmessagemanager;1"].
+           getService(Ci.nsIFrameMessageManager);
+  mM.addMessageListener("ContentPref:setPref", messageProxy);
+  mM.addMessageListener("ContentPref:getPref", messageProxy);
+  mM.addMessageListener("ContentPref:QUIT", messageProxy);
+
+  // Mock storage. This is necessary because
+  // the IPC xpcshell setup doesn't do well with the normal storage
+  // engine.
+
+  cps = cps.wrappedJSObject;
+  cps._mockStorage = {};
+
+  cps.old_setPref = cps.setPref;
+  cps.setPref = function(aGroup, aName, aValue) {
+    this._mockStorage[aGroup+':'+aName] = aValue;
+  }
+
+  cps.old_getPref = cps.getPref;
+  cps.getPref = function(aGroup, aName) {
+    return this._mockStorage[aGroup+':'+aName];
+  }
+
+  cps.old__dbInit = cps._dbInit;
+  cps._dbInit = function(){};
+
+  cps._messageProxy = messageProxy; // keep it alive
+  do_test_pending();
+
+  run_test_in_child("contentPrefs_childipc.js");
+}
+