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
--- a/caps/ExpandedPrincipal.cpp
+++ b/caps/ExpandedPrincipal.cpp
@@ -95,25 +95,24 @@ ExpandedPrincipal::SetDomain(nsIURI* aDo
}
bool
ExpandedPrincipal::SubsumesInternal(nsIPrincipal* aOther,
BasePrincipal::DocumentDomainConsideration aConsideration)
{
// 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;
}
}
return true;
}
// We're dealing with a regular principal. One of our principals must subsume
// it.
@@ -146,21 +145,20 @@ ExpandedPrincipal::GetHashValue(uint32_t
NS_IMETHODIMP
ExpandedPrincipal::GetURI(nsIURI** aURI)
{
*aURI = nullptr;
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
ExpandedPrincipal::GetBaseDomain(nsACString& aBaseDomain)
{
return NS_ERROR_NOT_AVAILABLE;
}
--- a/caps/nsIPrincipal.idl
+++ b/caps/nsIPrincipal.idl
@@ -37,17 +37,17 @@ class OriginAttributes;
%}
interface nsIURI;
interface nsIContentSecurityPolicy;
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)]
interface nsIPrincipal : nsISerializable
{
/**
* Returns whether the other principal is equivalent to this principal.
* Principals are considered equal if they are the same principal, or
@@ -335,10 +335,11 @@ interface nsIPrincipal : nsISerializable
[uuid(f3e177Df-6a5e-489f-80a7-2dd1481471d8)]
interface nsIExpandedPrincipal : nsISupports
{
/**
* An array of principals that the expanded principal subsumes.
* 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();
};
--- a/caps/nsScriptSecurityManager.cpp
+++ b/caps/nsScriptSecurityManager.cpp
@@ -15,16 +15,17 @@
#include "nsIServiceManager.h"
#include "nsIScriptObjectPrincipal.h"
#include "nsIScriptContext.h"
#include "nsIURL.h"
#include "nsINestedURI.h"
#include "nspr.h"
#include "nsJSPrincipals.h"
#include "mozilla/BasePrincipal.h"
+#include "ExpandedPrincipal.h"
#include "SystemPrincipal.h"
#include "NullPrincipal.h"
#include "DomainPolicy.h"
#include "nsString.h"
#include "nsCRT.h"
#include "nsCRTGlue.h"
#include "nsDocShell.h"
#include "nsError.h"
@@ -663,22 +664,21 @@ nsScriptSecurityManager::CheckLoadURIWit
if (aPrincipal == mSystemPrincipal) {
// Allow access
return NS_OK;
}
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)) {
// Allow access if it succeeded with one of the white listed principals
return NS_OK;
}
}
// None of our whitelisted principals worked.
--- a/dom/base/nsDocument.cpp
+++ b/dom/base/nsDocument.cpp
@@ -2489,21 +2489,19 @@ nsDocument::MaybeDowngradePrincipal(nsIP
// We can't load a document with an expanded principal. If we're given one,
// automatically downgrade it to the last principal it subsumes (which is the
// extension principal, in the case of extension content scripts).
auto* basePrin = BasePrincipal::Cast(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);
-
- return do_AddRef(whitelist->LastElement().get());
+ MOZ_ASSERT(expanded->WhiteList().Length() > 0);
+
+ return do_AddRef(expanded->WhiteList().LastElement().get());
}
if (!sChromeInContentPrefCached) {
sChromeInContentPrefCached = true;
Preferences::AddBoolVarCache(&sChromeInContentAllowed,
kChromeInContentPref, false);
}
if (!sChromeInContentAllowed &&
--- a/dom/xhr/XMLHttpRequestMainThread.cpp
+++ b/dom/xhr/XMLHttpRequestMainThread.cpp
@@ -2514,26 +2514,21 @@ XMLHttpRequestMainThread::CreateChannel(
// Using the provided principal as the triggeringPrincipal is fine, since we
// want to be able to access any of the origins that the principal has access
// to during the security checks, but we don't want a document to inherit an
// expanded principal, so in that case we need to select the principal in the
// expanded principal's whitelist that can load our URL as principalToInherit.
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;
}
}
}
nsCOMPtr<nsILoadInfo> loadInfo = mChannel->GetLoadInfo();
--- a/extensions/cookie/nsPermissionManager.cpp
+++ b/extensions/cookie/nsPermissionManager.cpp
@@ -39,16 +39,17 @@
#include "mozilla/Telemetry.h"
#include "nsIConsoleService.h"
#include "nsINavHistoryService.h"
#include "nsToolkitCompsCID.h"
#include "nsIObserverService.h"
#include "nsPrintfCString.h"
#include "mozilla/AbstractThread.h"
#include "ContentPrincipal.h"
+#include "ExpandedPrincipal.h"
static nsPermissionManager *gPermissionManager = nullptr;
using namespace mozilla;
using namespace mozilla::dom;
static bool
IsChildProcess()
@@ -2228,26 +2229,23 @@ nsPermissionManager::CommonTestPermissio
return NS_OK;
}
// Set the default.
*aPermission = nsIPermissionManager::UNKNOWN_ACTION;
// 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->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;
return NS_OK;
} else if (perm == nsIPermissionManager::PROMPT_ACTION) {
// Store it, but keep going to see if we can do better.
*aPermission = perm;
}
--- a/ipc/glue/BackgroundUtils.cpp
+++ b/ipc/glue/BackgroundUtils.cpp
@@ -187,28 +187,25 @@ PrincipalToPrincipalInfo(nsIPrincipal* a
}
if (isSystemPrincipal) {
*aPrincipalInfo = SystemPrincipalInfo();
return NS_OK;
}
// 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;
}
// append that spec to the whitelist
whitelistInfo.AppendElement(info);
}
*aPrincipalInfo =
--- a/js/xpconnect/src/XPCJSContext.cpp
+++ b/js/xpconnect/src/XPCJSContext.cpp
@@ -578,19 +578,17 @@ static bool
IsWebExtensionContentScript(BasePrincipal* principal, nsAString& addonId)
{
if (!principal->Is<ExpandedPrincipal>()) {
return false;
}
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;
}
}
return false;
}