Bug 1407428: Hand out a const array reference for expanded principal whiteList. r=krizsa

The current API makes the life time and ownership of the result array unclear
without careful reading. The result array is always owned by the principal,
and its lifetime tied to the lifetime of the principal itself. Returning a
const array reference makes this clear, and should prevent callers from
accidentally modifying the returned array.

MozReview-Commit-ID: 3f8mhynkKAj

--HG--
extra : source : 237acf2879f6222bc4b076c377bf026d18a6ebef
extra : amend_source : dfaf6e88e3c4758f7fdcf7fb422d457edafab1b7

caps/ExpandedPrincipal.cpp

@@ -100,15 +100,14 @@ ExpandedPrincipal::SubsumesInternal(nsIPrincipal* aOther,
 {
   // If aOther is an ExpandedPrincipal too, we break it down into its component
   // nsIPrincipals, and check subsumes on each one.
-  nsCOMPtr<nsIExpandedPrincipal> expanded = do_QueryInterface(aOther);
-  if (expanded) {
-    nsTArray< nsCOMPtr<nsIPrincipal> >* otherList;
-    expanded->GetWhiteList(&otherList);
-    for (uint32_t i = 0; i < otherList->Length(); ++i){
+  if (Cast(aOther)->Is<ExpandedPrincipal>()) {
+    auto* expanded = Cast(aOther)->As<ExpandedPrincipal>();
+
+    for (auto& other : expanded->WhiteList()) {
       // Use SubsumesInternal rather than Subsumes here, since OriginAttribute
       // checks are only done between non-expanded sub-principals, and we don't
       // need to incur the extra virtual call overhead.
-      if (!SubsumesInternal((*otherList)[i], aConsideration)) {
+      if (!SubsumesInternal(other, aConsideration)) {
         return false;
       }
     }
@@ -151,11 +150,10 @@ ExpandedPrincipal::GetURI(nsIURI** aURI)
   return NS_OK;
 }
 
-NS_IMETHODIMP
-ExpandedPrincipal::GetWhiteList(nsTArray<nsCOMPtr<nsIPrincipal> >** aWhiteList)
+const nsTArray<nsCOMPtr<nsIPrincipal>>&
+ExpandedPrincipal::WhiteList()
 {
-  *aWhiteList = &mPrincipals;
-  return NS_OK;
+  return mPrincipals;
 }
 
 NS_IMETHODIMP

caps/nsIPrincipal.idl

@@ -42,7 +42,7 @@ interface nsIDOMDocument;
 
 [ptr] native JSContext(JSContext);
 [ptr] native JSPrincipals(JSPrincipals);
-[ptr] native PrincipalArray(nsTArray<nsCOMPtr<nsIPrincipal> >);
+[ref] native PrincipalArray(const nsTArray<nsCOMPtr<nsIPrincipal>>);
 [ref] native const_OriginAttributes(const mozilla::OriginAttributes);
 
 [scriptable, builtinclass, uuid(f75f502d-79fd-48be-a079-e5a7b8f80c8b)]
@@ -340,5 +340,6 @@ interface nsIExpandedPrincipal : nsISupports
    * Note: this list is not reference counted, it is shared, so
    * should not be changed and should only be used ephemerally.
    */
-  [noscript] readonly attribute PrincipalArray whiteList;
+  [noscript, notxpcom, nostdcall]
+  PrincipalArray WhiteList();
 };

caps/nsScriptSecurityManager.cpp

@@ -20,6 +20,7 @@
 #include "nspr.h"
 #include "nsJSPrincipals.h"
 #include "mozilla/BasePrincipal.h"
+#include "ExpandedPrincipal.h"
 #include "SystemPrincipal.h"
 #include "NullPrincipal.h"
 #include "DomainPolicy.h"
@@ -668,12 +669,11 @@ nsScriptSecurityManager::CheckLoadURIWithPrincipal(nsIPrincipal* aPrincipal,
     nsCOMPtr<nsIURI> sourceURI;
     aPrincipal->GetURI(getter_AddRefs(sourceURI));
     if (!sourceURI) {
-        nsCOMPtr<nsIExpandedPrincipal> expanded = do_QueryInterface(aPrincipal);
-        if (expanded) {
-            nsTArray< nsCOMPtr<nsIPrincipal> > *whiteList;
-            expanded->GetWhiteList(&whiteList);
-            for (uint32_t i = 0; i < whiteList->Length(); ++i) {
-                nsresult rv = CheckLoadURIWithPrincipal((*whiteList)[i],
+        auto* basePrin = BasePrincipal::Cast(aPrincipal);
+        if (basePrin->Is<ExpandedPrincipal>()) {
+            auto expanded = basePrin->As<ExpandedPrincipal>();
+            for (auto& prin : expanded->WhiteList()) {
+                nsresult rv = CheckLoadURIWithPrincipal(prin,
                                                         aTargetURI,
                                                         aFlags);
                 if (NS_SUCCEEDED(rv)) {

dom/base/nsDocument.cpp

@@ -2494,11 +2494,9 @@ nsDocument::MaybeDowngradePrincipal(nsIPrincipal* aPrincipal)
   if (basePrin->Is<ExpandedPrincipal>()) {
     auto* expanded = basePrin->As<ExpandedPrincipal>();
 
-    nsTArray<nsCOMPtr<nsIPrincipal>>* whitelist;
-    MOZ_ALWAYS_SUCCEEDS(expanded->GetWhiteList(&whitelist));
-    MOZ_ASSERT(whitelist->Length() > 0);
+    MOZ_ASSERT(expanded->WhiteList().Length() > 0);
 
-    return do_AddRef(whitelist->LastElement().get());
+    return do_AddRef(expanded->WhiteList().LastElement().get());
   }
 
   if (!sChromeInContentPrefCached) {

dom/xhr/XMLHttpRequestMainThread.cpp

@@ -2499,16 +2499,11 @@ XMLHttpRequestMainThread::CreateChannel()
   nsCOMPtr<nsIPrincipal> resultingDocumentPrincipal(mPrincipal);
   nsCOMPtr<nsIExpandedPrincipal> ep = do_QueryInterface(mPrincipal);
   if (ep) {
-    nsTArray<nsCOMPtr<nsIPrincipal>>* whitelist = nullptr;
-    ep->GetWhiteList(&whitelist);
-    if (!whitelist) {
-      return NS_ERROR_FAILURE;
-    }
     MOZ_ASSERT(!(secFlags & nsILoadInfo::SEC_REQUIRE_SAME_ORIGIN_DATA_INHERITS));
     bool dataInherits = (secFlags &
       (nsILoadInfo::SEC_ALLOW_CROSS_ORIGIN_DATA_INHERITS |
        nsILoadInfo::SEC_REQUIRE_CORS_DATA_INHERITS)) != 0;
-    for (const auto& principal : *whitelist) {
+    for (const auto& principal : ep->WhiteList()) {
       if (NS_SUCCEEDED(principal->CheckMayLoad(mRequestURL, false, dataInherits))) {
         resultingDocumentPrincipal = principal;
         break;

extensions/cookie/nsPermissionManager.cpp

@@ -44,6 +44,7 @@
 #include "nsPrintfCString.h"
 #include "mozilla/AbstractThread.h"
 #include "ContentPrincipal.h"
+#include "ExpandedPrincipal.h"
 
 static nsPermissionManager *gPermissionManager = nullptr;
 
@@ -2233,16 +2234,13 @@ nsPermissionManager::CommonTestPermissionInternal(nsIPrincipal* aPrincipal,
 
   // For expanded principals, we want to iterate over the whitelist and see
   // if the permission is granted for any of them.
-  nsCOMPtr<nsIExpandedPrincipal> ep = do_QueryInterface(aPrincipal);
-  if (ep) {
-    nsTArray<nsCOMPtr<nsIPrincipal>>* whitelist;
-    nsresult rv = ep->GetWhiteList(&whitelist);
-    NS_ENSURE_SUCCESS(rv, rv);
-
-    for (size_t i = 0; i < whitelist->Length(); ++i) {
+  auto* basePrin = BasePrincipal::Cast(aPrincipal);
+  if (basePrin && basePrin->Is<ExpandedPrincipal>()) {
+    auto ep = basePrin->As<ExpandedPrincipal>();
+    for (auto& prin : ep->WhiteList()) {
       uint32_t perm;
-      rv = CommonTestPermission(whitelist->ElementAt(i), aType, &perm,
-                                aExactHostMatch, aIncludingSession);
+      nsresult rv = CommonTestPermission(prin, aType, &perm,
+                                         aExactHostMatch, aIncludingSession);
       NS_ENSURE_SUCCESS(rv, rv);
       if (perm == nsIPermissionManager::ALLOW_ACTION) {
         *aPermission = perm;

ipc/glue/BackgroundUtils.cpp

@@ -192,18 +192,15 @@ PrincipalToPrincipalInfo(nsIPrincipal* aPrincipal,
   }
 
   // might be an expanded principal
-  nsCOMPtr<nsIExpandedPrincipal> expanded =
-    do_QueryInterface(aPrincipal);
+  auto* basePrin = BasePrincipal::Cast(aPrincipal);
+  if (basePrin->Is<ExpandedPrincipal>()) {
+    auto* expanded = basePrin->As<ExpandedPrincipal>();
 
-  if (expanded) {
     nsTArray<PrincipalInfo> whitelistInfo;
     PrincipalInfo info;
 
-    nsTArray< nsCOMPtr<nsIPrincipal> >* whitelist;
-    MOZ_ALWAYS_SUCCEEDS(expanded->GetWhiteList(&whitelist));
-
-    for (uint32_t i = 0; i < whitelist->Length(); i++) {
-      rv = PrincipalToPrincipalInfo((*whitelist)[i], &info);
+    for (auto& prin : expanded->WhiteList()) {
+      rv = PrincipalToPrincipalInfo(prin, &info);
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
       }

js/xpconnect/src/XPCJSContext.cpp

@@ -583,9 +583,7 @@ IsWebExtensionContentScript(BasePrincipal* principal, nsAString& addonId)
 
     auto expanded = principal->As<ExpandedPrincipal>();
 
-    nsTArray<nsCOMPtr<nsIPrincipal>>* principals;
-    expanded->GetWhiteList(&principals);
-    for (auto prin : *principals) {
+    for (auto& prin : expanded->WhiteList()) {
         if (IsWebExtensionPrincipal(prin, addonId)) {
             return true;
         }