Bug 1517014 - Part 1: Add nsIPermissionManager.testPermissionOriginNoSuffix(), an API for testing permissions using an origin string without the overhead of parsing it into a URI; r=nika
authorEhsan Akhgari <ehsan@mozilla.com>
Mon, 07 Jan 2019 18:45:26 +0000
changeset 509832 d316c363485743e50a71e906e3a80139307bd947
parent 509831 24e7758eb4cef7dcf24412a3c0cd33a150b407d4
child 509833 da74b409b3da3a7271521569ee0772a1e0a82aeb
push id10547
push userffxbld-merge
push dateMon, 21 Jan 2019 13:03:58 +0000
treeherdermozilla-beta@24ec1916bffe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnika
bugs1517014
milestone66.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 1517014 - Part 1: Add nsIPermissionManager.testPermissionOriginNoSuffix(), an API for testing permissions using an origin string without the overhead of parsing it into a URI; r=nika For consumers which have an origin string, currently they need to parse it into a URI before they can call testPermission(). Internally, in the common case this nsIURI* argument will be immediately converted back into the same origin string in PermissionKey::CreateFromURI(). This means that the cost of parsing the original origin string will effectively end up being wasted in the common case. This patch adds an API that allows the consumer to test a permission using the origin string directly, and only parse it into an nsIURI when necessary, thereby avoiding this overhead. Differential Revision: https://phabricator.services.mozilla.com/D15543
extensions/cookie/nsPermissionManager.cpp
extensions/cookie/nsPermissionManager.h
extensions/cookie/test/unit/test_permmanager_subdomains.js
netwerk/base/nsIPermissionManager.idl
--- a/extensions/cookie/nsPermissionManager.cpp
+++ b/extensions/cookie/nsPermissionManager.cpp
@@ -755,16 +755,22 @@ nsPermissionManager::PermissionKey::Crea
   aResult = ContentPrincipal::GenerateOriginNoSuffixFromURI(aURI, origin);
   if (NS_WARN_IF(NS_FAILED(aResult))) {
     return nullptr;
   }
 
   return new PermissionKey(origin);
 }
 
+nsPermissionManager::PermissionKey*
+nsPermissionManager::PermissionKey::CreateFromOriginNoSuffix(
+    const nsACString& aOriginNoSuffix) {
+  return new PermissionKey(aOriginNoSuffix);
+}
+
 /**
  * Simple callback used by |AsyncClose| to trigger a treatment once
  * the database is closed.
  *
  * Note: Beware that, if you hold onto a |CloseDatabaseListener| from a
  * |nsPermissionManager|, this will create a cycle.
  *
  * Note: Once the callback has been called this DeleteFromMozHostListener cannot
@@ -2191,16 +2197,24 @@ nsPermissionManager::TestExactPermanentP
 
 NS_IMETHODIMP
 nsPermissionManager::TestPermission(nsIURI* aURI, const char* aType,
                                     uint32_t* aPermission) {
   return CommonTestPermission(aURI, aType, aPermission, false, true);
 }
 
 NS_IMETHODIMP
+nsPermissionManager::TestPermissionOriginNoSuffix(
+    const nsACString& aOriginNoSuffix, const char* aType,
+    uint32_t* aPermission) {
+  return CommonTestPermissionInternal(nullptr, nullptr, aOriginNoSuffix, aType,
+                                      aPermission, false, true);
+}
+
+NS_IMETHODIMP
 nsPermissionManager::TestPermissionFromWindow(mozIDOMWindow* aWindow,
                                               const char* aType,
                                               uint32_t* aPermission) {
   NS_ENSURE_ARG(aWindow);
   nsCOMPtr<nsPIDOMWindowInner> window = nsPIDOMWindowInner::From(aWindow);
 
   // Get the document for security check
   RefPtr<Document> document = window->GetExtantDoc();
@@ -2279,21 +2293,23 @@ nsPermissionManager::GetPermissionObject
   if (NS_WARN_IF(!r)) {
     return NS_ERROR_FAILURE;
   }
   r.forget(aResult);
   return NS_OK;
 }
 
 nsresult nsPermissionManager::CommonTestPermissionInternal(
-    nsIPrincipal* aPrincipal, nsIURI* aURI, const char* aType,
-    uint32_t* aPermission, bool aExactHostMatch, bool aIncludingSession) {
-  MOZ_ASSERT(aPrincipal || aURI);
-  MOZ_ASSERT_IF(aPrincipal, !aURI);
-  NS_ENSURE_ARG_POINTER(aPrincipal || aURI);
+    nsIPrincipal* aPrincipal, nsIURI* aURI, const nsACString& aOriginNoSuffix,
+    const char* aType, uint32_t* aPermission, bool aExactHostMatch,
+    bool aIncludingSession) {
+  MOZ_ASSERT(aPrincipal || aURI || !aOriginNoSuffix.IsEmpty());
+  MOZ_ASSERT_IF(aPrincipal, !aURI && aOriginNoSuffix.IsEmpty());
+  MOZ_ASSERT_IF(aURI, !aPrincipal && aOriginNoSuffix.IsEmpty());
+  NS_ENSURE_ARG_POINTER(aPrincipal || aURI || !aOriginNoSuffix.IsEmpty());
   NS_ENSURE_ARG_POINTER(aType);
 
   if (aPrincipal && nsContentUtils::IsSystemPrincipal(aPrincipal)) {
     *aPermission = nsIPermissionManager::ALLOW_ACTION;
     return NS_OK;
   }
 
   // Set the default.
@@ -2331,31 +2347,37 @@ nsresult nsPermissionManager::CommonTest
 
     return NS_OK;
   }
 
 #ifdef DEBUG
   {
     nsCOMPtr<nsIPrincipal> prin = aPrincipal;
     if (!prin) {
-      prin = mozilla::BasePrincipal::CreateCodebasePrincipal(
-          aURI, OriginAttributes());
+      if (aURI) {
+        prin = mozilla::BasePrincipal::CreateCodebasePrincipal(
+            aURI, OriginAttributes());
+      } else if (!aOriginNoSuffix.IsEmpty()) {
+        prin = mozilla::BasePrincipal::CreateCodebasePrincipal(aOriginNoSuffix);
+      }
     }
+    MOZ_ASSERT(prin);
     MOZ_ASSERT(PermissionAvailable(prin, aType));
   }
 #endif
 
   int32_t typeIndex = GetTypeIndex(aType, false);
   // If type == -1, the type isn't known,
   // so just return NS_OK
   if (typeIndex == -1) return NS_OK;
 
   PermissionHashKey* entry =
       aPrincipal ? GetPermissionHashKey(aPrincipal, typeIndex, aExactHostMatch)
-                 : GetPermissionHashKey(aURI, typeIndex, aExactHostMatch);
+                 : GetPermissionHashKey(aURI, aOriginNoSuffix, typeIndex,
+                                        aExactHostMatch);
   if (!entry || (!aIncludingSession &&
                  entry->GetPermission(typeIndex).mNonSessionExpireType ==
                      nsIPermissionManager::EXPIRE_SESSION)) {
     return NS_OK;
   }
 
   *aPermission = aIncludingSession
                      ? entry->GetPermission(typeIndex).mPermission
@@ -2418,29 +2440,42 @@ nsPermissionManager::GetPermissionHashKe
 }
 
 // Returns PermissionHashKey for a given { host, appId, isInBrowserElement }
 // tuple. This is not simply using PermissionKey because we will walk-up domains
 // in case of |host| contains sub-domains. Returns null if nothing found. Also
 // accepts host on the format "<foo>". This will perform an exact match lookup
 // as the string doesn't contain any dots.
 nsPermissionManager::PermissionHashKey*
-nsPermissionManager::GetPermissionHashKey(nsIURI* aURI, uint32_t aType,
+nsPermissionManager::GetPermissionHashKey(nsIURI* aURI,
+                                          const nsACString& aOriginNoSuffix,
+                                          uint32_t aType,
                                           bool aExactHostMatch) {
+  MOZ_ASSERT(aURI || !aOriginNoSuffix.IsEmpty());
+  MOZ_ASSERT_IF(aURI, aOriginNoSuffix.IsEmpty());
+
 #ifdef DEBUG
   {
     nsCOMPtr<nsIPrincipal> principal;
-    nsresult rv = GetPrincipal(aURI, getter_AddRefs(principal));
+    nsresult rv = NS_OK;
+    if (aURI) {
+      rv = GetPrincipal(aURI, getter_AddRefs(principal));
+    } else {
+      principal =
+          mozilla::BasePrincipal::CreateCodebasePrincipal(aOriginNoSuffix);
+    }
     MOZ_ASSERT_IF(NS_SUCCEEDED(rv),
                   PermissionAvailable(principal, mTypeArray[aType].get()));
   }
 #endif
 
   nsresult rv;
-  RefPtr<PermissionKey> key = PermissionKey::CreateFromURI(aURI, rv);
+  RefPtr<PermissionKey> key =
+      aURI ? PermissionKey::CreateFromURI(aURI, rv)
+           : PermissionKey::CreateFromOriginNoSuffix(aOriginNoSuffix);
   if (!key) {
     return nullptr;
   }
 
   PermissionHashKey* entry = mPermissionTable.GetEntry(key);
 
   if (entry) {
     PermissionEntry permEntry = entry->GetPermission(aType);
@@ -2450,36 +2485,50 @@ nsPermissionManager::GetPermissionHashKe
     if ((permEntry.mExpireType == nsIPermissionManager::EXPIRE_TIME ||
          (permEntry.mExpireType == nsIPermissionManager::EXPIRE_SESSION &&
           permEntry.mExpireTime != 0)) &&
         permEntry.mExpireTime <= (PR_Now() / 1000)) {
       entry = nullptr;
       // If we need to remove a permission we mint a principal.  This is a bit
       // inefficient, but hopefully this code path isn't super common.
       nsCOMPtr<nsIPrincipal> principal;
-      nsresult rv = GetPrincipal(aURI, getter_AddRefs(principal));
-      if (NS_WARN_IF(NS_FAILED(rv))) {
-        return nullptr;
+      if (aURI) {
+        nsresult rv = GetPrincipal(aURI, getter_AddRefs(principal));
+        if (NS_WARN_IF(NS_FAILED(rv))) {
+          return nullptr;
+        }
+      } else {
+        principal =
+            mozilla::BasePrincipal::CreateCodebasePrincipal(aOriginNoSuffix);
       }
       RemoveFromPrincipal(principal, mTypeArray[aType].get());
     } else if (permEntry.mPermission == nsIPermissionManager::UNKNOWN_ACTION) {
       entry = nullptr;
     }
   }
 
   if (entry) {
     return entry;
   }
 
   // If aExactHostMatch wasn't true, we can check if the base domain has a
   // permission entry.
   if (!aExactHostMatch) {
-    nsCOMPtr<nsIURI> uri = GetNextSubDomainURI(aURI);
+    nsCOMPtr<nsIURI> uri;
+    if (aURI) {
+      uri = GetNextSubDomainURI(aURI);
+    } else {
+      nsresult rv = NS_NewURI(getter_AddRefs(uri), aOriginNoSuffix);
+      if (NS_WARN_IF(NS_FAILED(rv))) {
+        return nullptr;
+      }
+      uri = GetNextSubDomainURI(uri);
+    }
     if (uri) {
-      return GetPermissionHashKey(uri, aType, aExactHostMatch);
+      return GetPermissionHashKey(uri, EmptyCString(), aType, aExactHostMatch);
     }
   }
 
   // No entry, really...
   return nullptr;
 }
 
 NS_IMETHODIMP nsPermissionManager::GetEnumerator(nsISimpleEnumerator** aEnum) {
--- a/extensions/cookie/nsPermissionManager.h
+++ b/extensions/cookie/nsPermissionManager.h
@@ -70,16 +70,18 @@ class nsPermissionManager final : public
    * NOTE: It could be implementing nsIHashable but there is no reason to worry
    * with XPCOM interfaces while we don't need to.
    */
   class PermissionKey {
    public:
     static PermissionKey* CreateFromPrincipal(nsIPrincipal* aPrincipal,
                                               nsresult& aResult);
     static PermissionKey* CreateFromURI(nsIURI* aURI, nsresult& aResult);
+    static PermissionKey* CreateFromOriginNoSuffix(
+        const nsACString& aOriginNoSuffix);
 
     explicit PermissionKey(const nsACString& aOrigin) : mOrigin(aOrigin) {}
 
     bool operator==(const PermissionKey& aKey) const {
       return mOrigin.Equals(aKey.mOrigin);
     }
 
     PLDHashNumber GetHashCode() const { return mozilla::HashString(mOrigin); }
@@ -261,33 +263,37 @@ class nsPermissionManager final : public
 
  private:
   virtual ~nsPermissionManager();
 
   int32_t GetTypeIndex(const char* aTypeString, bool aAdd);
 
   PermissionHashKey* GetPermissionHashKey(nsIPrincipal* aPrincipal,
                                           uint32_t aType, bool aExactHostMatch);
-  PermissionHashKey* GetPermissionHashKey(nsIURI* aURI, uint32_t aType,
-                                          bool aExactHostMatch);
+  PermissionHashKey* GetPermissionHashKey(nsIURI* aURI,
+                                          const nsACString& aOriginNoSuffix,
+                                          uint32_t aType, bool aExactHostMatch);
 
   nsresult CommonTestPermission(nsIPrincipal* aPrincipal, const char* aType,
                                 uint32_t* aPermission, bool aExactHostMatch,
                                 bool aIncludingSession) {
-    return CommonTestPermissionInternal(aPrincipal, nullptr, aType, aPermission,
-                                        aExactHostMatch, aIncludingSession);
+    return CommonTestPermissionInternal(aPrincipal, nullptr, EmptyCString(),
+                                        aType, aPermission, aExactHostMatch,
+                                        aIncludingSession);
   }
   nsresult CommonTestPermission(nsIURI* aURI, const char* aType,
                                 uint32_t* aPermission, bool aExactHostMatch,
                                 bool aIncludingSession) {
-    return CommonTestPermissionInternal(nullptr, aURI, aType, aPermission,
-                                        aExactHostMatch, aIncludingSession);
+    return CommonTestPermissionInternal(nullptr, aURI, EmptyCString(), aType,
+                                        aPermission, aExactHostMatch,
+                                        aIncludingSession);
   }
   // Only one of aPrincipal or aURI is allowed to be passed in.
   nsresult CommonTestPermissionInternal(nsIPrincipal* aPrincipal, nsIURI* aURI,
+                                        const nsACString& aOriginNoSuffix,
                                         const char* aType,
                                         uint32_t* aPermission,
                                         bool aExactHostMatch,
                                         bool aIncludingSession);
 
   nsresult OpenDatabase(nsIFile* permissionsFile);
   nsresult InitDB(bool aRemoveFile);
   nsresult CreateTable();
--- a/extensions/cookie/test/unit/test_permmanager_subdomains.js
+++ b/extensions/cookie/test/unit/test_permmanager_subdomains.js
@@ -11,47 +11,64 @@ function getPrincipalFromURI(aURI) {
 function run_test() {
   var pm = Cc["@mozilla.org/permissionmanager;1"].
            getService(Ci.nsIPermissionManager);
 
   // Adds a permission to a sub-domain. Checks if it is working.
   let sub1Principal = getPrincipalFromURI("http://sub1.example.com");
   pm.addFromPrincipal(sub1Principal, "test/subdomains", pm.ALLOW_ACTION, 0, 0);
   Assert.equal(pm.testPermissionFromPrincipal(sub1Principal, "test/subdomains"), pm.ALLOW_ACTION);
+  Assert.equal(pm.testPermissionOriginNoSuffix(sub1Principal.originNoSuffix, "test/subdomains"), pm.ALLOW_ACTION);
 
   // A sub-sub-domain should get the permission.
   let subsubPrincipal = getPrincipalFromURI("http://sub.sub1.example.com");
   Assert.equal(pm.testPermissionFromPrincipal(subsubPrincipal, "test/subdomains"), pm.ALLOW_ACTION);
+  Assert.equal(pm.testPermissionOriginNoSuffix(subsubPrincipal.originNoSuffix, "test/subdomains"), pm.ALLOW_ACTION);
 
   // Another sub-domain shouldn't get the permission.
   let sub2Principal = getPrincipalFromURI("http://sub2.example.com");
   Assert.equal(pm.testPermissionFromPrincipal(sub2Principal, "test/subdomains"), pm.UNKNOWN_ACTION);
+  Assert.equal(pm.testPermissionOriginNoSuffix(sub2Principal.originNoSuffix, "test/subdomains"), pm.UNKNOWN_ACTION);
 
   // Remove current permissions.
   pm.removeFromPrincipal(sub1Principal, "test/subdomains");
   Assert.equal(pm.testPermissionFromPrincipal(sub1Principal, "test/subdomains"), pm.UNKNOWN_ACTION);
+  Assert.equal(pm.testPermissionOriginNoSuffix(sub1Principal.originNoSuffix, "test/subdomains"), pm.UNKNOWN_ACTION);
 
   // Adding the permission to the main domain. Checks if it is working.
   let mainPrincipal = getPrincipalFromURI("http://example.com");
   pm.addFromPrincipal(mainPrincipal, "test/subdomains", pm.ALLOW_ACTION, 0, 0);
   Assert.equal(pm.testPermissionFromPrincipal(mainPrincipal, "test/subdomains"), pm.ALLOW_ACTION);
+  Assert.equal(pm.testPermissionOriginNoSuffix(mainPrincipal.originNoSuffix, "test/subdomains"), pm.ALLOW_ACTION);
 
   // All sub-domains should have the permission now.
   Assert.equal(pm.testPermissionFromPrincipal(sub1Principal, "test/subdomains"), pm.ALLOW_ACTION);
+  Assert.equal(pm.testPermissionOriginNoSuffix(sub1Principal.originNoSuffix, "test/subdomains"), pm.ALLOW_ACTION);
   Assert.equal(pm.testPermissionFromPrincipal(sub2Principal, "test/subdomains"), pm.ALLOW_ACTION);
+  Assert.equal(pm.testPermissionOriginNoSuffix(sub2Principal.originNoSuffix, "test/subdomains"), pm.ALLOW_ACTION);
   Assert.equal(pm.testPermissionFromPrincipal(subsubPrincipal, "test/subdomains"), pm.ALLOW_ACTION);
+  Assert.equal(pm.testPermissionOriginNoSuffix(subsubPrincipal.originNoSuffix, "test/subdomains"), pm.ALLOW_ACTION);
 
   // Remove current permissions.
   pm.removeFromPrincipal(mainPrincipal, "test/subdomains");
   Assert.equal(pm.testPermissionFromPrincipal(mainPrincipal, "test/subdomains"), pm.UNKNOWN_ACTION);
+  Assert.equal(pm.testPermissionOriginNoSuffix(mainPrincipal.originNoSuffix, "test/subdomains"), pm.UNKNOWN_ACTION);
   Assert.equal(pm.testPermissionFromPrincipal(sub1Principal, "test/subdomains"), pm.UNKNOWN_ACTION);
+  Assert.equal(pm.testPermissionOriginNoSuffix(sub1Principal.originNoSuffix, "test/subdomains"), pm.UNKNOWN_ACTION);
   Assert.equal(pm.testPermissionFromPrincipal(sub2Principal, "test/subdomains"), pm.UNKNOWN_ACTION);
+  Assert.equal(pm.testPermissionOriginNoSuffix(sub2Principal.originNoSuffix, "test/subdomains"), pm.UNKNOWN_ACTION);
   Assert.equal(pm.testPermissionFromPrincipal(subsubPrincipal, "test/subdomains"), pm.UNKNOWN_ACTION);
+  Assert.equal(pm.testPermissionOriginNoSuffix(subsubPrincipal.originNoSuffix, "test/subdomains"), pm.UNKNOWN_ACTION);
 
   // A sanity check that the previous implementation wasn't passing...
   let crazyPrincipal = getPrincipalFromURI("http://com");
   pm.addFromPrincipal(crazyPrincipal, "test/subdomains", pm.ALLOW_ACTION, 0, 0);
   Assert.equal(pm.testPermissionFromPrincipal(crazyPrincipal, "test/subdomains"),  pm.ALLOW_ACTION);
+  Assert.equal(pm.testPermissionOriginNoSuffix(crazyPrincipal.originNoSuffix, "test/subdomains"),  pm.ALLOW_ACTION);
   Assert.equal(pm.testPermissionFromPrincipal(mainPrincipal, "test/subdomains"),   pm.UNKNOWN_ACTION);
+  Assert.equal(pm.testPermissionOriginNoSuffix(mainPrincipal.originNoSuffix, "test/subdomains"),   pm.UNKNOWN_ACTION);
   Assert.equal(pm.testPermissionFromPrincipal(sub1Principal, "test/subdomains"),   pm.UNKNOWN_ACTION);
+  Assert.equal(pm.testPermissionOriginNoSuffix(sub1Principal.originNoSuffix, "test/subdomains"),   pm.UNKNOWN_ACTION);
   Assert.equal(pm.testPermissionFromPrincipal(sub2Principal, "test/subdomains"),   pm.UNKNOWN_ACTION);
+  Assert.equal(pm.testPermissionOriginNoSuffix(sub2Principal.originNoSuffix, "test/subdomains"),   pm.UNKNOWN_ACTION);
   Assert.equal(pm.testPermissionFromPrincipal(subsubPrincipal, "test/subdomains"), pm.UNKNOWN_ACTION);
+  Assert.equal(pm.testPermissionOriginNoSuffix(subsubPrincipal.originNoSuffix, "test/subdomains"), pm.UNKNOWN_ACTION);
 }
--- a/netwerk/base/nsIPermissionManager.idl
+++ b/netwerk/base/nsIPermissionManager.idl
@@ -207,16 +207,31 @@ interface nsIPermissionManager : nsISupp
    * System principals will always have permissions granted.
    * This function will perform a pref lookup to permissions.default.<type>
    * if the specific permission type is part of the whitelist for that functionality.
    */
   uint32_t testPermissionFromPrincipal(in nsIPrincipal principal,
                                        in string type);
 
   /**
+   * Test whether a website specified by a given origin string has permission
+   * to perform the given action.  This function is similar to testPermission()
+   * and is intended to be used where the cost of parsing a URI in the common
+   * case is to be avoided.
+   * @param originNoSuffix the origin string to be tested.
+   * @param type           a case-sensitive ASCII string, identifying the
+   *                       permission.
+   * @param return         see add(), param permission. returns UNKNOWN_ACTION
+   *                       when there is no stored permission for this uri and/
+   *                       or type.
+   */
+  uint32_t testPermissionOriginNoSuffix(in ACString originNoSuffix,
+                                        in string type);
+
+  /**
    * Test whether the principal associated with the window's document has the
    * permission to perform a given action.  System principals will always
    * have permissions granted.
    * This function will perform a pref lookup to permissions.default.<type>
    * if the specific permission type is part of the whitelist for that functionality.
    */
   uint32_t testPermissionFromWindow(in mozIDOMWindow window,
                                     in string type);