Bug 777072 - 2/7 - AddInternal() and CommonTestPermission() should use nsIPrincipal. r=sicking
authorMounir Lamouri <mounir.lamouri@gmail.com>
Sat, 25 Aug 2012 09:53:48 -0700
changeset 105522 0e1c51a8387d2298be9a4202301d406c80f22759
parent 105521 2b4f2729b40176a6e54df204955584bf5ec6025f
child 105523 ef0744b50b6112938601a17f170292c880dc6943
push id55
push usershu@rfrn.org
push dateThu, 30 Aug 2012 01:33:09 +0000
reviewerssicking
bugs777072
milestone17.0a1
Bug 777072 - 2/7 - AddInternal() and CommonTestPermission() should use nsIPrincipal. r=sicking
dom/ipc/ContentChild.cpp
extensions/cookie/nsPermissionManager.cpp
extensions/cookie/nsPermissionManager.h
--- a/dom/ipc/ContentChild.cpp
+++ b/dom/ipc/ContentChild.cpp
@@ -90,16 +90,19 @@
 #include "mozilla/dom/indexedDB/PIndexedDBChild.h"
 #include "mozilla/dom/sms/SmsChild.h"
 #include "mozilla/dom/devicestorage/DeviceStorageRequestChild.h"
 
 #include "nsDOMFile.h"
 #include "nsIRemoteBlob.h"
 #include "StructuredCloneUtils.h"
 #include "URIUtils.h"
+#include "nsIScriptSecurityManager.h"
+#include "nsContentUtils.h"
+#include "nsIPrincipal.h"
 
 using namespace mozilla::docshell;
 using namespace mozilla::dom::devicestorage;
 using namespace mozilla::dom::sms;
 using namespace mozilla::dom::indexedDB;
 using namespace mozilla::hal_sandbox;
 using namespace mozilla::ipc;
 using namespace mozilla::layers;
@@ -842,17 +845,28 @@ ContentChild::RecvAddPermission(const IP
 #if MOZ_PERMISSIONS
   nsCOMPtr<nsIPermissionManager> permissionManagerIface =
       do_GetService(NS_PERMISSIONMANAGER_CONTRACTID);
   nsPermissionManager* permissionManager =
       static_cast<nsPermissionManager*>(permissionManagerIface.get());
   NS_ABORT_IF_FALSE(permissionManager, 
                    "We have no permissionManager in the Content process !");
 
-  permissionManager->AddInternal(nsCString(permission.host),
+  nsCOMPtr<nsIURI> uri;
+  NS_NewURI(getter_AddRefs(uri), NS_LITERAL_CSTRING("http://") + nsCString(permission.host));
+  NS_ENSURE_TRUE(uri, true);
+
+  nsIScriptSecurityManager* secMan = nsContentUtils::GetSecurityManager();
+  MOZ_ASSERT(secMan);
+
+  nsCOMPtr<nsIPrincipal> principal;
+  nsresult rv = secMan->GetNoAppCodebasePrincipal(uri, getter_AddRefs(principal));
+  NS_ENSURE_SUCCESS(rv, true);
+
+  permissionManager->AddInternal(principal,
                                  nsCString(permission.type),
                                  permission.capability,
                                  0,
                                  permission.expireType,
                                  permission.expireTime,
                                  nsPermissionManager::eNotify,
                                  nsPermissionManager::eNoDBOperation);
 #endif
--- a/extensions/cookie/nsPermissionManager.cpp
+++ b/extensions/cookie/nsPermissionManager.cpp
@@ -88,16 +88,34 @@ ArenaStrDup(const char* str, PLArenaPool
   void* mem;
   const uint32_t size = strlen(str) + 1;
   PL_ARENA_ALLOCATE(mem, aArena, size);
   if (mem)
     memcpy(mem, str, size);
   return static_cast<char*>(mem);
 }
 
+namespace {
+
+nsresult
+GetPrincipalForHost(const nsACString& aHost, nsIPrincipal** aPrincipal)
+{
+  nsIScriptSecurityManager* secMan = nsContentUtils::GetSecurityManager();
+  NS_ENSURE_TRUE(secMan, NS_ERROR_FAILURE);
+
+  nsCOMPtr<nsIURI> uri;
+  // NOTE: we use "http://" as a protocal but we will just use the host so it
+  // doesn't really matter.
+  NS_NewURI(getter_AddRefs(uri), NS_LITERAL_CSTRING("http://") + aHost);
+
+  return secMan->GetNoAppCodebasePrincipal(uri, aPrincipal);
+}
+
+} // anonymous namespace
+
 nsHostEntry::nsHostEntry(const char* aHost)
 {
   mHost = ArenaStrDup(aHost, gHostArena);
 }
 
 // XXX this can fail on OOM
 nsHostEntry::nsHostEntry(const nsHostEntry& toCopy)
  : mHost(toCopy.mHost)
@@ -279,17 +297,22 @@ nsPermissionManager::Init()
 
   if (IsChildProcess()) {
     // Get the permissions from the parent process
     InfallibleTArray<IPC::Permission> perms;
     ChildProcess()->SendReadPermissions(&perms);
 
     for (uint32_t i = 0; i < perms.Length(); i++) {
       const IPC::Permission &perm = perms[i];
-      AddInternal(perm.host, perm.type, perm.capability, 0, perm.expireType,
+
+      nsCOMPtr<nsIPrincipal> principal;
+      nsresult rv = GetPrincipalForHost(perm.host, getter_AddRefs(principal));
+      NS_ENSURE_SUCCESS(rv, rv);
+
+      AddInternal(principal, perm.type, perm.capability, 0, perm.expireType,
                   perm.expireTime, eNotify, eNoDBOperation);
     }
 
     // Stop here; we don't need the DB in the child process
     return NS_OK;
   }
 
   // ignore failure here, since it's non-fatal (we can run fine without
@@ -516,40 +539,40 @@ nsPermissionManager::AddFromPrincipal(ns
   }
 
   // We don't add the system principal because it actually has no URI and we
   // always allow action for them.
   if (nsContentUtils::IsSystemPrincipal(aPrincipal)) {
     return NS_OK;
   }
 
+  return AddInternal(aPrincipal, nsDependentCString(aType), aPermission, 0,
+                     aExpireType, aExpireTime, eNotify, eWriteToDB);
+}
+
+nsresult
+nsPermissionManager::AddInternal(nsIPrincipal* aPrincipal,
+                                 const nsAFlatCString &aType,
+                                 uint32_t              aPermission,
+                                 int64_t               aID,
+                                 uint32_t              aExpireType,
+                                 int64_t               aExpireTime,
+                                 NotifyOperationType   aNotifyOperation,
+                                 DBOperationType       aDBOperation)
+{
   nsCOMPtr<nsIURI> uri;
   nsresult rv = aPrincipal->GetURI(getter_AddRefs(uri));
   NS_ENSURE_SUCCESS(rv, rv);
 
   nsCAutoString host;
   rv = GetHost(uri, host);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  return AddInternal(host, nsDependentCString(aType), aPermission, 0, 
-                     aExpireType, aExpireTime, eNotify, eWriteToDB);
-}
-
-nsresult
-nsPermissionManager::AddInternal(const nsAFlatCString &aHost,
-                                 const nsAFlatCString &aType,
-                                 uint32_t              aPermission,
-                                 int64_t               aID,
-                                 uint32_t              aExpireType,
-                                 int64_t               aExpireTime,
-                                 NotifyOperationType   aNotifyOperation,
-                                 DBOperationType       aDBOperation)
-{
   if (!IsChildProcess()) {
-    IPC::Permission permission((aHost),
+    IPC::Permission permission((host),
                                (aType),
                                aPermission, aExpireType, aExpireTime);
 
     nsTArray<ContentParent*> cplist;
     ContentParent::GetAll(cplist);
     for (uint32_t i = 0; i < cplist.Length(); ++i) {
       ContentParent* cp = cplist[i];
       if (cp->NeedsPermissionsUpdate())
@@ -565,17 +588,17 @@ nsPermissionManager::AddInternal(const n
   }
 
   // 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
-  nsHostEntry *entry = mHostTable.PutEntry(aHost.get());
+  nsHostEntry *entry = mHostTable.PutEntry(host.get());
   if (!entry) return NS_ERROR_FAILURE;
   if (!entry->GetKey()) {
     mHostTable.RawRemoveEntry(entry);
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
   // figure out the transaction type, and get any existing permission value
   OperationType op;
@@ -623,20 +646,20 @@ nsPermissionManager::AddInternal(const n
       } else {
         // we're reading from the database - use the id already assigned
         id = aID;
       }
 
       entry->GetPermissions().AppendElement(nsPermissionEntry(typeIndex, aPermission, id, aExpireType, aExpireTime));
 
       if (aDBOperation == eWriteToDB && aExpireType != nsIPermissionManager::EXPIRE_SESSION)
-        UpdateDB(op, mStmtInsert, id, aHost, aType, aPermission, aExpireType, aExpireTime);
+        UpdateDB(op, mStmtInsert, id, host, aType, aPermission, aExpireType, aExpireTime);
 
       if (aNotifyOperation == eNotify) {
-        NotifyObserversWithPermission(aHost,
+        NotifyObserversWithPermission(host,
                                       mTypeArray[typeIndex],
                                       aPermission,
                                       aExpireType,
                                       aExpireTime,
                                       NS_LITERAL_STRING("added").get());
       }
 
       break;
@@ -652,17 +675,17 @@ nsPermissionManager::AddInternal(const n
       if (entry->GetPermissions().IsEmpty())
         mHostTable.RawRemoveEntry(entry);
 
       if (aDBOperation == eWriteToDB)
         UpdateDB(op, mStmtDelete, id, EmptyCString(), EmptyCString(), 0, 
                  nsIPermissionManager::EXPIRE_NEVER, 0);
 
       if (aNotifyOperation == eNotify) {
-        NotifyObserversWithPermission(aHost,
+        NotifyObserversWithPermission(host,
                                       mTypeArray[typeIndex],
                                       oldPermissionEntry.mPermission,
                                       oldPermissionEntry.mExpireType,
                                       oldPermissionEntry.mExpireTime,
                                       NS_LITERAL_STRING("deleted").get());
       }
 
       break;
@@ -672,17 +695,17 @@ nsPermissionManager::AddInternal(const n
     {
       id = entry->GetPermissions()[index].mID;
       entry->GetPermissions()[index].mPermission = aPermission;
 
       if (aDBOperation == eWriteToDB && aExpireType != nsIPermissionManager::EXPIRE_SESSION)
         UpdateDB(op, mStmtUpdate, id, EmptyCString(), EmptyCString(), aPermission, aExpireType, aExpireTime);
 
       if (aNotifyOperation == eNotify) {
-        NotifyObserversWithPermission(aHost,
+        NotifyObserversWithPermission(host,
                                       mTypeArray[typeIndex],
                                       aPermission,
                                       aExpireType,
                                       aExpireTime,
                                       NS_LITERAL_STRING("changed").get());
       }
 
       break;
@@ -692,25 +715,18 @@ nsPermissionManager::AddInternal(const n
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsPermissionManager::Remove(const nsACString &aHost,
                             const char       *aType)
 {
   nsCOMPtr<nsIPrincipal> principal;
-  nsIScriptSecurityManager* secMan = nsContentUtils::GetSecurityManager();
-  MOZ_ASSERT(secMan, "No security manager!?");
 
-  nsCOMPtr<nsIURI> uri;
-  // NOTE: we use "http://" as a protocal but we will just use the host so it
-  // doesn't really matter.
-  NS_NewURI(getter_AddRefs(uri), NS_LITERAL_CSTRING("http://") + aHost);
-
-  nsresult rv = secMan->GetNoAppCodebasePrincipal(uri, getter_AddRefs(principal));
+  nsresult rv = GetPrincipalForHost(aHost, getter_AddRefs(principal));
   NS_ENSURE_SUCCESS(rv, rv);
 
   return RemoveFromPrincipal(principal, aType);
 }
 
 NS_IMETHODIMP
 nsPermissionManager::RemoveFromPrincipal(nsIPrincipal* aPrincipal,
                                          const char* aType)
@@ -719,26 +735,18 @@ nsPermissionManager::RemoveFromPrincipal
   NS_ENSURE_ARG_POINTER(aPrincipal);
   NS_ENSURE_ARG_POINTER(aType);
 
   // System principals are never added to the database, no need to remove them.
   if (nsContentUtils::IsSystemPrincipal(aPrincipal)) {
     return NS_OK;
   }
 
-  nsCOMPtr<nsIURI> uri;
-  nsresult rv = aPrincipal->GetURI(getter_AddRefs(uri));
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  nsCAutoString host;
-  rv = GetHost(uri, host);
-  NS_ENSURE_SUCCESS(rv, rv);
-
   // AddInternal() handles removal, just let it do the work
-  return AddInternal(PromiseFlatCString(host),
+  return AddInternal(aPrincipal,
                      nsDependentCString(aType),
                      nsIPermissionManager::UNKNOWN_ACTION,
                      0,
                      nsIPermissionManager::EXPIRE_NEVER,
                      0,
                      eNotify,
                      eWriteToDB);
 }
@@ -823,20 +831,17 @@ nsPermissionManager::TestExactPermission
 
   // System principals do not have URI so we can't try to get
   // retro-compatibility here.
   if (nsContentUtils::IsSystemPrincipal(aPrincipal)) {
     *aPermission = nsIPermissionManager::ALLOW_ACTION;
     return NS_OK;
   }
 
-  nsCOMPtr<nsIURI> uri;
-  aPrincipal->GetURI(getter_AddRefs(uri));
-
-  return CommonTestPermission(uri, aType, aPermission, true);
+  return CommonTestPermission(aPrincipal, aType, aPermission, true);
 }
 
 NS_IMETHODIMP
 nsPermissionManager::TestPermission(nsIURI     *aURI,
                                     const char *aType,
                                     uint32_t   *aPermission)
 {
   nsCOMPtr<nsIPrincipal> principal;
@@ -858,50 +863,52 @@ nsPermissionManager::TestPermissionFromP
 
   // System principals do not have URI so we can't try to get
   // retro-compatibility here.
   if (nsContentUtils::IsSystemPrincipal(aPrincipal)) {
     *aPermission = nsIPermissionManager::ALLOW_ACTION;
     return NS_OK;
   }
 
-  nsCOMPtr<nsIURI> uri;
-  aPrincipal->GetURI(getter_AddRefs(uri));
-
-  return CommonTestPermission(uri, aType, aPermission, false);
+  return CommonTestPermission(aPrincipal, aType, aPermission, false);
 }
 
 nsresult
-nsPermissionManager::CommonTestPermission(nsIURI     *aURI,
+nsPermissionManager::CommonTestPermission(nsIPrincipal* aPrincipal,
                                           const char *aType,
                                           uint32_t   *aPermission,
                                           bool        aExactHostMatch)
 {
-  NS_ENSURE_ARG_POINTER(aURI);
+  NS_ENSURE_ARG_POINTER(aPrincipal);
   NS_ENSURE_ARG_POINTER(aType);
 
   // set the default
   *aPermission = nsIPermissionManager::UNKNOWN_ACTION;
 
+  nsCOMPtr<nsIURI> uri;
+  nsresult rv = aPrincipal->GetURI(getter_AddRefs(uri));
+  NS_ENSURE_SUCCESS(rv, rv);
+
   nsCAutoString host;
-  nsresult rv = GetHost(aURI, host);
+  rv = GetHost(uri, host);
+
   // No host doesn't mean an error. Just return the default. Unless this is
   // a file uri. In that case use a magic host.
   if (NS_FAILED(rv)) {
     bool isFile;
-    rv = aURI->SchemeIs("file", &isFile);
+    rv = uri->SchemeIs("file", &isFile);
     NS_ENSURE_SUCCESS(rv, rv);
     if (isFile) {
       host.AssignLiteral("<file>");
     }
     else {
       return NS_OK;
     }
   }
-  
+
   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;
 
   nsHostEntry *entry = GetHostEntry(host, typeIndex, aExactHostMatch);
   if (entry)
     *aPermission = entry->GetPermission(typeIndex).mPermission;
@@ -1142,17 +1149,21 @@ nsPermissionManager::Read()
     NS_ENSURE_SUCCESS(rv, rv);
 
     permission = stmt->AsInt32(3);
     expireType = stmt->AsInt32(4);
 
     // convert into int64_t value (milliseconds)
     expireTime = stmt->AsInt64(5);
 
-    rv = AddInternal(host, type, permission, id, expireType, expireTime,
+    nsCOMPtr<nsIPrincipal> principal;
+    nsresult rv = GetPrincipalForHost(host, getter_AddRefs(principal));
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    rv = AddInternal(principal, type, permission, id, expireType, expireTime,
                      eDontNotify, eNoDBOperation);
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
   return NS_OK;
 }
 
 static const char kMatchTypeHost[] = "host";
@@ -1212,17 +1223,21 @@ nsPermissionManager::Import()
 
       // hosts might be encoded in UTF8; switch them to ACE to be consistent
       if (!IsASCII(lineArray[3])) {
         rv = NormalizeToACE(lineArray[3]);
         if (NS_FAILED(rv))
           continue;
       }
 
-      rv = AddInternal(lineArray[3], lineArray[1], permission, 0, 
+      nsCOMPtr<nsIPrincipal> principal;
+      nsresult rv = GetPrincipalForHost(lineArray[3], getter_AddRefs(principal));
+      NS_ENSURE_SUCCESS(rv, rv);
+
+      rv = AddInternal(principal, lineArray[1], permission, 0,
                        nsIPermissionManager::EXPIRE_NEVER, 0, eDontNotify, eWriteToDB);
       NS_ENSURE_SUCCESS(rv, rv);
     }
   }
 
   // we're done importing - delete the old file
   permissionsFile->Remove(false);
 
--- a/extensions/cookie/nsPermissionManager.h
+++ b/extensions/cookie/nsPermissionManager.h
@@ -149,17 +149,17 @@ public:
     eWriteToDB
   };
 
   enum NotifyOperationType {
     eDontNotify,
     eNotify
   };
 
-  nsresult AddInternal(const nsAFlatCString &aHost,
+  nsresult AddInternal(nsIPrincipal* aPrincipal,
                        const nsAFlatCString &aType,
                        uint32_t aPermission,
                        int64_t aID,
                        uint32_t aExpireType,
                        int64_t  aExpireTime,
                        NotifyOperationType aNotifyOperation,
                        DBOperationType aDBOperation);
 
@@ -167,17 +167,17 @@ private:
 
   int32_t GetTypeIndex(const char *aTypeString,
                        bool        aAdd);
 
   nsHostEntry *GetHostEntry(const nsAFlatCString &aHost,
                             uint32_t              aType,
                             bool                  aExactHostMatch);
 
-  nsresult CommonTestPermission(nsIURI     *aURI,
+  nsresult CommonTestPermission(nsIPrincipal* aPrincipal,
                                 const char *aType,
                                 uint32_t   *aPermission,
                                 bool        aExactHostMatch);
 
   nsresult InitDB(bool aRemoveFile);
   nsresult CreateTable();
   nsresult Import();
   nsresult Read();