Bug 1521919 - SitePermissions.get should check for nsIURI. r=johannh
☠☠ backed out by d9708fb34224 ☠ ☠
authorphoenixabhishek <phoenixgyaan@gmail.com>
Fri, 22 Mar 2019 10:32:30 +0000
changeset 465703 01cdf8342a49c2e06728f15835217f9d23b25cb9
parent 465702 98343ff4132291a8ba9b5a23d3e8ec6e94a8c454
child 465704 5643338d4df0f6ee00cfcb99ad82c9687c07a59c
push id35746
push usershindli@mozilla.com
push dateSat, 23 Mar 2019 09:46:24 +0000
treeherdermozilla-central@02b7484f316b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjohannh
bugs1521919
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 1521919 - SitePermissions.get should check for nsIURI. r=johannh functions now throw an error if the uri parameter is not an nsIURI. Differential Revision: https://phabricator.services.mozilla.com/D23672
browser/components/preferences/in-content/tests/browser_permissions_dialog.js
browser/modules/SitePermissions.jsm
browser/modules/test/unit/test_SitePermissions.js
--- a/browser/components/preferences/in-content/tests/browser_permissions_dialog.js
+++ b/browser/components/preferences/in-content/tests/browser_permissions_dialog.js
@@ -50,28 +50,28 @@ add_task(async function addPermission() 
 
   // Add notification permission for a website.
   SitePermissions.set(URI, "desktop-notification", SitePermissions.ALLOW);
 
   // Observe the added permission changes in the dialog UI.
   Assert.equal(richlistbox.itemCount, 1);
   checkPermissionItem(URL, Services.perms.ALLOW_ACTION);
 
-  SitePermissions.remove(URL, "desktop-notification");
+  SitePermissions.remove(URI, "desktop-notification");
 });
 
 add_task(async function observePermissionChange() {
   SitePermissions.set(URI, "desktop-notification", SitePermissions.ALLOW);
 
   // Change the permission.
   SitePermissions.set(URI, "desktop-notification", SitePermissions.BLOCK);
 
   checkPermissionItem(URL, Services.perms.DENY_ACTION);
 
-  SitePermissions.remove(URL, "desktop-notification");
+  SitePermissions.remove(URI, "desktop-notification");
 });
 
 add_task(async function observePermissionDelete() {
   let doc = sitePermissionsDialog.document;
   let richlistbox = doc.getElementById("permissionsBox");
 
   SitePermissions.set(URI, "desktop-notification", SitePermissions.ALLOW);
 
--- a/browser/modules/SitePermissions.jsm
+++ b/browser/modules/SitePermissions.jsm
@@ -240,16 +240,18 @@ var SitePermissions = {
    *
    * @return {Array} a list of objects with the keys:
    *          - id: the permissionId of the permission
    *          - scope: the scope of the permission (e.g. SitePermissions.SCOPE_TEMPORARY)
    *          - state: a constant representing the current permission state
    *            (e.g. SitePermissions.ALLOW)
    */
   getAllByURI(uri) {
+    if (!(uri instanceof Ci.nsIURI))
+      throw new Error("uri parameter should be an nsIURI");
     let result = [];
     if (!this.isSupportedURI(uri)) {
       return result;
     }
 
     let permissions = Services.perms.getAllForURI(uri);
     while (permissions.hasMoreElements()) {
       let permission = permissions.getNext();
@@ -445,16 +447,18 @@ var SitePermissions = {
    *
    * @return {Object} an object with the keys:
    *           - state: The current state of the permission
    *             (e.g. SitePermissions.ALLOW)
    *           - scope: The scope of the permission
    *             (e.g. SitePermissions.SCOPE_PERSISTENT)
    */
   get(uri, permissionID, browser) {
+    if ((!uri && !browser) || (uri && !(uri instanceof Ci.nsIURI)))
+      throw new Error("uri parameter should be an nsIURI or a browser parameter is needed");
     let defaultState = this.getDefault(permissionID);
     let result = { state: defaultState, scope: this.SCOPE_PERSISTENT };
     if (this.isSupportedURI(uri)) {
       let permission = null;
       if (permissionID in gPermissionObject &&
         gPermissionObject[permissionID].exactHostMatch) {
         permission = Services.perms.getPermissionObjectForURI(uri, permissionID, true);
       } else {
@@ -499,16 +503,18 @@ var SitePermissions = {
    *        The state of the permission.
    * @param {SitePermissions scope} scope (optional)
    *        The scope of the permission. Defaults to SCOPE_PERSISTENT.
    * @param {Browser} browser (optional)
    *        The browser object to set temporary permissions on.
    *        This needs to be provided if the scope is SCOPE_TEMPORARY!
    */
   set(uri, permissionID, state, scope = this.SCOPE_PERSISTENT, browser = null) {
+    if ((!uri && !browser) || (uri && !(uri instanceof Ci.nsIURI)))
+      throw new Error("uri parameter should be an nsIURI or a browser parameter is needed");
     if (scope == this.SCOPE_GLOBAL && state == this.BLOCK) {
       GloballyBlockedPermissions.set(browser, permissionID);
       browser.dispatchEvent(new browser.ownerGlobal.CustomEvent("PermissionStateChange"));
       return;
     }
 
     if (state == this.UNKNOWN || state == this.getDefault(permissionID)) {
       // Because they are controlled by two prefs with many states that do not
@@ -566,16 +572,18 @@ var SitePermissions = {
    * @param {nsIURI} uri
    *        The URI to remove the permission for.
    * @param {String} permissionID
    *        The id of the permission.
    * @param {Browser} browser (optional)
    *        The browser object to remove temporary permissions on.
    */
   remove(uri, permissionID, browser) {
+    if ((!uri && !browser) || (uri && !(uri instanceof Ci.nsIURI)))
+      throw new Error("uri parameter should be an nsIURI or a browser parameter is needed");
     if (this.isSupportedURI(uri))
       Services.perms.remove(uri, permissionID);
 
     // TemporaryPermissions.get() deletes expired permissions automatically,
     if (TemporaryPermissions.get(browser, permissionID)) {
       // If it exists but has not expired, remove it explicitly.
       TemporaryPermissions.remove(browser, permissionID);
       // Send a PermissionStateChange event only if the permission hasn't expired.
--- a/browser/modules/test/unit/test_SitePermissions.js
+++ b/browser/modules/test/unit/test_SitePermissions.js
@@ -4,16 +4,34 @@
 "use strict";
 
 const {SitePermissions} = ChromeUtils.import("resource:///modules/SitePermissions.jsm");
 const {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm");
 
 const RESIST_FINGERPRINTING_ENABLED = Services.prefs.getBoolPref("privacy.resistFingerprinting");
 const MIDI_ENABLED = Services.prefs.getBoolPref("dom.webmidi.enabled");
 
+add_task(async function testNsIURI() {
+  let uri = "http://foo.com/bar/baz";
+
+  Assert.throws(() => SitePermissions.getAllByURI(uri),
+                /Not_nsIURI_Error/,
+                "Should throw if arguments is not of type nsIURI.");
+  Assert.throws(() => SitePermissions.get(uri),
+                /Not_nsIURI_Error/,
+                "Should throw if arguments is not of type nsIURI.");
+  Assert.throws(() => SitePermissions.set(uri),
+                /Not_nsIURI_Error/,
+                "Should throw if arguments is not of type nsIURI.");
+  Assert.throws(() => SitePermissions.remove(uri),
+                /Not_nsIURI_Error/,
+                "Should throw if arguments is not of type nsIURI.");
+});
+
+
 add_task(async function testPermissionsListing() {
   let expectedPermissions = ["autoplay-media", "camera", "cookie", "desktop-notification", "focus-tab-by-prompt",
      "geo", "image", "install", "microphone", "plugin:flash", "popup", "screen", "shortcuts",
      "persistent-storage", "storage-access"];
   if (RESIST_FINGERPRINTING_ENABLED) {
     // Canvas permission should be hidden unless privacy.resistFingerprinting
     // is true.
     expectedPermissions.push("canvas");