Bug 1208756 - Hoist shared CheckMayLoad logic into BasePrincipal. r=bz
authorBobby Holley <bobbyholley@gmail.com>
Wed, 30 Sep 2015 20:03:36 -0700
changeset 265847 ecb7068b07a1b0843c9bda2926d77ebfb3c46a99
parent 265846 5845fb9f14cf669216d997341809b10526ff0cd3
child 265848 06a3886ffdc948fb69f49744f5d5e74f0968a98d
push id29470
push userphilringnalda@gmail.com
push dateSat, 03 Oct 2015 22:38:52 +0000
treeherdermozilla-central@2e62c35afb2c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1208756
milestone44.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 1208756 - Hoist shared CheckMayLoad logic into BasePrincipal. r=bz This is a pure refactoring.
caps/BasePrincipal.cpp
caps/BasePrincipal.h
caps/nsNullPrincipal.cpp
caps/nsNullPrincipal.h
caps/nsPrincipal.cpp
caps/nsPrincipal.h
caps/nsSystemPrincipal.cpp
caps/nsSystemPrincipal.h
--- a/caps/BasePrincipal.cpp
+++ b/caps/BasePrincipal.cpp
@@ -265,16 +265,48 @@ NS_IMETHODIMP
 BasePrincipal::SubsumesConsideringDomain(nsIPrincipal *aOther, bool *aResult)
 {
   NS_ENSURE_TRUE(aOther, NS_ERROR_INVALID_ARG);
   *aResult = Subsumes(aOther, ConsiderDocumentDomain);
   return NS_OK;
 }
 
 NS_IMETHODIMP
+BasePrincipal::CheckMayLoad(nsIURI* aURI, bool aReport, bool aAllowIfInheritsPrincipal)
+{
+  // Check the internal method first, which allows us to quickly approve loads
+  // for the System Principal.
+  if (MayLoadInternal(aURI)) {
+    return NS_OK;
+  }
+
+  nsresult rv;
+  if (aAllowIfInheritsPrincipal) {
+    // If the caller specified to allow loads of URIs that inherit
+    // our principal, allow the load if this URI inherits its principal.
+    bool doesInheritSecurityContext;
+    rv = NS_URIChainHasFlags(aURI, nsIProtocolHandler::URI_INHERITS_SECURITY_CONTEXT,
+                             &doesInheritSecurityContext);
+    if (NS_SUCCEEDED(rv) && doesInheritSecurityContext) {
+      return NS_OK;
+    }
+  }
+
+  if (aReport) {
+    nsCOMPtr<nsIURI> prinURI;
+    rv = GetURI(getter_AddRefs(prinURI));
+    if (NS_SUCCEEDED(rv) && prinURI) {
+      nsScriptSecurityManager::ReportError(nullptr, NS_LITERAL_STRING("CheckSameOriginError"), prinURI, aURI);
+    }
+  }
+
+  return NS_ERROR_DOM_BAD_URI;
+}
+
+NS_IMETHODIMP
 BasePrincipal::GetCsp(nsIContentSecurityPolicy** aCsp)
 {
   NS_IF_ADDREF(*aCsp = mCSP);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 BasePrincipal::SetCsp(nsIContentSecurityPolicy* aCsp)
--- a/caps/BasePrincipal.h
+++ b/caps/BasePrincipal.h
@@ -13,16 +13,18 @@
 
 #include "mozilla/dom/ChromeUtilsBinding.h"
 
 class nsIContentSecurityPolicy;
 class nsILoadContext;
 class nsIObjectOutputStream;
 class nsIObjectInputStream;
 
+class nsExpandedPrincipal;
+
 namespace mozilla {
 
 class OriginAttributes : public dom::OriginAttributesDictionary
 {
 public:
   OriginAttributes() {}
   OriginAttributes(uint32_t aAppId, bool aInBrowser)
   {
@@ -136,16 +138,17 @@ public:
   bool Subsumes(nsIPrincipal* aOther, DocumentDomainConsideration aConsideration);
 
   NS_IMETHOD GetOrigin(nsACString& aOrigin) final;
   NS_IMETHOD GetOriginNoSuffix(nsACString& aOrigin) final;
   NS_IMETHOD Equals(nsIPrincipal* other, bool* _retval) final;
   NS_IMETHOD EqualsConsideringDomain(nsIPrincipal* other, bool* _retval) final;
   NS_IMETHOD Subsumes(nsIPrincipal* other, bool* _retval) final;
   NS_IMETHOD SubsumesConsideringDomain(nsIPrincipal* other, bool* _retval) final;
+  NS_IMETHOD CheckMayLoad(nsIURI* uri, bool report, bool allowIfInheritsPrincipal) final;
   NS_IMETHOD GetCsp(nsIContentSecurityPolicy** aCsp) override;
   NS_IMETHOD SetCsp(nsIContentSecurityPolicy* aCsp) override;
   NS_IMETHOD GetCspJSON(nsAString& outCSPinJSON) override;
   NS_IMETHOD GetIsNullPrincipal(bool* aIsNullPrincipal) override;
   NS_IMETHOD GetJarPrefix(nsACString& aJarPrefix) final;
   NS_IMETHOD GetOriginAttributes(JSContext* aCx, JS::MutableHandle<JS::Value> aVal) final;
   NS_IMETHOD GetOriginSuffix(nsACString& aOriginSuffix) final;
   NS_IMETHOD GetAppStatus(uint16_t* aAppStatus) final;
@@ -169,16 +172,22 @@ public:
   bool IsInBrowserElement() const { return mOriginAttributes.mInBrowser; }
 
 protected:
   virtual ~BasePrincipal();
 
   virtual nsresult GetOriginInternal(nsACString& aOrigin) = 0;
   virtual bool SubsumesInternal(nsIPrincipal* aOther, DocumentDomainConsideration aConsider) = 0;
 
+  // Internal, side-effect-free check to determine whether the concrete
+  // principal would allow the load ignoring any common behavior implemented in
+  // BasePrincipal::CheckMayLoad.
+  virtual bool MayLoadInternal(nsIURI* aURI) = 0;
+  friend class ::nsExpandedPrincipal;
+
   // Helper to check whether this principal is associated with an addon that
   // allows unprivileged code to load aURI.
   bool AddonAllowsLoad(nsIURI* aURI);
 
   nsCOMPtr<nsIContentSecurityPolicy> mCSP;
   OriginAttributes mOriginAttributes;
 };
 
--- a/caps/nsNullPrincipal.cpp
+++ b/caps/nsNullPrincipal.cpp
@@ -102,42 +102,31 @@ nsNullPrincipal::SetDomain(nsIURI* aDoma
 }
 
 nsresult
 nsNullPrincipal::GetOriginInternal(nsACString& aOrigin)
 {
   return mURI->GetSpec(aOrigin);
 }
 
-NS_IMETHODIMP
-nsNullPrincipal::CheckMayLoad(nsIURI* aURI, bool aReport, bool aAllowIfInheritsPrincipal)
- {
-  if (aAllowIfInheritsPrincipal) {
-    if (nsPrincipal::IsPrincipalInherited(aURI)) {
-      return NS_OK;
-    }
-  }
-
+bool
+nsNullPrincipal::MayLoadInternal(nsIURI* aURI)
+{
   // Also allow the load if we are the principal of the URI being checked.
   nsCOMPtr<nsIURIWithPrincipal> uriPrinc = do_QueryInterface(aURI);
   if (uriPrinc) {
     nsCOMPtr<nsIPrincipal> principal;
     uriPrinc->GetPrincipal(getter_AddRefs(principal));
 
     if (principal == this) {
-      return NS_OK;
+      return true;
     }
   }
 
-  if (aReport) {
-    nsScriptSecurityManager::ReportError(
-      nullptr, NS_LITERAL_STRING("CheckSameOriginError"), mURI, aURI);
-  }
-
-  return NS_ERROR_DOM_BAD_URI;
+  return false;
 }
 
 NS_IMETHODIMP
 nsNullPrincipal::GetIsNullPrincipal(bool* aIsNullPrincipal)
 {
   *aIsNullPrincipal = true;
   return NS_OK;
 }
--- a/caps/nsNullPrincipal.h
+++ b/caps/nsNullPrincipal.h
@@ -39,17 +39,16 @@ public:
 
   NS_DECL_NSISERIALIZABLE
 
   NS_IMETHOD QueryInterface(REFNSIID aIID, void** aInstancePtr) override;
   NS_IMETHOD GetHashValue(uint32_t* aHashValue) override;
   NS_IMETHOD GetURI(nsIURI** aURI) override;
   NS_IMETHOD GetDomain(nsIURI** aDomain) override;
   NS_IMETHOD SetDomain(nsIURI* aDomain) override;
-  NS_IMETHOD CheckMayLoad(nsIURI* uri, bool report, bool allowIfInheritsPrincipal) override;
   NS_IMETHOD GetIsNullPrincipal(bool* aIsNullPrincipal) override;
   NS_IMETHOD GetBaseDomain(nsACString& aBaseDomain) override;
   nsresult GetOriginInternal(nsACString& aOrigin) override;
 
   // Returns null on failure.
   static already_AddRefed<nsNullPrincipal> CreateWithInheritedAttributes(nsIPrincipal *aInheritFrom);
 
   // Returns null on failure.
@@ -63,13 +62,15 @@ public:
  protected:
   virtual ~nsNullPrincipal() {}
 
   bool SubsumesInternal(nsIPrincipal* aOther, DocumentDomainConsideration aConsideration) override
   {
     return aOther == this;
   }
 
+  bool MayLoadInternal(nsIURI* aURI) override;
+
   nsCOMPtr<nsIURI> mURI;
   nsCOMPtr<nsIContentSecurityPolicy> mCSP;
 };
 
 #endif // nsNullPrincipal_h__
--- a/caps/nsPrincipal.cpp
+++ b/caps/nsPrincipal.cpp
@@ -233,61 +233,50 @@ nsPrincipal::GetURI(nsIURI** aURI)
   if (!mCodebase) {
     *aURI = nullptr;
     return NS_OK;
   }
 
   return NS_EnsureSafeToReturn(mCodebase, aURI);
 }
 
-NS_IMETHODIMP
-nsPrincipal::CheckMayLoad(nsIURI* aURI, bool aReport, bool aAllowIfInheritsPrincipal)
+bool
+nsPrincipal::MayLoadInternal(nsIURI* aURI)
 {
-   if (aAllowIfInheritsPrincipal) {
-    // If the caller specified to allow loads of URIs that inherit
-    // our principal, allow the load if this URI inherits its principal
-    if (nsPrincipal::IsPrincipalInherited(aURI)) {
-      return NS_OK;
-    }
-  }
-
   // See if aURI is something like a Blob URI that is actually associated with
   // a principal.
   nsCOMPtr<nsIURIWithPrincipal> uriWithPrin = do_QueryInterface(aURI);
   nsCOMPtr<nsIPrincipal> uriPrin;
   if (uriWithPrin) {
     uriWithPrin->GetPrincipal(getter_AddRefs(uriPrin));
   }
   if (uriPrin && nsIPrincipal::Subsumes(uriPrin)) {
-      return NS_OK;
+    return true;
   }
 
   // If this principal is associated with an addon, check whether that addon
   // has been given permission to load from this domain.
   if (AddonAllowsLoad(aURI)) {
-    return NS_OK;
+    return true;
   }
 
   if (nsScriptSecurityManager::SecurityCompareURIs(mCodebase, aURI)) {
-    return NS_OK;
+    return true;
   }
 
   // If strict file origin policy is in effect, local files will always fail
   // SecurityCompareURIs unless they are identical. Explicitly check file origin
   // policy, in that case.
   if (nsScriptSecurityManager::GetStrictFileOriginPolicy() &&
       NS_URIIsLocalFile(aURI) &&
       NS_RelaxStrictFileOriginPolicy(aURI, mCodebase)) {
-    return NS_OK;
+    return true;
   }
 
-  if (aReport) {
-    nsScriptSecurityManager::ReportError(nullptr, NS_LITERAL_STRING("CheckSameOriginError"), mCodebase, aURI);
-  }
-  return NS_ERROR_DOM_BAD_URI;
+  return false;
 }
 
 void
 nsPrincipal::SetURI(nsIURI* aURI)
 {
   mCodebase = NS_TryToMakeImmutable(aURI);
   mCodebaseImmutable = URIIsImmutable(mCodebase);
 }
@@ -753,27 +742,26 @@ nsExpandedPrincipal::SubsumesInternal(ns
     if (Cast(mPrincipals[i])->Subsumes(aOther, aConsideration)) {
       return true;
     }
   }
 
   return false;
 }
 
-NS_IMETHODIMP
-nsExpandedPrincipal::CheckMayLoad(nsIURI* uri, bool aReport, bool aAllowIfInheritsPrincipal)
+bool
+nsExpandedPrincipal::MayLoadInternal(nsIURI* uri)
 {
-  nsresult rv;
   for (uint32_t i = 0; i < mPrincipals.Length(); ++i){
-    rv = mPrincipals[i]->CheckMayLoad(uri, aReport, aAllowIfInheritsPrincipal);
-    if (NS_SUCCEEDED(rv))
-      return rv;
+    if (BasePrincipal::Cast(mPrincipals[i])->MayLoadInternal(uri)) {
+      return true;
+    }
   }
 
-  return NS_ERROR_DOM_BAD_URI;
+  return false;
 }
 
 NS_IMETHODIMP
 nsExpandedPrincipal::GetHashValue(uint32_t* result)
 {
   MOZ_CRASH("extended principal should never be used as key in a hash map");
 }
 
--- a/caps/nsPrincipal.h
+++ b/caps/nsPrincipal.h
@@ -21,47 +21,29 @@ class nsPrincipal final : public mozilla
 {
 public:
   NS_DECL_NSISERIALIZABLE
   NS_IMETHOD QueryInterface(REFNSIID aIID, void** aInstancePtr) override;
   NS_IMETHOD GetHashValue(uint32_t* aHashValue) override;
   NS_IMETHOD GetURI(nsIURI** aURI) override;
   NS_IMETHOD GetDomain(nsIURI** aDomain) override;
   NS_IMETHOD SetDomain(nsIURI* aDomain) override;
-  NS_IMETHOD CheckMayLoad(nsIURI* uri, bool report, bool allowIfInheritsPrincipal) override;
   NS_IMETHOD GetBaseDomain(nsACString& aBaseDomain) override;
   virtual bool IsOnCSSUnprefixingWhitelist() override;
   bool IsCodebasePrincipal() const override { return true; }
   nsresult GetOriginInternal(nsACString& aOrigin) override;
 
   nsPrincipal();
 
   // Init() must be called before the principal is in a usable state.
   nsresult Init(nsIURI* aCodebase, const mozilla::OriginAttributes& aOriginAttributes);
 
   virtual void GetScriptLocation(nsACString& aStr) override;
   void SetURI(nsIURI* aURI);
 
-  static bool IsPrincipalInherited(nsIURI* aURI) {
-    // return true if the loadee URI has
-    // the URI_INHERITS_SECURITY_CONTEXT flag set.
-    bool doesInheritSecurityContext;
-    nsresult rv =
-    NS_URIChainHasFlags(aURI,
-                        nsIProtocolHandler::URI_INHERITS_SECURITY_CONTEXT,
-                        &doesInheritSecurityContext);
-
-    if (NS_SUCCEEDED(rv) && doesInheritSecurityContext) {
-      return true;
-    }
-
-    return false;
-  }
-
-
   /**
    * Computes the puny-encoded origin of aURI.
    */
   static nsresult GetOriginForURI(nsIURI* aURI, nsACString& aOrigin);
 
   /**
    * Called at startup to setup static data, e.g. about:config pref-observers.
    */
@@ -74,42 +56,43 @@ public:
   bool mDomainImmutable;
   bool mInitialized;
   mozilla::Maybe<bool> mIsOnCSSUnprefixingWhitelist; // Lazily-computed
 
 protected:
   virtual ~nsPrincipal();
 
   bool SubsumesInternal(nsIPrincipal* aOther, DocumentDomainConsideration aConsideration) override;
+  bool MayLoadInternal(nsIURI* aURI) override;
 };
 
 class nsExpandedPrincipal : public nsIExpandedPrincipal, public mozilla::BasePrincipal
 {
 public:
   explicit nsExpandedPrincipal(nsTArray< nsCOMPtr<nsIPrincipal> > &aWhiteList);
 
   NS_DECL_NSIEXPANDEDPRINCIPAL
   NS_DECL_NSISERIALIZABLE
   NS_IMETHODIMP_(MozExternalRefCountType) AddRef() override { return nsJSPrincipals::AddRef(); };
   NS_IMETHODIMP_(MozExternalRefCountType) Release() override { return nsJSPrincipals::Release(); };
   NS_IMETHOD QueryInterface(REFNSIID aIID, void** aInstancePtr) override;
   NS_IMETHOD GetHashValue(uint32_t* aHashValue) override;
   NS_IMETHOD GetURI(nsIURI** aURI) override;
   NS_IMETHOD GetDomain(nsIURI** aDomain) override;
   NS_IMETHOD SetDomain(nsIURI* aDomain) override;
-  NS_IMETHOD CheckMayLoad(nsIURI* uri, bool report, bool allowIfInheritsPrincipal) override;
   NS_IMETHOD GetBaseDomain(nsACString& aBaseDomain) override;
   virtual bool IsOnCSSUnprefixingWhitelist() override;
   virtual void GetScriptLocation(nsACString &aStr) override;
   nsresult GetOriginInternal(nsACString& aOrigin) override;
 
 protected:
   virtual ~nsExpandedPrincipal();
 
   bool SubsumesInternal(nsIPrincipal* aOther, DocumentDomainConsideration aConsideration) override;
+  bool MayLoadInternal(nsIURI* aURI) override;
 
 private:
   nsTArray< nsCOMPtr<nsIPrincipal> > mPrincipals;
 };
 
 #define NS_PRINCIPAL_CONTRACTID "@mozilla.org/principal;1"
 #define NS_PRINCIPAL_CID \
 { 0x653e0e4d, 0x3ee4, 0x45fa, \
--- a/caps/nsSystemPrincipal.cpp
+++ b/caps/nsSystemPrincipal.cpp
@@ -37,22 +37,16 @@ nsSystemPrincipal::GetScriptLocation(nsA
     aStr.AssignLiteral(SYSTEM_PRINCIPAL_SPEC);
 }
 
 ///////////////////////////////////////
 // Methods implementing nsIPrincipal //
 ///////////////////////////////////////
 
 NS_IMETHODIMP
-nsSystemPrincipal::CheckMayLoad(nsIURI* uri, bool aReport, bool aAllowIfInheritsPrincipal)
-{
-    return NS_OK;
-}
-
-NS_IMETHODIMP
 nsSystemPrincipal::GetHashValue(uint32_t *result)
 {
     *result = NS_PTR_TO_INT32(this);
     return NS_OK;
 }
 
 NS_IMETHODIMP 
 nsSystemPrincipal::GetURI(nsIURI** aURI)
--- a/caps/nsSystemPrincipal.h
+++ b/caps/nsSystemPrincipal.h
@@ -24,28 +24,32 @@ class nsSystemPrincipal final : public m
 {
 public:
   NS_DECL_NSISERIALIZABLE
   NS_IMETHOD QueryInterface(REFNSIID aIID, void** aInstancePtr) override;
   NS_IMETHOD GetHashValue(uint32_t* aHashValue) override;
   NS_IMETHOD GetURI(nsIURI** aURI) override;
   NS_IMETHOD GetDomain(nsIURI** aDomain) override;
   NS_IMETHOD SetDomain(nsIURI* aDomain) override;
-  NS_IMETHOD CheckMayLoad(nsIURI* uri, bool report, bool allowIfInheritsPrincipal) override;
   NS_IMETHOD GetCsp(nsIContentSecurityPolicy** aCsp) override;
   NS_IMETHOD SetCsp(nsIContentSecurityPolicy* aCsp) override;
   NS_IMETHOD GetBaseDomain(nsACString& aBaseDomain) override;
   nsresult GetOriginInternal(nsACString& aOrigin) override;
 
   nsSystemPrincipal() {}
 
   virtual void GetScriptLocation(nsACString &aStr) override;
 
 protected:
   virtual ~nsSystemPrincipal(void) {}
 
   bool SubsumesInternal(nsIPrincipal *aOther, DocumentDomainConsideration aConsideration) override
   {
     return true;
   }
+
+  bool MayLoadInternal(nsIURI* aURI) override
+  {
+    return true;
+  }
 };
 
 #endif // nsSystemPrincipal_h__