Bug 824502 - When Sync receives incoming queries with folder= in them, add excludeItems=1 to the end to avoid broken queries loading the whole database. r?kitcambridge draft
authorMark Banner <standard8@mozilla.com>
Tue, 15 May 2018 16:28:29 +0100
changeset 795331 ab81cce24e0b0da8faff7c6df709fb919703dfc4
parent 795330 601972caa0ad62197942b6e269afbd12d241ca7c
push id109935
push userbmo:standard8@mozilla.com
push dateTue, 15 May 2018 15:57:31 +0000
reviewerskitcambridge
bugs824502
milestone62.0a1
Bug 824502 - When Sync receives incoming queries with folder= in them, add excludeItems=1 to the end to avoid broken queries loading the whole database. r?kitcambridge MozReview-Commit-ID: CI4YCvDqGSA
toolkit/components/places/SyncedBookmarksMirror.jsm
toolkit/components/places/tests/sync/test_bookmark_kinds.js
--- a/toolkit/components/places/SyncedBookmarksMirror.jsm
+++ b/toolkit/components/places/SyncedBookmarksMirror.jsm
@@ -742,16 +742,24 @@ class SyncedBookmarksMirror {
       let tagFolderName = validateTag(record.folderName);
       if (!tagFolderName) {
         MirrorLog.warn("Ignoring tag query ${guid} with invalid tag name " +
                        "${tagFolderName}", { guid, tagFolderName });
         ignoreCounts.query.url++;
         return;
       }
       url = new URL(`place:tag=${tagFolderName}`);
+    } else {
+      // For any other legacy queries with a "folder" parameter, we are not able to
+      // know which folder it should point to. Therefore, add excludeItems onto the
+      // end, so that it doesn't return everything in the database.
+      let folder = params.get("folder");
+      if (folder) {
+        url.href = `${url.href}&excludeItems=1`;
+      }
     }
 
     await this.maybeStoreRemoteURL(url);
 
     let serverModified = determineServerModified(record);
     let dateAdded = determineDateAdded(record);
     let title = validateTitle(record.title);
     let description = validateDescription(record.description);
--- a/toolkit/components/places/tests/sync/test_bookmark_kinds.js
+++ b/toolkit/components/places/tests/sync/test_bookmark_kinds.js
@@ -304,17 +304,17 @@ add_task(async function test_queries() {
       },
     ],
   });
 
   info("Make remote changes");
   await storeRecords(buf, shuffle([{
     id: "toolbar",
     type: "folder",
-    children: ["queryEEEEEEE", "queryFFFFFFF", "queryGGGGGGG"],
+    children: ["queryEEEEEEE", "queryFFFFFFF", "queryGGGGGGG", "queryHHHHHHH"],
   }, {
     // Legacy tag query.
     id: "queryEEEEEEE",
     type: "query",
     title: "E",
     bmkUri: "place:type=7&folder=999",
     folderName: "taggy",
   }, {
@@ -326,16 +326,22 @@ add_task(async function test_queries() {
     folderName: "a-tag",
   }, {
     // Legacy tag query referencing the same tag as the new query.
     id: "queryGGGGGGG",
     type: "query",
     title: "G",
     bmkUri: "place:type=7&folder=111&something=else",
     folderName: "a-tag",
+  }, {
+    // Legacy folder lookup query.
+    id: "queryHHHHHHH",
+    type: "query",
+    title: "H",
+    bmkUri: "place:folder=1",
   }]));
 
   info("Create records to upload");
   let changes = await buf.apply();
   Assert.strictEqual(changes.queryAAAAAAA.cleartext.folderName, tag.title);
   Assert.strictEqual(changes.queryBBBBBBB.cleartext.folderName, "b-tag");
   Assert.strictEqual(changes.queryCCCCCCC.cleartext.folderName, undefined);
   Assert.strictEqual(changes.queryDDDDDDD.cleartext.folderName, tag.title);
@@ -358,18 +364,24 @@ add_task(async function test_queries() {
       title: "F",
       url: "place:tag=a-tag",
     }, {
       guid: "queryGGGGGGG",
       type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
       index: 2,
       title: "G",
       url: "place:tag=a-tag",
+    }, {
+      guid: "queryHHHHHHH",
+      type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
+      index: 3,
+      title: "H",
+      url: "place:folder=1&excludeItems=1",
     }],
-  }, "Should rewrite legacy remote tag queries");
+  }, "Should rewrite legacy remote queries");
 
   await buf.finalize();
   await PlacesUtils.bookmarks.eraseEverything();
   await PlacesSyncUtils.bookmarks.reset();
 });
 
 // Bug 632287.
 add_task(async function test_mismatched_but_compatible_folder_types() {