Bug 1329457, r=bz,dao, a=jcristau,gchang
MozReview-Commit-ID: 9WNuDkQnyM0
--- 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
@@ -55,23 +55,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;
}
}
};