Bug 1322518 - PermissionKey should propagate the error if nsIPrincipal::GetOrigin fails, r=ehsan
authorAndrea Marchesini <amarchesini@mozilla.com>
Thu, 08 Dec 2016 10:45:52 -1000
changeset 325454 8f375033dba4ea7216fda052f7b921e80db2ad8d
parent 325453 b357fbee89ca5d0303b74cf9a822a66e590a1549
child 325455 28d3c72eaecdc719fa4a01d5bb2d345b216070a9
push id24
push usermaklebus@msu.edu
push dateTue, 20 Dec 2016 03:11:33 +0000
reviewersehsan
bugs1322518
milestone53.0a1
Bug 1322518 - PermissionKey should propagate the error if nsIPrincipal::GetOrigin fails, r=ehsan
extensions/cookie/nsPermissionManager.cpp
extensions/cookie/nsPermissionManager.h
--- a/extensions/cookie/nsPermissionManager.cpp
+++ b/extensions/cookie/nsPermissionManager.cpp
@@ -599,19 +599,27 @@ IsExpandedPrincipal(nsIPrincipal* aPrinc
   nsCOMPtr<nsIExpandedPrincipal> ep = do_QueryInterface(aPrincipal);
   return !!ep;
 }
 
 } // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
 
-nsPermissionManager::PermissionKey::PermissionKey(nsIPrincipal* aPrincipal)
+nsPermissionManager::PermissionKey*
+nsPermissionManager::PermissionKey::CreateFromPrincipal(nsIPrincipal* aPrincipal,
+                                                        nsresult& aResult)
 {
-  MOZ_ALWAYS_SUCCEEDS(GetOriginFromPrincipal(aPrincipal, mOrigin));
+  nsAutoCString origin;
+  aResult = GetOriginFromPrincipal(aPrincipal, origin);
+  if (NS_WARN_IF(NS_FAILED(aResult))) {
+    return nullptr;
+  }
+
+  return new PermissionKey(origin);
 }
 
 /**
  * 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.
@@ -1579,17 +1587,23 @@ nsPermissionManager::AddInternal(nsIPrin
   }
 
   // look up the type index
   int32_t typeIndex = GetTypeIndex(aType.get(), true);
   NS_ENSURE_TRUE(typeIndex != -1, NS_ERROR_OUT_OF_MEMORY);
 
   // When an entry already exists, PutEntry will return that, instead
   // of adding a new one
-  RefPtr<PermissionKey> key = new PermissionKey(aPrincipal);
+  RefPtr<PermissionKey> key =
+    PermissionKey::CreateFromPrincipal(aPrincipal, rv);
+  if (!key) {
+    MOZ_ASSERT(NS_FAILED(rv));
+    return rv;
+  }
+
   PermissionHashKey* entry = mPermissionTable.PutEntry(key);
   if (!entry) return NS_ERROR_FAILURE;
   if (!entry->GetKey()) {
     mPermissionTable.RemoveEntry(entry);
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
   // figure out the transaction type, and get any existing permission value
@@ -2128,20 +2142,24 @@ nsPermissionManager::CommonTestPermissio
 // 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(nsIPrincipal* aPrincipal,
                                           uint32_t aType,
                                           bool aExactHostMatch)
 {
-  PermissionHashKey* entry = nullptr;
-
-  RefPtr<PermissionKey> key = new PermissionKey(aPrincipal);
-  entry = mPermissionTable.GetEntry(key);
+  nsresult rv;
+  RefPtr<PermissionKey> key =
+    PermissionKey::CreateFromPrincipal(aPrincipal, rv);
+  if (!key) {
+    return nullptr;
+  }
+
+  PermissionHashKey* entry = mPermissionTable.GetEntry(key);
 
   if (entry) {
     PermissionEntry permEntry = entry->GetPermission(aType);
 
     // if the entry is expired, remove and keep looking for others.
     // Note that EXPIRE_SESSION only honors expireTime if it is nonzero.
     if ((permEntry.mExpireType == nsIPermissionManager::EXPIRE_TIME ||
          (permEntry.mExpireType == nsIPermissionManager::EXPIRE_SESSION &&
@@ -2247,17 +2265,22 @@ NS_IMETHODIMP nsPermissionManager::GetEn
 NS_IMETHODIMP nsPermissionManager::GetAllForURI(nsIURI* aURI, nsISimpleEnumerator **aEnum)
 {
   nsCOMArray<nsIPermission> array;
 
   nsCOMPtr<nsIPrincipal> principal;
   nsresult rv = GetPrincipal(aURI, getter_AddRefs(principal));
   NS_ENSURE_SUCCESS(rv, rv);
 
-  RefPtr<PermissionKey> key = new PermissionKey(principal);
+  RefPtr<PermissionKey> key = PermissionKey::CreateFromPrincipal(principal, rv);
+  if (!key) {
+    MOZ_ASSERT(NS_FAILED(rv));
+    return rv;
+  }
+
   PermissionHashKey* entry = mPermissionTable.GetEntry(key);
 
   if (entry) {
     for (const auto& permEntry : entry->GetPermissions()) {
       // Only return custom permissions
       if (permEntry.mPermission == nsIPermissionManager::UNKNOWN_ACTION) {
         continue;
       }
--- a/extensions/cookie/nsPermissionManager.h
+++ b/extensions/cookie/nsPermissionManager.h
@@ -66,17 +66,19 @@ public:
    * PermissionKey is the key used by PermissionHashKey hash table.
    *
    * 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:
-    explicit PermissionKey(nsIPrincipal* aPrincipal);
+    static PermissionKey* CreateFromPrincipal(nsIPrincipal* aPrincipal,
+                                              nsresult& aResult);
+
     explicit PermissionKey(const nsACString& aOrigin)
       : mOrigin(aOrigin)
     {
     }
 
     bool operator==(const PermissionKey& aKey) const {
       return mOrigin.Equals(aKey.mOrigin);
     }