Bug 1208756 - Hoist shared CheckMayLoad logic into BasePrincipal. r=bz

This is a pure refactoring.
2015-09-30 20:03:36 -07:00 · 2015-09-30 20:03:36 -07:00 · dca7589731
--- a/caps/BasePrincipal.cpp
+++ b/caps/BasePrincipal.cpp
@ -269,6 +269,38 @@ BasePrincipal::SubsumesConsideringDomain(nsIPrincipal *aOther, bool *aResult)
  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)
 {
--- a/caps/BasePrincipal.h
+++ b/caps/BasePrincipal.h
@ -18,6 +18,8 @@ class nsILoadContext;
 class nsIObjectOutputStream;
 class nsIObjectInputStream;

+class nsExpandedPrincipal;
+
 namespace mozilla {

 class OriginAttributes : public dom::OriginAttributesDictionary
@ -141,6 +143,7 @@ public:
  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;
@ -174,6 +177,12 @@ protected:
  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);
--- a/caps/nsNullPrincipal.cpp
+++ b/caps/nsNullPrincipal.cpp
@ -107,15 +107,9 @@ 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) {
@ -123,16 +117,11 @@ nsNullPrincipal::CheckMayLoad(nsIURI* aURI, bool aReport, bool aAllowIfInheritsP
    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
--- a/caps/nsNullPrincipal.h
+++ b/caps/nsNullPrincipal.h
@ -44,7 +44,6 @@ public:
  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;
@ -68,6 +67,8 @@ public:
    return aOther == this;
  }

+  bool MayLoadInternal(nsIURI* aURI) override;
+
  nsCOMPtr<nsIURI> mURI;
  nsCOMPtr<nsIContentSecurityPolicy> mCSP;
 };
--- a/caps/nsPrincipal.cpp
+++ b/caps/nsPrincipal.cpp
@ -238,17 +238,9 @@ nsPrincipal::GetURI(nsIURI** aURI)
  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);
@ -257,17 +249,17 @@ nsPrincipal::CheckMayLoad(nsIURI* aURI, bool aReport, bool aAllowIfInheritsPrinc
    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
@ -276,13 +268,10 @@ nsPrincipal::CheckMayLoad(nsIURI* aURI, bool aReport, bool aAllowIfInheritsPrinc
  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
@ -758,17 +747,16 @@ nsExpandedPrincipal::SubsumesInternal(nsIPrincipal* aOther,
  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
--- a/caps/nsPrincipal.h
+++ b/caps/nsPrincipal.h
@ -26,7 +26,6 @@ public:
  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; }
@ -40,23 +39,6 @@ public:
  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.
   */
@ -79,6 +61,7 @@ protected:
  virtual ~nsPrincipal();

  bool SubsumesInternal(nsIPrincipal* aOther, DocumentDomainConsideration aConsideration) override;
+  bool MayLoadInternal(nsIURI* aURI) override;
 };

 class nsExpandedPrincipal : public nsIExpandedPrincipal, public mozilla::BasePrincipal
@ -95,7 +78,6 @@ public:
  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;
@ -105,6 +87,7 @@ protected:
  virtual ~nsExpandedPrincipal();

  bool SubsumesInternal(nsIPrincipal* aOther, DocumentDomainConsideration aConsideration) override;
+  bool MayLoadInternal(nsIURI* aURI) override;

 private:
  nsTArray< nsCOMPtr<nsIPrincipal> > mPrincipals;
--- a/caps/nsSystemPrincipal.cpp
+++ b/caps/nsSystemPrincipal.cpp
@ -41,12 +41,6 @@ nsSystemPrincipal::GetScriptLocation(nsACString &aStr)
 // Methods implementing nsIPrincipal //
 ///////////////////////////////////////

-NS_IMETHODIMP
-nsSystemPrincipal::CheckMayLoad(nsIURI* uri, bool aReport, bool aAllowIfInheritsPrincipal)
-{
-    return NS_OK;
-}
-
 NS_IMETHODIMP
 nsSystemPrincipal::GetHashValue(uint32_t *result)
 {
--- a/caps/nsSystemPrincipal.h
+++ b/caps/nsSystemPrincipal.h
@ -29,7 +29,6 @@ public:
  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;
@ -46,6 +45,11 @@ protected:
  {
    return true;
  }
+
+  bool MayLoadInternal(nsIURI* aURI) override
+  {
+    return true;
+  }
 };

 #endif // nsSystemPrincipal_h__