Bug 1541924 - Enforce that BookmarkJSONUtils.importFromURL can only be used with chrome and file URLs. r=mak
authorMark Banner <standard8@mozilla.com>
Fri, 12 Apr 2019 12:34:44 +0000
changeset 469274 c49b36c011b2e462a9cf66a31ace780e3ef66b73
parent 469273 32342cbe673a3fb37e9e9de2e818c379198a292c
child 469275 d562b158decc56ba2b9b270f4857f75458534d53
push id112776
push usershindli@mozilla.com
push dateFri, 12 Apr 2019 16:20:17 +0000
treeherdermozilla-inbound@b4501ced5619 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1541924
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1541924 - Enforce that BookmarkJSONUtils.importFromURL can only be used with chrome and file URLs. r=mak Differential Revision: https://phabricator.services.mozilla.com/D26921
toolkit/components/places/BookmarkJSONUtils.jsm
toolkit/components/places/tests/bookmarks/bookmarks_long_tag.json
toolkit/components/places/tests/bookmarks/test_1129529.js
toolkit/components/places/tests/bookmarks/xpcshell.ini
toolkit/components/places/tests/unit/test_bookmarks_json.js
--- a/toolkit/components/places/BookmarkJSONUtils.jsm
+++ b/toolkit/components/places/BookmarkJSONUtils.jsm
@@ -41,21 +41,21 @@ function generateHash(aString) {
   // base64 allows the '/' char, but we can't use it for filenames.
   return cryptoHash.finish(true).replace(/\//g, "-");
 }
 
 var BookmarkJSONUtils = Object.freeze({
   /**
    * Import bookmarks from a url.
    *
-   * @param aSpec
+   * @param {string} aSpec
    *        url of the bookmark data.
-   * @param [options.replace]
+   * @param {boolean} [options.replace]
    *        Whether we should erase existing bookmarks before importing.
-   * @param [options.source]
+   * @param {PlacesUtils.bookmarks.SOURCES} [options.source]
    *        The bookmark change source, used to determine the sync status for
    *        imported bookmarks. Defaults to `RESTORE` if `replace = true`, or
    *        `IMPORT` otherwise.
    *
    * @return {Promise}
    * @resolves When the new bookmarks have been created.
    * @rejects JavaScript exception.
    */
@@ -68,16 +68,17 @@ var BookmarkJSONUtils = Object.freeze({
     try {
       let importer = new BookmarkImporter(aReplace, aSource);
       await importer.importFromURL(aSpec);
 
       notifyObservers(PlacesUtils.TOPIC_BOOKMARKS_RESTORE_SUCCESS, aReplace);
     } catch (ex) {
       Cu.reportError("Failed to restore bookmarks from " + aSpec + ": " + ex);
       notifyObservers(PlacesUtils.TOPIC_BOOKMARKS_RESTORE_FAILED, aReplace);
+      throw ex;
     }
   },
 
   /**
    * Restores bookmarks and tags from a JSON file.
    *
    * @param aFilePath
    *        OS.File path string of bookmarks in JSON or JSONlz4 format to be restored.
@@ -169,24 +170,28 @@ var BookmarkJSONUtils = Object.freeze({
 function BookmarkImporter(aReplace, aSource) {
   this._replace = aReplace;
   this._source = aSource;
 }
 BookmarkImporter.prototype = {
   /**
    * Import bookmarks from a url.
    *
-   * @param aSpec
+   * @param {string} aSpec
    *        url of the bookmark data.
    *
    * @return {Promise}
    * @resolves When the new bookmarks have been created.
    * @rejects JavaScript exception.
    */
   async importFromURL(spec) {
+    if (!spec.startsWith("chrome://") &&
+        !spec.startsWith("file://")) {
+      throw new Error("importFromURL can only be used with chrome:// and file:// URLs");
+    }
     let nodes = await (await fetch(spec)).json();
 
     if (!nodes.children || !nodes.children.length) {
       return;
     }
 
     await this.import(nodes);
   },
new file mode 100644
--- /dev/null
+++ b/toolkit/components/places/tests/bookmarks/bookmarks_long_tag.json
@@ -0,0 +1,53 @@
+{
+  "guid": "root________",
+  "index": 0,
+  "id": 1,
+  "type": "text/x-moz-place-container",
+  "dateAdded": 1554906792778,
+  "lastModified": 1554906792778,
+  "root": "placesRoot",
+  "children": [{
+    "guid": "unfiled_____",
+    "index": 0,
+    "id": 2,
+    "type": "text/x-moz-place-container",
+    "dateAdded": 1554906792778,
+    "lastModified": 1554906792778,
+    "root": "unfiledBookmarksFolder",
+    "children": [
+      {
+        "guid": "___guid1____",
+        "index": 0,
+        "id": 3,
+        "charset": "UTF-16",
+        "tags": "tag0",
+        "type": "text/x-moz-place",
+        "dateAdded": 1554906792778,
+        "lastModified": 1554906792778,
+        "uri": "http://test0.com/"
+      },
+      {
+        "guid": "___guid2____",
+        "index": 1,
+        "id": 4,
+        "charset": "UTF-16",
+        "tags": "tag1,a0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789",
+        "type": "text/x-moz-place",
+        "dateAdded": 1554906792778,
+        "lastModified": 1554906792778,
+        "uri": "http://test1.com/"
+      },
+      {
+        "guid": "___guid3____",
+        "index": 2,
+        "id": 5,
+        "charset": "UTF-16",
+        "tags": "tag2",
+        "type": "text/x-moz-place",
+        "dateAdded": 1554906792778,
+        "lastModified": 1554906792778,
+        "uri": "http://test2.com/"
+      }
+    ]
+  }]
+}
--- a/toolkit/components/places/tests/bookmarks/test_1129529.js
+++ b/toolkit/components/places/tests/bookmarks/test_1129529.js
@@ -1,76 +1,23 @@
 var now = Date.now() * 1000;
 
 // Test that importing bookmark data where a bookmark has a tag longer than 100
 // chars imports everything except the tags for that bookmark.
 add_task(async function() {
-  let aData = {
-    guid: "root________",
-    index: 0,
-    id: 1,
-    type: "text/x-moz-place-container",
-    dateAdded: now,
-    lastModified: now,
-    root: "placesRoot",
-    children: [{
-      guid: "unfiled_____",
-      index: 0,
-      id: 2,
-      type: "text/x-moz-place-container",
-      dateAdded: now,
-      lastModified: now,
-      root: "unfiledBookmarksFolder",
-      children: [
-        {
-          guid: "___guid1____",
-          index: 0,
-          id: 3,
-          charset: "UTF-16",
-          tags: "tag0",
-          type: "text/x-moz-place",
-          dateAdded: now,
-          lastModified: now,
-          uri: "http://test0.com/",
-        },
-        {
-          guid: "___guid2____",
-          index: 1,
-          id: 4,
-          charset: "UTF-16",
-          tags: "tag1,a" + "0123456789".repeat(10), // 101 chars
-          type: "text/x-moz-place",
-          dateAdded: now,
-          lastModified: now,
-          uri: "http://test1.com/",
-        },
-        {
-          guid: "___guid3____",
-          index: 2,
-          id: 5,
-          charset: "UTF-16",
-          tags: "tag2",
-          type: "text/x-moz-place",
-          dateAdded: now,
-          lastModified: now,
-          uri: "http://test2.com/",
-        },
-      ],
-    }],
-  };
+  let bookmarksFile = OS.Path.join(do_get_cwd().path, "bookmarks_long_tag.json");
+  let bookmarksUrl = OS.Path.toFileURI(bookmarksFile);
 
-  let contentType = "application/json";
-  let uri = "data:" + contentType + "," + JSON.stringify(aData);
-  await BookmarkJSONUtils.importFromURL(uri);
+  await BookmarkJSONUtils.importFromURL(bookmarksUrl);
 
   let [bookmarks] = await PlacesBackups.getBookmarksTree();
   let unsortedBookmarks = bookmarks.children[2].children;
   Assert.equal(unsortedBookmarks.length, 3);
 
   for (let i = 0; i < unsortedBookmarks.length; ++i) {
     let bookmark = unsortedBookmarks[i];
     Assert.equal(bookmark.charset, "UTF-16");
-    Assert.equal(bookmark.dateAdded, now);
-    Assert.equal(bookmark.lastModified, now);
-    Assert.equal(bookmark.uri, "http://test" + i + ".com/");
-    Assert.equal(bookmark.tags, "tag" + i);
+    Assert.equal(bookmark.dateAdded, 1554906792000);
+    Assert.equal(bookmark.lastModified, 1554906792000);
+    Assert.equal(bookmark.uri, `http://test${i}.com/`);
+    Assert.equal(bookmark.tags, `tag${i}`);
   }
 });
--- a/toolkit/components/places/tests/bookmarks/xpcshell.ini
+++ b/toolkit/components/places/tests/bookmarks/xpcshell.ini
@@ -16,16 +16,18 @@ firefox-appdir = browser
 [test_466303-json-remove-backups.js]
 [test_477583_json-backup-in-future.js]
 [test_818584-discard-duplicate-backups.js]
 [test_818587_compress-bookmarks-backups.js]
 [test_818593-store-backup-metadata.js]
 [test_992901-backup-unsorted-hierarchy.js]
 [test_997030-bookmarks-html-encode.js]
 [test_1129529.js]
+support-files =
+  bookmarks_long_tag.json
 [test_async_observers.js]
 [test_bmindex.js]
 [test_bookmarkstree_cache.js]
 [test_bookmarks_eraseEverything.js]
 [test_bookmarks_fetch.js]
 [test_bookmarks_getRecent.js]
 [test_bookmarks_insert.js]
 [test_bookmarks_insertTree.js]
--- a/toolkit/components/places/tests/unit/test_bookmarks_json.js
+++ b/toolkit/components/places/tests/unit/test_bookmarks_json.js
@@ -113,16 +113,25 @@ var test_bookmarks = {
       url: `place:parent=OCyeUO5uu9FF&parent=${PlacesUtils.bookmarks.menuGuid}`,
     },
   ],
 };
 
 // Exported bookmarks file pointer.
 var bookmarksExportedFile;
 
+add_task(async function test_import_bookmarks_disallowed_url() {
+  await Assert.rejects(BookmarkJSONUtils.importFromURL("http://example.com/bookmarks.json"),
+    /importFromURL can only be used with/,
+    "Should reject importing from an http based url");
+  await Assert.rejects(BookmarkJSONUtils.importFromURL("https://example.com/bookmarks.json"),
+    /importFromURL can only be used with/,
+    "Should reject importing from an https based url");
+});
+
 add_task(async function test_import_bookmarks() {
   let bookmarksFile = OS.Path.join(do_get_cwd().path, "bookmarks.json");
 
   await BookmarkJSONUtils.importFromFile(bookmarksFile, { replace: true });
   await PlacesTestUtils.promiseAsyncUpdates();
   await testImportedBookmarks();
 });