Bug 1329457, r=bz,dao, a=jcristau,gchang
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Mon, 09 Jan 2017 16:07:57 +0000
changeset 353552 8a89dd47e7847c27ffe04b12832f0cdba7262b56
parent 353551 6a7701e2bb8ea480c6001069f0f04c938a2d4e53
child 353553 d51ff865f7da04783da5431d287ed24d57ee5a0e
push id6795
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 14:19:46 +0000
treeherdermozilla-esr52@76101b503191 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz, dao, jcristau, gchang
bugs1329457
milestone52.0a2
Bug 1329457, r=bz,dao, a=jcristau,gchang MozReview-Commit-ID: 9WNuDkQnyM0
browser/components/newtab/PlacesProvider.jsm
browser/components/newtab/tests/xpcshell/test_PlacesProvider.js
toolkit/modules/NewTabUtils.jsm
--- a/browser/components/newtab/PlacesProvider.jsm
+++ b/browser/components/newtab/PlacesProvider.jsm
@@ -1,13 +1,13 @@
 /* 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/. */
 
-/* global XPCOMUtils, Services, PlacesUtils, gPrincipal, EventEmitter */
+/* global XPCOMUtils, Services, PlacesUtils, EventEmitter */
 /* global gLinks */
 /* exported PlacesProvider */
 
 "use strict";
 
 this.EXPORTED_SYMBOLS = ["PlacesProvider"];
 
 const {interfaces: Ci, utils: Cu} = Components;
@@ -17,62 +17,24 @@ Cu.import("resource://gre/modules/XPCOMU
 XPCOMUtils.defineLazyModuleGetter(this, "PlacesUtils",
   "resource://gre/modules/PlacesUtils.jsm");
 
 XPCOMUtils.defineLazyGetter(this, "EventEmitter", function() {
   const {EventEmitter} = Cu.import("resource://devtools/shared/event-emitter.js", {});
   return EventEmitter;
 });
 
-XPCOMUtils.defineLazyGetter(this, "gPrincipal", function() {
-  let uri = Services.io.newURI("about:newtab", null, null);
-  return Services.scriptSecurityManager.getNoAppCodebasePrincipal(uri);
-});
-
 XPCOMUtils.defineLazyModuleGetter(this, "Task",
                                   "resource://gre/modules/Task.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "NewTabUtils",
+                                  "resource://gre/modules/NewTabUtils.jsm");
 
 // The maximum number of results PlacesProvider retrieves from history.
 const HISTORY_RESULTS_LIMIT = 100;
 
-/**
- * Singleton that checks if a given link should be displayed on about:newtab
- * or if we should rather not do it for security reasons. URIs that inherit
- * their caller's principal will be filtered.
- */
-let LinkChecker = {
-  _cache: new Map(),
-
-  get flags() {
-    return Ci.nsIScriptSecurityManager.DISALLOW_INHERIT_PRINCIPAL |
-           Ci.nsIScriptSecurityManager.DONT_REPORT_ERRORS;
-  },
-
-  checkLoadURI: function LinkChecker_checkLoadURI(aURI) {
-    if (!this._cache.has(aURI)) {
-      this._cache.set(aURI, this._doCheckLoadURI(aURI));
-    }
-
-    return this._cache.get(aURI);
-  },
-
-  _doCheckLoadURI: function LinkChecker_doCheckLoadURI(aURI) {
-    let result = false;
-    try {
-      Services.scriptSecurityManager.
-        checkLoadURIStrWithPrincipal(gPrincipal, aURI, this.flags);
-      result = true;
-    } catch (e) {
-      // We got a weird URI or one that would inherit the caller's principal.
-      Cu.reportError(e);
-    }
-    return result;
-  }
-};
-
 /* Queries history to retrieve the most visited sites. Emits events when the
  * history changes.
  * Implements the EventEmitter interface.
  */
 let Links = function Links() {
   EventEmitter.decorate(this);
 };
 
@@ -100,37 +62,40 @@ Links.prototype = {
     onClearHistory: function historyObserver_onClearHistory() {
       gLinks.emit("clearHistory");
     },
 
     onFrecencyChanged: function historyObserver_onFrecencyChanged(aURI,
                            aNewFrecency, aGUID, aHidden, aLastVisitDate) { // jshint ignore:line
       // The implementation of the query in getLinks excludes hidden and
       // unvisited pages, so it's important to exclude them here, too.
-      if (!aHidden && aLastVisitDate) {
+      if (!aHidden && aLastVisitDate &&
+          NewTabUtils.linkChecker.checkLoadURI(aURI.spec)) {
         gLinks.emit("linkChanged", {
           url: aURI.spec,
           frecency: aNewFrecency,
           lastVisitDate: aLastVisitDate,
           type: "history",
         });
       }
     },
 
     onManyFrecenciesChanged: function historyObserver_onManyFrecenciesChanged() {
       // Called when frecencies are invalidated and also when clearHistory is called
       // See toolkit/components/places/tests/unit/test_frecency_observers.js
       gLinks.emit("manyLinksChanged");
     },
 
     onTitleChanged: function historyObserver_onTitleChanged(aURI, aNewTitle) {
-      gLinks.emit("linkChanged", {
-        url: aURI.spec,
-        title: aNewTitle
-      });
+      if (NewTabUtils.linkChecker.checkLoadURI(aURI.spec)) {
+        gLinks.emit("linkChanged", {
+          url: aURI.spec,
+          title: aNewTitle
+        });
+      }
     },
 
     QueryInterface: XPCOMUtils.generateQI([Ci.nsINavHistoryObserver,
                         Ci.nsISupportsWeakReference])
   },
 
   /**
    * Must be called before the provider is used.
@@ -179,17 +144,17 @@ Links.prototype = {
                    GROUP BY rev_host HAVING MAX(lastVisitDate)
                    ORDER BY frecency DESC, lastVisitDate DESC, url`;
 
     let links = yield this.executePlacesQuery(sqlQuery, {
                   columns: ["url", "title", "lastVisitDate", "frecency", "type"],
                   params: {limit: this.maxNumLinks}
                 });
 
-    return links.filter(link => LinkChecker.checkLoadURI(link.url));
+    return links.filter(link => NewTabUtils.linkChecker.checkLoadURI(link.url));
   }),
 
   /**
    * Executes arbitrary query against places database
    *
    * @param {String} aSql
    *        SQL query to execute
    * @param {Object} [optional] aOptions
@@ -243,11 +208,15 @@ Links.prototype = {
 };
 
 /**
  * Singleton that serves as the default link provider for the grid.
  */
 const gLinks = new Links(); // jshint ignore:line
 
 let PlacesProvider = {
-  LinkChecker: LinkChecker,
   links: gLinks,
 };
+
+// Kept only for backwards-compatibility
+XPCOMUtils.defineLazyGetter(PlacesProvider, "LinkChecker",
+  () => NewTabUtils.linkChecker);
+
--- a/browser/components/newtab/tests/xpcshell/test_PlacesProvider.js
+++ b/browser/components/newtab/tests/xpcshell/test_PlacesProvider.js
@@ -54,23 +54,23 @@ function makeVisit(index, daysAgo, isTyp
   };
 }
 
 /** Test LinkChecker **/
 
 add_task(function test_LinkChecker_securityCheck() {
 
   let urls = [
-    {url: "file://home/file/image.png", expected: false},
-    {url: "resource:///modules/PlacesProvider.jsm", expected: false},
     {url: "javascript:alert('hello')", expected: false}, // jshint ignore:line
     {url: "data:image/png;base64,XXX", expected: false},
     {url: "about:newtab", expected: true},
     {url: "https://example.com", expected: true},
     {url: "ftp://example.com", expected: true},
+    {url: "file://home/file/image.png", expected: true},
+    {url: "resource:///modules/PlacesProvider.jsm", expected: true},
   ];
   for (let {url, expected} of urls) {
     let observed = PlacesProvider.LinkChecker.checkLoadURI(url);
     equal(observed, expected, `can load "${url}"?`);
   }
 });
 
 /** Test Provider **/
--- a/toolkit/modules/NewTabUtils.jsm
+++ b/toolkit/modules/NewTabUtils.jsm
@@ -18,21 +18,16 @@ XPCOMUtils.defineLazyModuleGetter(this, 
   "resource://gre/modules/PlacesUtils.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "PageThumbs",
   "resource://gre/modules/PageThumbs.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "BinarySearch",
   "resource://gre/modules/BinarySearch.jsm");
 
-XPCOMUtils.defineLazyGetter(this, "gPrincipal", function () {
-  let uri = Services.io.newURI("about:newtab", null, null);
-  return Services.scriptSecurityManager.createCodebasePrincipal(uri, {});
-});
-
 XPCOMUtils.defineLazyGetter(this, "gCryptoHash", function () {
   return Cc["@mozilla.org/security/hash;1"].createInstance(Ci.nsICryptoHash);
 });
 
 XPCOMUtils.defineLazyGetter(this, "gUnicodeConverter", function () {
   let converter = Cc["@mozilla.org/intl/scriptableunicodeconverter"]
                     .createInstance(Ci.nsIScriptableUnicodeConverter);
   converter.charset = 'utf8';
@@ -1348,18 +1343,22 @@ var LinkChecker = {
     if (!(aURI in this._cache))
       this._cache[aURI] = this._doCheckLoadURI(aURI);
 
     return this._cache[aURI];
   },
 
   _doCheckLoadURI: function Links_doCheckLoadURI(aURI) {
     try {
+      // about:newtab is currently privileged. In any case, it should be
+      // possible for tiles to point to pretty much everything - but not
+      // to stuff that inherits the system principal, so we check:
+      let systemPrincipal = Services.scriptSecurityManager.getSystemPrincipal();
       Services.scriptSecurityManager.
-        checkLoadURIStrWithPrincipal(gPrincipal, aURI, this.flags);
+        checkLoadURIStrWithPrincipal(systemPrincipal, aURI, this.flags);
       return true;
     } catch (e) {
       // We got a weird URI or one that would inherit the caller's principal.
       return false;
     }
   }
 };