Bug 1368232: Handle invalid paths in addonStartup.json when profiles are shared between OSes. r?rhelmer draft
authorKris Maglione <maglione.k@gmail.com>
Sat, 27 May 2017 12:06:42 -0700
changeset 585661 7bb274db0f4a1e6e78badcf225bc1963b6289b5c
parent 585260 880ae57e07a43ec0c8b08f93b8a1ae52d85bac41
child 585669 4a8034b8955df4cd4ea92c7c89645dac48ac24c6
push id61165
push usermaglione.k@gmail.com
push dateSat, 27 May 2017 19:07:11 +0000
reviewersrhelmer
bugs1368232
milestone55.0a1
Bug 1368232: Handle invalid paths in addonStartup.json when profiles are shared between OSes. r?rhelmer MozReview-Commit-ID: 1q0k3y4T7rq
toolkit/mozapps/extensions/AddonManagerStartup.cpp
toolkit/mozapps/extensions/AddonManagerStartup.h
toolkit/mozapps/extensions/test/xpcshell/test_addonStartup.js
toolkit/mozapps/extensions/test/xpcshell/xpcshell.ini
--- a/toolkit/mozapps/extensions/AddonManagerStartup.cpp
+++ b/toolkit/mozapps/extensions/AddonManagerStartup.cpp
@@ -365,61 +365,61 @@ public:
 
   bool Enabled() { return GetBool("enabled"); }
 
   bool ShimsEnabled() { return GetBool("enableShims"); }
 
   double LastModifiedTime() { return GetNumber("lastModifiedTime"); }
 
 
-  already_AddRefed<nsIFile> FullPath();
+  Result<nsCOMPtr<nsIFile>, nsresult> FullPath();
 
   NSLocationType LocationType();
 
-  bool UpdateLastModifiedTime();
+  Result<bool, nsresult> UpdateLastModifiedTime();
 
 
 private:
   nsString mId;
   InstallLocation& mLocation;
 };
 
-already_AddRefed<nsIFile>
+Result<nsCOMPtr<nsIFile>, nsresult>
 Addon::FullPath()
 {
   nsString path = Path();
 
   // First check for an absolute path, in case we have a proxy file.
   nsCOMPtr<nsIFile> file;
   if (NS_SUCCEEDED(NS_NewLocalFile(path, false, getter_AddRefs(file)))) {
-    return file.forget();
+    return Move(file);
   }
 
   // If not an absolute path, fall back to a relative path from the location.
-  NS_NewLocalFile(mLocation.Path(), false, getter_AddRefs(file));
-  MOZ_RELEASE_ASSERT(file);
+  NS_TRY(NS_NewLocalFile(mLocation.Path(), false, getter_AddRefs(file)));
 
-  file->AppendRelativePath(path);
-  return file.forget();
+  NS_TRY(file->AppendRelativePath(path));
+  return Move(file);
 }
 
 NSLocationType
 Addon::LocationType()
 {
   nsString type = GetString("type", "extension");
   if (type.LowerCaseEqualsLiteral("theme")) {
     return NS_SKIN_LOCATION;
   }
   return NS_EXTENSION_LOCATION;
 }
 
-bool
+Result<bool, nsresult>
 Addon::UpdateLastModifiedTime()
 {
-  nsCOMPtr<nsIFile> file = FullPath();
+  nsCOMPtr<nsIFile> file;
+  MOZ_TRY_VAR(file, FullPath());
 
   bool result;
   if (NS_FAILED(file->Exists(&result)) || !result) {
     return true;
   }
 
   PRTime time;
 
@@ -477,40 +477,40 @@ EnableShims(const nsAString& addonId)
 
   if (!interposition || !xpc::SetAddonInterposition(id, interposition)) {
     return;
   }
 
   Unused << xpc::AllowCPOWsInAddon(id, true);
 }
 
-void
+Result<Ok, nsresult>
 AddonManagerStartup::AddInstallLocation(Addon& addon)
 {
-  nsCOMPtr<nsIFile> file = addon.FullPath();
+  nsCOMPtr<nsIFile> file;
+  MOZ_TRY_VAR(file, addon.FullPath());
 
   nsString path;
-  if (NS_FAILED(file->GetPath(path))) {
-    return;
-  }
+  NS_TRY(file->GetPath(path));
 
   auto type = addon.LocationType();
 
   if (type == NS_SKIN_LOCATION) {
     mThemePaths.AppendElement(file);
   } else {
     mExtensionPaths.AppendElement(file);
   }
 
   if (StringTail(path, 4).LowerCaseEqualsLiteral(".xpi")) {
     XRE_AddJarManifestLocation(type, file);
   } else {
     nsCOMPtr<nsIFile> manifest = CloneAndAppend(file, "chrome.manifest");
     XRE_AddManifestLocation(type, manifest);
   }
+  return Ok();
 }
 
 nsresult
 AddonManagerStartup::ReadStartupData(JSContext* cx, JS::MutableHandleValue locations)
 {
   locations.set(JS::UndefinedValue());
 
   nsCOMPtr<nsIFile> file = CloneAndAppend(ProfileDir(), "addonStartup.json.lz4");
@@ -535,18 +535,22 @@ AddonManagerStartup::ReadStartupData(JSC
 
     if (!loc.ShouldCheckStartupModifications()) {
       continue;
     }
 
     for (auto e2 : loc.Addons()) {
       Addon addon(e2);
 
-      if (addon.Enabled() && addon.UpdateLastModifiedTime()) {
-        loc.SetChanged(true);
+      if (addon.Enabled()) {
+        bool changed;
+        MOZ_TRY_VAR(changed, addon.UpdateLastModifiedTime());
+        if (changed) {
+          loc.SetChanged(true);
+        }
       }
     }
   }
 
   return NS_OK;
 }
 
 nsresult
@@ -566,17 +570,17 @@ AddonManagerStartup::InitializeExtension
   JS::RootedObject locs(cx, &locations.toObject());
   for (auto e1 : PropertyIter(cx, locs)) {
     InstallLocation loc(e1);
 
     for (auto e2 : loc.Addons()) {
       Addon addon(e2);
 
       if (addon.Enabled() && !addon.Bootstrapped()) {
-        AddInstallLocation(addon);
+        Unused << AddInstallLocation(addon);
 
         if (enableInterpositions && addon.ShimsEnabled()) {
           EnableShims(addon.Id());
         }
       }
     }
   }
 
--- a/toolkit/mozapps/extensions/AddonManagerStartup.h
+++ b/toolkit/mozapps/extensions/AddonManagerStartup.h
@@ -2,16 +2,17 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef AddonManagerStartup_h
 #define AddonManagerStartup_h
 
 #include "amIAddonManagerStartup.h"
+#include "mozilla/Result.h"
 #include "nsCOMArray.h"
 #include "nsCOMPtr.h"
 #include "nsIFile.h"
 #include "nsISupports.h"
 
 #include "jsapi.h"
 
 namespace mozilla {
@@ -40,17 +41,17 @@ public:
   }
 
   const nsCOMArray<nsIFile>& ThemePaths()
   {
     return mExtensionPaths;
   }
 
 private:
-  void AddInstallLocation(Addon& addon);
+  Result<Ok, nsresult> AddInstallLocation(Addon& addon);
 
   nsIFile* ProfileDir();
 
   nsCOMPtr<nsIFile> mProfileDir;
 
   nsCOMArray<nsIFile> mExtensionPaths;
   nsCOMArray<nsIFile> mThemePaths;
 
new file mode 100644
--- /dev/null
+++ b/toolkit/mozapps/extensions/test/xpcshell/test_addonStartup.js
@@ -0,0 +1,119 @@
+"use strict";
+
+const Cr = Components.results;
+
+add_task(async function test_XPIStates_invalid_paths() {
+  let {path} = gAddonStartup;
+
+  let startupDatasets = [
+    {
+      "app-global": {
+        "addons": {
+          "{972ce4c6-7e08-4474-a285-3208198ce6fd}": {
+            "enabled": true,
+            "lastModifiedTime": 1,
+            "path": "{972ce4c6-7e08-4474-a285-3208198ce6fd}",
+            "type": "theme",
+            "version": "55.0a1",
+          }
+        },
+        "checkStartupModifications": true,
+        "path": "c:\\Program Files\\Mozilla Firefox\\extensions",
+      },
+      "app-profile": {
+        "addons": {
+          "xpcshell-something-or-other@mozilla.org": {
+            "bootstrapped": true,
+            "dependencies": [],
+            "enabled": true,
+            "hasEmbeddedWebExtension": false,
+            "lastModifiedTime": 1,
+            "path": "xpcshell-something-or-other@mozilla.org",
+            "version": "0.0.0",
+          },
+        },
+        "checkStartupModifications": true,
+        "path": "/home/xpcshell/.mozilla/firefox/default/extensions",
+      },
+    },
+    {
+      "app-global": {
+        "addons": {
+          "{972ce4c6-7e08-4474-a285-3208198ce6fd}": {
+            "enabled": true,
+            "lastModifiedTime": 1,
+            "path": "{972ce4c6-7e08-4474-a285-3208198ce6fd}",
+            "type": "theme",
+            "version": "55.0a1",
+          }
+        },
+        "checkStartupModifications": true,
+        "path": "c:\\Program Files\\Mozilla Firefox\\extensions",
+      },
+      "app-profile": {
+        "addons": {
+          "xpcshell-something-or-other@mozilla.org": {
+            "bootstrapped": true,
+            "dependencies": [],
+            "enabled": true,
+            "hasEmbeddedWebExtension": false,
+            "lastModifiedTime": 1,
+            "path": "xpcshell-something-or-other@mozilla.org",
+            "version": "0.0.0",
+          },
+        },
+        "checkStartupModifications": true,
+        "path": "c:\\Users\\XpcShell\\Application Data\\Mozilla Firefox\\Profiles\\meh",
+      },
+    },
+    {
+      "app-profile": {
+        "addons": {
+          "xpcshell-something-or-other@mozilla.org": {
+            "bootstrapped": true,
+            "dependencies": [],
+            "enabled": true,
+            "hasEmbeddedWebExtension": false,
+            "lastModifiedTime": 1,
+            "path": "/home/xpcshell/my-extensions/something-or-other",
+            "version": "0.0.0",
+          },
+        },
+        "checkStartupModifications": true,
+        "path": "/home/xpcshell/.mozilla/firefox/default/extensions",
+      },
+    },
+    {
+      "app-profile": {
+        "addons": {
+          "xpcshell-something-or-other@mozilla.org": {
+            "bootstrapped": true,
+            "dependencies": [],
+            "enabled": true,
+            "hasEmbeddedWebExtension": false,
+            "lastModifiedTime": 1,
+            "path": "c:\\Users\\XpcShell\\my-extensions\\something-or-other",
+            "version": "0.0.0",
+          },
+        },
+        "checkStartupModifications": true,
+        "path": "c:\\Users\\XpcShell\\Application Data\\Mozilla Firefox\\Profiles\\meh",
+      },
+    },
+  ];
+
+  for (let startupData of startupDatasets) {
+    let data = new TextEncoder().encode(JSON.stringify(startupData));
+
+    await OS.File.writeAtomic(path, data, {compression: "lz4"});
+
+    try {
+      let result = aomStartup.readStartupData();
+      do_print(`readStartupData() returned ${JSON.stringify(result)}`);
+    } catch (e) {
+      // We don't care if this throws, only that it doesn't crash.
+      do_print(`readStartupData() threw: ${e}`);
+      equal(e.result, Cr.NS_ERROR_FILE_UNRECOGNIZED_PATH, "Got expected error code");
+    }
+  }
+});
--- a/toolkit/mozapps/extensions/test/xpcshell/xpcshell.ini
+++ b/toolkit/mozapps/extensions/test/xpcshell/xpcshell.ini
@@ -4,16 +4,17 @@ tags = addons
 head = head_addons.js
 firefox-appdir = browser
 dupe-manifest =
 support-files =
   data/**
   xpcshell-shared.ini
 
 [test_addon_path_service.js]
+[test_addonStartup.js]
 [test_asyncBlocklistLoad.js]
 tags = blocklist
 [test_blocklist_gfx.js]
 tags = blocklist
 [test_cache_certdb.js]
 run-if = addon_signing
 [test_cacheflush.js]
 [test_DeferredSave.js]