From 989a1447a25fb72535acd50df3e9a26bb1cfd95b Mon Sep 17 00:00:00 2001
From: Gregory Szorc <gps@mozilla.com>
Date: Tue, 7 Feb 2012 14:14:41 -0800
Subject: [PATCH] Bug 725083 - Handle undefined sourceURI when installing
 add-ons; r=rnewman

---
 services/sync/modules/engines/addons.js       | 26 +++++++++++++++---
 .../sync/tests/unit/missing-sourceuri.xml     | 27 +++++++++++++++++++
 services/sync/tests/unit/test_addons_store.js | 23 ++++++++++++++++
 3 files changed, 73 insertions(+), 3 deletions(-)
 create mode 100644 services/sync/tests/unit/missing-sourceuri.xml

diff --git a/services/sync/modules/engines/addons.js b/services/sync/modules/engines/addons.js
index f855f378c2c..7ebd7addabb 100644
--- a/services/sync/modules/engines/addons.js
+++ b/services/sync/modules/engines/addons.js
@@ -935,6 +935,7 @@ AddonsStore.prototype = {
           return;
         }
 
+        let expectedInstallCount = 0;
         let finishedCount = 0;
         let installCallback = function installCallback(error, result) {
           finishedCount++;
@@ -947,7 +948,7 @@ AddonsStore.prototype = {
             ourResult.addons.push(result.addon);
           }
 
-          if (finishedCount >= addonsLength) {
+          if (finishedCount >= expectedInstallCount) {
             if (ourResult.errors.length > 0) {
               cb(new Error("1 or more add-ons failed to install"), ourResult);
             } else {
@@ -956,12 +957,24 @@ AddonsStore.prototype = {
           }
         }.bind(this);
 
+        let toInstall = [];
+
         // Rewrite the "src" query string parameter of the source URI to note
         // that the add-on was installed by Sync and not something else so
         // server-side metrics aren't skewed (bug 708134). The server should
         // ideally send proper URLs, but this solution was deemed too
         // complicated at the time the functionality was implemented.
         for each (let addon in addons) {
+          // 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;
+          }
+
+          toInstall.push(addon);
+
           // We should always be able to QI the nsIURI to nsIURL. If not, we
           // still try to install the add-on, but we don't rewrite the URL,
           // potentially skewing metrics.
@@ -986,10 +999,17 @@ AddonsStore.prototype = {
           addon.sourceURI.query = params.join("&");
         }
 
+        expectedInstallCount = toInstall.length;
+
+        if (!expectedInstallCount) {
+          cb(null, ourResult);
+          return;
+        }
+
         // Start all the installs asynchronously. They will report back to us
         // as they finish, eventually triggering the global callback.
-        for (let i = 0; i < addonsLength; i++) {
-          this.installAddonFromSearchResult(addons[i], installCallback);
+        for each (let addon in toInstall) {
+          this.installAddonFromSearchResult(addon, installCallback);
         }
 
       }.bind(this),
diff --git a/services/sync/tests/unit/missing-sourceuri.xml b/services/sync/tests/unit/missing-sourceuri.xml
new file mode 100644
index 00000000000..dbc83e17fe3
--- /dev/null
+++ b/services/sync/tests/unit/missing-sourceuri.xml
@@ -0,0 +1,27 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<searchresults total_results="1">
+  <addon id="5617">
+  <name>Restartless Test Extension</name>
+  <type id="1">Extension</type>
+  <guid>missing-sourceuri@tests.mozilla.org</guid>
+  <slug>missing-sourceuri</slug>
+  <version>1.0</version>
+
+  <compatible_applications><application>
+      <name>Firefox</name>
+      <application_id>1</application_id>
+      <min_version>3.6</min_version>
+      <max_version>*</max_version>
+      <appID>{3e3ba16c-1675-4e88-b9c8-afef81b3d2ef}</appID>
+    </application></compatible_applications>
+  <all_compatible_os><os>ALL</os></all_compatible_os>
+
+  <install os="ALL" size="485"></install>
+  <created epoch="1252903662">
+    2009-09-14T04:47:42Z
+  </created>
+  <last_updated epoch="1315255329">
+    2011-09-05T20:42:09Z
+  </last_updated>
+</addon>
+</searchresults>
diff --git a/services/sync/tests/unit/test_addons_store.js b/services/sync/tests/unit/test_addons_store.js
index 3ff084eeead..aaa4461e447 100644
--- a/services/sync/tests/unit/test_addons_store.js
+++ b/services/sync/tests/unit/test_addons_store.js
@@ -56,6 +56,9 @@ function createAndStartHTTPServer(port) {
     server.registerFile("/search/guid:rewrite%40tests.mozilla.org",
                         do_get_file("rewrite-search.xml"));
 
+    server.registerFile("/search/guid:missing-sourceuri%40tests.mozilla.org",
+                        do_get_file("missing-sourceuri.xml"));
+
     server.start(port);
 
     return server;
@@ -478,3 +481,23 @@ add_test(function test_source_uri_rewrite() {
   Svc.Prefs.reset("addons.ignoreRepositoryChecking");
   server.stop(run_next_test);
 });
+
+add_test(function test_handle_empty_source_uri() {
+  _("Ensure that search results without a sourceURI are properly ignored.");
+
+  Svc.Prefs.set("addons.ignoreRepositoryChecking", true);
+
+  let server = createAndStartHTTPServer(HTTP_PORT);
+
+  const ID = "missing-sourceuri@tests.mozilla.org";
+
+  let cb = Async.makeSpinningCallback();
+  store.installAddonsFromIDs([ID], cb);
+  let result = cb.wait();
+
+  do_check_true("installedIDs" in result);
+  do_check_eq(0, result.installedIDs.length);
+
+  Svc.Prefs.reset("addons.ignoreRepositoryChecking");
+  server.stop(run_next_test);
+});