1 files changed, 25 insertions, 57 deletions

diff --git a/services/sync/modules/addonutils.js b/services/sync/modules/addonutils.js
index 95da6be0a..3332f4cfc 100644
--- a/services/sync/modules/addonutils.js
+++ b/services/sync/modules/addonutils.js
@@ -38,10 +38,21 @@ AddonUtilsInternal.prototype = {
    *        Function to be called with result of operation.
    */
   getInstallFromSearchResult:
-    function getInstallFromSearchResult(addon, cb) {
+    function getInstallFromSearchResult(addon, cb, requireSecureURI=true) {
 
     this._log.debug("Obtaining install for " + addon.id);
 
+    // Verify that the source URI uses TLS. We don't allow installs from
+    // insecure sources for security reasons. The Addon Manager ensures that
+    // cert validation, etc is performed.
+    if (requireSecureURI) {
+      let scheme = addon.sourceURI.scheme;
+      if (scheme != "https") {
+        cb(new Error("Insecure source URI scheme: " + scheme), addon.install);
+        return;
+      }
+    }
+
     // We should theoretically be able to obtain (and use) addon.install if
     // it is available. However, the addon.sourceURI rewriting won't be
     // reflected in the AddonInstall, so we can't use it. If we ever get rid
@@ -69,6 +80,8 @@ AddonUtilsInternal.prototype = {
    *   syncGUID - Sync GUID to use for the new add-on.
    *   enabled - Boolean indicating whether the add-on should be enabled upon
    *             install.
+   *   requireSecureURI - Boolean indicating whether to require a secure
+   *     URI to install from. This defaults to true.
    *
    * When complete it calls a callback with 2 arguments, error and result.
    *
@@ -92,6 +105,10 @@ AddonUtilsInternal.prototype = {
     function installAddonFromSearchResult(addon, options, cb) {
     this._log.info("Trying to install add-on from search result: " + addon.id);
 
+    if (options.requireSecureURI === undefined) {
+      options.requireSecureURI = true;
+    }
+
     this.getInstallFromSearchResult(addon, function onResult(error, install) {
       if (error) {
         cb(error, null);
@@ -147,10 +164,10 @@ AddonUtilsInternal.prototype = {
         install.install();
       }
       catch (ex) {
-        this._log.error("Error installing add-on", ex);
+        this._log.error("Error installing add-on: ", ex);
         cb(ex, null);
       }
-    }.bind(this));
+    }.bind(this), options.requireSecureURI);
   },
 
   /**
@@ -244,7 +261,6 @@ AddonUtilsInternal.prototype = {
           installedIDs: [],
           installs:     [],
           addons:       [],
-          skipped:      [],
           errors:       []
         };
 
@@ -283,20 +299,14 @@ AddonUtilsInternal.prototype = {
         // ideally send proper URLs, but this solution was deemed too
         // complicated at the time the functionality was implemented.
         for (let addon of addons) {
-          // Find the specified options for this addon.
-          let options;
-          for (let install of installs) {
-            if (install.id == addon.id) {
-              options = install;
-              break;
-            }
-          }
-          if (!this.canInstallAddon(addon, options)) {
-            ourResult.skipped.push(addon.id);
+          // sourceURI presence isn't enforced by AddonRepository. So, we skip
+          // add-ons without a sourceURI.
+          if (!addon.sourceURI) {
+            this._log.info("Skipping install of add-on because missing " +
+                           "sourceURI: " + addon.id);
             continue;
           }
 
-          // We can go ahead and attempt to install it.
           toInstall.push(addon);
 
           // We should always be able to QI the nsIURI to nsIURL. If not, we
@@ -353,48 +363,6 @@ AddonUtilsInternal.prototype = {
   },
 
   /**
-   * Returns true if we are able to install the specified addon, false
-   * otherwise. It is expected that this will log the reason if it returns
-   * false.
-   *
-   * @param addon
-   *        (Addon) Add-on instance to check.
-   * @param options
-   *        (object) The options specified for this addon. See installAddons()
-   *        for the valid elements.
-   */
-  canInstallAddon(addon, options) {
-    // sourceURI presence isn't enforced by AddonRepository. So, we skip
-    // add-ons without a sourceURI.
-    if (!addon.sourceURI) {
-      this._log.info("Skipping install of add-on because missing " +
-                     "sourceURI: " + addon.id);
-      return false;
-    }
-    // Verify that the source URI uses TLS. We don't allow installs from
-    // insecure sources for security reasons. The Addon Manager ensures
-    // that cert validation etc is performed.
-    // (We should also consider just dropping this entirely and calling
-    // XPIProvider.isInstallAllowed, but that has additional semantics we might
-    // need to think through...)
-    let requireSecureURI = true;
-    if (options && options.requireSecureURI !== undefined) {
-      requireSecureURI = options.requireSecureURI;
-    }
-
-    if (requireSecureURI) {
-      let scheme = addon.sourceURI.scheme;
-      if (scheme != "https") {
-        this._log.info(`Skipping install of add-on "${addon.id}" because sourceURI's scheme of "${scheme}" is not trusted`);
-        return false;
-      }
-    }
-    this._log.info(`Add-on "${addon.id}" is able to be installed`);
-    return true;
-  },
-
-
-  /**
    * Update the user disabled flag for an add-on.
    *
    * The supplied callback will be called when the operation is