Bug 1347386 - Do not allow reserved bookmark folders to be removed via bookmarks API, r?kmag draft
authorBob Silverberg <bsilverberg@mozilla.com>
Mon, 17 Apr 2017 15:45:19 -0400
changeset 563764 bf84ecc309f700de4ba7eba4cf7c36bbd2f590c7
parent 563687 05c212a94183838f12feebb2c3fd483a6eec18c2
child 624575 1948ee8aa6cfa979cfb4b4d073d38f3428c5b689
push id54419
push userbmo:bob.silverberg@gmail.com
push dateMon, 17 Apr 2017 20:11:42 +0000
reviewerskmag
bugs1347386
milestone55.0a1
Bug 1347386 - Do not allow reserved bookmark folders to be removed via bookmarks API, r?kmag MozReview-Commit-ID: FqnCVFg0hGF
browser/components/extensions/ext-bookmarks.js
browser/components/extensions/test/xpcshell/test_ext_bookmarks.js
--- a/browser/components/extensions/ext-bookmarks.js
+++ b/browser/components/extensions/ext-bookmarks.js
@@ -2,16 +2,24 @@
 /* vim: set sts=2 sw=2 et tw=80: */
 "use strict";
 
 XPCOMUtils.defineLazyModuleGetter(this, "PlacesUtils",
                                   "resource://gre/modules/PlacesUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Task",
                                   "resource://gre/modules/Task.jsm");
 
+const NON_REMOVABLE_GUIDS = [
+  PlacesUtils.bookmarks.rootGuid,
+  PlacesUtils.bookmarks.menuGuid,
+  PlacesUtils.bookmarks.toolbarGuid,
+  PlacesUtils.bookmarks.unfiledGuid,
+  PlacesUtils.bookmarks.mobileGuid,
+];
+
 let listenerCount = 0;
 
 function getTree(rootGuid, onlyChildren) {
   function convert(node, parent) {
     let treenode = {
       id: node.guid,
       title: node.title || "",
       index: node.index,
@@ -281,30 +289,40 @@ this.bookmarks = class extends Extension
             return PlacesUtils.bookmarks.update(info).then(convert)
               .catch(error => Promise.reject({message: error.message}));
           } catch (e) {
             return Promise.reject({message: `Invalid bookmark: ${JSON.stringify(info)}`});
           }
         },
 
         remove: function(id) {
+          // Do not remove the root nodes.
+          if (NON_REMOVABLE_GUIDS.includes(id)) {
+            return Promise.reject({message: `Cannot remove bookmark with id: ${id}`});
+          }
+
           let info = {
             guid: id,
           };
 
           // The API doesn't give you the old bookmark at the moment
           try {
             return PlacesUtils.bookmarks.remove(info, {preventRemovalOfNonEmptyFolders: true}).then(result => {})
               .catch(error => Promise.reject({message: error.message}));
           } catch (e) {
             return Promise.reject({message: `Invalid bookmark: ${JSON.stringify(info)}`});
           }
         },
 
         removeTree: function(id) {
+          // Do not remove the root nodes.
+          if (NON_REMOVABLE_GUIDS.includes(id)) {
+            return Promise.reject({message: `Cannot remove bookmark with id: ${id}`});
+          }
+
           let info = {
             guid: id,
           };
 
           try {
             return PlacesUtils.bookmarks.remove(info).then(result => {})
               .catch(error => Promise.reject({message: error.message}));
           } catch (e) {
--- a/browser/components/extensions/test/xpcshell/test_ext_bookmarks.js
+++ b/browser/components/extensions/test/xpcshell/test_ext_bookmarks.js
@@ -652,8 +652,58 @@ add_task(async function test_get_recent_
     let expected = createdBookmarks[i];
     equal(actual.url, expected.url, "Bookmark has the expected url.");
     equal(actual.title, expected.title, "Bookmark has the expected title.");
     equal(actual.parentId, expected.parentGuid, "Bookmark has the expected parentId.");
   }
 
   await extension.unload();
 });
+
+add_task(async function test_cannot_remove_reserved_folders() {
+  const NON_REMOVABLE_GUIDS = [
+    PlacesUtils.bookmarks.rootGuid,
+    PlacesUtils.bookmarks.menuGuid,
+    PlacesUtils.bookmarks.toolbarGuid,
+    PlacesUtils.bookmarks.unfiledGuid,
+    PlacesUtils.bookmarks.mobileGuid,
+  ];
+
+  async function background() {
+    function expectedError() {
+      browser.test.fail("Did not get expected error");
+    }
+
+    browser.test.onMessage.addListener(msg => {
+      browser.bookmarks.remove(msg).then(expectedError, error => {
+        browser.test.assertTrue(
+          error.message.includes(`Cannot remove bookmark with id: ${msg}`),
+          `Expected error thrown when trying to remove bookmark with id: ${msg}`
+        );
+      }).then(() => {
+        browser.bookmarks.removeTree(msg).then(expectedError, error => {
+          browser.test.assertTrue(
+            error.message.includes(`Cannot remove bookmark with id: ${msg}`),
+            `Expected error thrown when trying to removeTree with bookmark id: ${msg}`
+          );
+        });
+      }).then(() => {
+        browser.test.sendMessage(msg);
+      });
+    });
+  }
+
+  let extension = ExtensionTestUtils.loadExtension({
+    background,
+    manifest: {
+      permissions: ["bookmarks"],
+    },
+  });
+
+  await extension.startup();
+
+  for (let id of NON_REMOVABLE_GUIDS) {
+    extension.sendMessage(id);
+    await extension.awaitMessage(id);
+  }
+
+  await extension.unload();
+});