Bug 1543617 - Disallow place: urls in text flavors. r=Standard8
authorMarco Bonardo <mbonardo@mozilla.com>
Thu, 11 Apr 2019 14:31:03 +0000
changeset 469230 3e5250ccb4fec40a10080dce6a16a21bbe12be86
parent 469229 5e05dde4f7cb57929e3427f2877156b01df5831f
child 469231 b5f523e6d49841c25d7423870fa6e36e026e24a0
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)
reviewersStandard8
bugs1543617
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 1543617 - Disallow place: urls in text flavors. r=Standard8 Differential Revision: https://phabricator.services.mozilla.com/D27048
toolkit/components/places/PlacesUtils.jsm
toolkit/components/places/tests/unit/test_PlacesUtils_unwrapNodes_place.js
toolkit/components/places/tests/unit/xpcshell.ini
--- a/toolkit/components/places/PlacesUtils.jsm
+++ b/toolkit/components/places/PlacesUtils.jsm
@@ -1044,16 +1044,17 @@ var PlacesUtils = {
   /**
    * Unwraps data from the Clipboard or the current Drag Session.
    * @param   blob
    *          A blob (string) of data, in some format we potentially know how
    *          to parse.
    * @param   type
    *          The content type of the blob.
    * @returns An array of objects representing each item contained by the source.
+   * @throws if the blob contains invalid data.
    */
   unwrapNodes: function PU_unwrapNodes(blob, type) {
     // We split on "\n"  because the transferable system converts "\r\n" to "\n"
     var nodes = [];
     switch (type) {
       case this.TYPE_X_MOZ_PLACE:
       case this.TYPE_X_MOZ_PLACE_SEPARATOR:
       case this.TYPE_X_MOZ_PLACE_CONTAINER:
@@ -1074,37 +1075,42 @@ var PlacesUtils = {
           } else {
             // for drag and drop of files, try to use the leafName as title
             try {
               titleString = Services.io.newURI(uriString).QueryInterface(Ci.nsIURL)
                                 .fileName;
             } catch (ex) {}
           }
           // note:  Services.io.newURI() will throw if uriString is not a valid URI
-          if (Services.io.newURI(uriString)) {
+          let uri = Services.io.newURI(uriString);
+          if (Services.io.newURI(uriString) && uri.scheme != "place") {
             nodes.push({ uri: uriString,
                          title: titleString ? titleString : uriString,
                          type: this.TYPE_X_MOZ_URL });
           }
         }
         break;
       }
       case this.TYPE_UNICODE: {
         let parts = blob.split("\n");
         for (let i = 0; i < parts.length; i++) {
           let uriString = parts[i];
           // text/uri-list is converted to TYPE_UNICODE but it could contain
-          // comments line prepended by #, we should skip them
-          if (uriString.substr(0, 1) == "\x23")
+          // comments line prepended by #, we should skip them, as well as
+          // empty uris.
+          if (uriString.substr(0, 1) == "\x23" || uriString == "") {
             continue;
+          }
           // note: Services.io.newURI) will throw if uriString is not a valid URI
-          if (uriString != "" && Services.io.newURI(uriString))
+          let uri = Services.io.newURI(uriString);
+          if (uri.scheme != "place") {
             nodes.push({ uri: uriString,
                          title: uriString,
                          type: this.TYPE_X_MOZ_URL });
+          }
         }
         break;
       }
       default:
         throw Cr.NS_ERROR_INVALID_ARG;
     }
     return nodes;
   },
new file mode 100644
--- /dev/null
+++ b/toolkit/components/places/tests/unit/test_PlacesUtils_unwrapNodes_place.js
@@ -0,0 +1,25 @@
+/* Any copyright is dedicated to the Public Domain.
+ * https://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Tests that unwrapNodes properly filters out place: uris from text flavors.
+
+add_task(function() {
+  let tests = [
+    // Single url.
+    ["place:type=0&sort=1:", PlacesUtils.TYPE_X_MOZ_URL],
+    // Multiple urls.
+    ["place:type=0&sort=1:\nfirst\nplace:type=0&sort=1\nsecond", PlacesUtils.TYPE_X_MOZ_URL],
+    // Url == title.
+    ["place:type=0&sort=1:\nplace:type=0&sort=1", PlacesUtils.TYPE_X_MOZ_URL],
+    // Malformed.
+    ["place:type=0&sort=1:\nplace:type=0&sort=1\nmalformed", PlacesUtils.TYPE_X_MOZ_URL],
+    // Single url.
+    ["place:type=0&sort=1:", PlacesUtils.TYPE_UNICODE],
+    // Multiple urls.
+    ["place:type=0&sort=1:\nplace:type=0&sort=1", PlacesUtils.TYPE_UNICODE],
+  ];
+  for (let [blob, type] of tests) {
+    Assert.deepEqual(PlacesUtils.unwrapNodes(blob, type), [],
+                     "No valid entries should be found");
+  }
+});
--- a/toolkit/components/places/tests/unit/xpcshell.ini
+++ b/toolkit/components/places/tests/unit/xpcshell.ini
@@ -85,16 +85,17 @@ support-files = noRoot.sqlite
 [test_onItemChanged_tags.js]
 [test_origins.js]
 [test_origins_parsing.js]
 [test_pageGuid_bookmarkGuid.js]
 [test_frecency_observers.js]
 [test_placeURIs.js]
 [test_PlacesUtils_invalidateCachedGuidFor.js]
 [test_PlacesUtils_isRootItem.js]
+[test_PlacesUtils_unwrapNodes_place.js]
 [test_promiseBookmarksTree.js]
 [test_resolveNullBookmarkTitles.js]
 [test_result_sort.js]
 [test_resultsAsVisit_details.js]
 [test_sql_function_origin.js]
 [test_sql_guid_functions.js]
 [test_tag_autocomplete_search.js]
 [test_tagging.js]