Bug 1347817 - Principal must always have a valid origin - part 4 - origin passed as argument when a principal is created, r=bholley

caps/BasePrincipal.cpp

@@ -14,6 +14,7 @@
 #include "nsIContentSecurityPolicy.h"
 #include "nsIObjectInputStream.h"
 #include "nsIObjectOutputStream.h"
+#include "nsIStandardURL.h"
 
 #include "ContentPrincipal.h"
 #include "nsNetUtil.h"
@@ -368,8 +369,31 @@ BasePrincipal::AddonHasPermission(const nsAString& aPerm)
 }
 
 already_AddRefed<BasePrincipal>
-BasePrincipal::CreateCodebasePrincipal(nsIURI* aURI, const OriginAttributes& aAttrs)
+BasePrincipal::CreateCodebasePrincipal(nsIURI* aURI,
+                                       const OriginAttributes& aAttrs)
 {
+  MOZ_ASSERT(aURI);
+
+  nsAutoCString originNoSuffix;
+  nsresult rv =
+    ContentPrincipal::GenerateOriginNoSuffixFromURI(aURI, originNoSuffix);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    // If the generation of the origin fails, we still want to have a valid
+    // principal. Better to return a null principal here.
+    return NullPrincipal::Create(aAttrs);
+  }
+
+  return CreateCodebasePrincipal(aURI, aAttrs, originNoSuffix);
+}
+
+already_AddRefed<BasePrincipal>
+BasePrincipal::CreateCodebasePrincipal(nsIURI* aURI,
+                                       const OriginAttributes& aAttrs,
+                                       const nsACString& aOriginNoSuffix)
+{
+  MOZ_ASSERT(aURI);
+  MOZ_ASSERT(!aOriginNoSuffix.IsEmpty());
+
   // If the URI is supposed to inherit the security context of whoever loads it,
   // we shouldn't make a codebase principal for it.
   bool inheritsPrincipal;
@@ -393,7 +417,7 @@ BasePrincipal::CreateCodebasePrincipal(nsIURI* aURI, const OriginAttributes& aAt
 
   // Mint a codebase principal.
   RefPtr<ContentPrincipal> codebase = new ContentPrincipal();
-  rv = codebase->Init(aURI, aAttrs);
+  rv = codebase->Init(aURI, aAttrs, aOriginNoSuffix);
   NS_ENSURE_SUCCESS(rv, nullptr);
   return codebase.forget();
 }

caps/BasePrincipal.h

@@ -75,9 +75,16 @@ public:
   virtual bool IsCodebasePrincipal() const { return false; };
 
   static BasePrincipal* Cast(nsIPrincipal* aPrin) { return static_cast<BasePrincipal*>(aPrin); }
+
+  static already_AddRefed<BasePrincipal>
+  CreateCodebasePrincipal(const nsACString& aOrigin);
+
+  // These following method may not create a codebase principal in case it's
+  // not possible to generate a correct origin from the passed URI. If this
+  // happens, a NullPrincipal is returned.
+
   static already_AddRefed<BasePrincipal>
   CreateCodebasePrincipal(nsIURI* aURI, const OriginAttributes& aAttrs);
-  static already_AddRefed<BasePrincipal> CreateCodebasePrincipal(const nsACString& aOrigin);
 
   const OriginAttributes& OriginAttributesRef() final { return mOriginAttributes; }
   uint32_t AppId() const { return mOriginAttributes.mAppId; }
@@ -130,6 +137,10 @@ protected:
   nsCOMPtr<nsIContentSecurityPolicy> mPreloadCSP;
 
 private:
+  static already_AddRefed<BasePrincipal>
+  CreateCodebasePrincipal(nsIURI* aURI, const OriginAttributes& aAttrs,
+                          const nsACString& aOriginNoSuffix);
+
   nsCOMPtr<nsIAtom> mOriginNoSuffix;
   nsCOMPtr<nsIAtom> mOriginSuffix;
 
@@ -157,20 +168,13 @@ BasePrincipal::FastEquals(nsIPrincipal* aOther)
     return this == other;
   }
 
-  if (mOriginNoSuffix) {
-    if (Kind() == eCodebasePrincipal) {
-      return mOriginNoSuffix == other->mOriginNoSuffix &&
-             mOriginSuffix == other->mOriginSuffix;
-    }
-
-    MOZ_ASSERT(Kind() == eExpandedPrincipal);
-    return mOriginNoSuffix == other->mOriginNoSuffix;
+  if (Kind() == eCodebasePrincipal) {
+    return mOriginNoSuffix == other->mOriginNoSuffix &&
+           mOriginSuffix == other->mOriginSuffix;
   }
 
-  // If mOriginNoSuffix is null on one of our principals, we must fall back
-  // to the slow path.
-  return Subsumes(aOther, DontConsiderDocumentDomain) &&
-         other->Subsumes(this, DontConsiderDocumentDomain);
+  MOZ_ASSERT(Kind() == eExpandedPrincipal);
+  return mOriginNoSuffix == other->mOriginNoSuffix;
 }
 
 inline bool
@@ -194,14 +198,11 @@ BasePrincipal::FastSubsumes(nsIPrincipal* aOther)
   // We deal with two special cases first:
   // Null principals only subsume each other if they are equal, and are only
   // equal if they're the same object.
-  // Also, if mOriginNoSuffix is null, FastEquals falls back to the slow path
-  // using Subsumes, so we don't want to use it in that case to avoid an
-  // infinite recursion.
   auto other = Cast(aOther);
   if (Kind() == eNullPrincipal && other->Kind() == eNullPrincipal) {
     return this == other;
   }
-  if (mOriginNoSuffix && FastEquals(aOther)) {
+  if (FastEquals(aOther)) {
     return true;
   }
 

caps/ContentPrincipal.cpp

@@ -95,7 +95,8 @@ ContentPrincipal::~ContentPrincipal()
 
 nsresult
 ContentPrincipal::Init(nsIURI *aCodebase,
-                       const OriginAttributes& aOriginAttributes)
+                       const OriginAttributes& aOriginAttributes,
+                       const nsACString& aOriginNoSuffix)
 {
   NS_ENSURE_ARG(aCodebase);
 
@@ -115,13 +116,7 @@ ContentPrincipal::Init(nsIURI *aCodebase,
   mCodebase = NS_TryToMakeImmutable(aCodebase);
   mCodebaseImmutable = URIIsImmutable(mCodebase);
 
-  nsAutoCString originNoSuffix;
-  nsresult rv = GenerateOriginNoSuffixFromURI(mCodebase, originNoSuffix);
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    return rv;
-  }
-
-  FinishInit(originNoSuffix, aOriginAttributes);
+  FinishInit(aOriginNoSuffix, aOriginAttributes);
 
   return NS_OK;
 }
@@ -485,7 +480,11 @@ ContentPrincipal::Read(nsIObjectInputStream* aStream)
   rv = NS_ReadOptionalObject(aStream, true, getter_AddRefs(supports));
   NS_ENSURE_SUCCESS(rv, rv);
 
-  rv = Init(codebase, attrs);
+  nsAutoCString originNoSuffix;
+  rv = GenerateOriginNoSuffixFromURI(codebase, originNoSuffix);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  rv = Init(codebase, attrs, originNoSuffix);
   NS_ENSURE_SUCCESS(rv, rv);
 
   mCSP = do_QueryInterface(supports, &rv);

caps/ContentPrincipal.h

@@ -32,7 +32,8 @@ public:
 
   // Init() must be called before the principal is in a usable state.
   nsresult Init(nsIURI* aCodebase,
-                const mozilla::OriginAttributes& aOriginAttributes);
+                const mozilla::OriginAttributes& aOriginAttributes,
+                const nsACString& aOriginNoSuffix);
 
   virtual nsresult GetScriptLocation(nsACString& aStr) override;
 
@@ -41,6 +42,9 @@ public:
    */
   static void InitializeStatics();
 
+  static nsresult
+  GenerateOriginNoSuffixFromURI(nsIURI* aURI, nsACString& aOrigin);
+
   nsCOMPtr<nsIURI> mDomain;
   nsCOMPtr<nsIURI> mCodebase;
   // If mCodebaseImmutable is true, mCodebase is non-null and immutable
@@ -55,9 +59,6 @@ protected:
   bool MayLoadInternal(nsIURI* aURI) override;
 
 private:
-  static nsresult 
-  GenerateOriginNoSuffixFromURI(nsIURI* aURI, nsACString& aOriginNoSuffix);
-
   mozilla::Maybe<nsString> mAddonIdCache;
 };
 

caps/ExpandedPrincipal.cpp

@@ -23,7 +23,7 @@ struct OriginComparator
   bool LessThan(nsIPrincipal* a, nsIPrincipal* b) const
   {
     nsAutoCString originA;
-    nsresult rv = a->GetOrigin(originA);
+    DebugOnly<nsresult> rv = a->GetOrigin(originA);
     MOZ_ASSERT(NS_SUCCEEDED(rv));
     nsAutoCString originB;
     rv = b->GetOrigin(originB);
@@ -34,7 +34,7 @@ struct OriginComparator
   bool Equals(nsIPrincipal* a, nsIPrincipal* b) const
   {
     nsAutoCString originA;
-    nsresult rv = a->GetOrigin(originA);
+    DebugOnly<nsresult> rv = a->GetOrigin(originA);
     MOZ_ASSERT(NS_SUCCEEDED(rv));
     nsAutoCString originB;
     rv = b->GetOrigin(originB);
@@ -71,7 +71,7 @@ ExpandedPrincipal::Create(nsTArray<nsCOMPtr<nsIPrincipal>>& aWhiteList,
     }
 
     nsAutoCString subOrigin;
-    nsresult rv = ep->mPrincipals.ElementAt(i)->GetOrigin(subOrigin);
+    DebugOnly<nsresult> rv = ep->mPrincipals.ElementAt(i)->GetOrigin(subOrigin);
     MOZ_ASSERT(NS_SUCCEEDED(rv));
     origin.Append(subOrigin);
   }

caps/NullPrincipal.cpp

@@ -85,7 +85,7 @@ NullPrincipal::Init(const OriginAttributes& aOriginAttributes, nsIURI* aURI)
   }
 
   nsAutoCString originNoSuffix;
-  nsresult rv = mURI->GetSpec(originNoSuffix);
+  DebugOnly<nsresult> rv = mURI->GetSpec(originNoSuffix);
   MOZ_ASSERT(NS_SUCCEEDED(rv));
 
   FinishInit(originNoSuffix, aOriginAttributes);

ipc/glue/BackgroundUtils.cpp

@@ -89,8 +89,7 @@ PrincipalInfoToPrincipal(const PrincipalInfo& aPrincipalInfo,
         attrs = info.attrs();
       }
       principal = BasePrincipal::CreateCodebasePrincipal(uri, attrs);
-      rv = principal ? NS_OK : NS_ERROR_FAILURE;
-      if (NS_WARN_IF(NS_FAILED(rv))) {
+      if (NS_WARN_IF(!principal)) {
         return nullptr;
       }