Backed out changeset 059bcee1ccda (bug 1347817)
authorSebastian Hengst <archaeopteryx@coole-files.de>
Wed, 29 Mar 2017 11:17:13 +0200
changeset 350219 8bda12cfb5acbdb70fbad9856f43569849ecaebe
parent 350218 8b74d843bf8e19124437c4846c341c63466c0b7f
child 350220 be4d4a7ef77f2e8ebab9fa5298899ec626cd8ada
push id88591
push userarchaeopteryx@coole-files.de
push dateWed, 29 Mar 2017 09:19:03 +0000
treeherdermozilla-inbound@933a668a6468 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1347817
milestone55.0a1
backs out059bcee1ccda55f0d270e99900f0a933caedde08
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
Backed out changeset 059bcee1ccda (bug 1347817)
caps/BasePrincipal.cpp
caps/BasePrincipal.h
caps/ContentPrincipal.cpp
caps/ContentPrincipal.h
caps/ExpandedPrincipal.cpp
caps/NullPrincipal.cpp
ipc/glue/BackgroundUtils.cpp
--- a/caps/BasePrincipal.cpp
+++ b/caps/BasePrincipal.cpp
@@ -9,17 +9,16 @@
 #include "nsDocShell.h"
 #ifdef MOZ_CRASHREPORTER
 #include "nsExceptionHandler.h"
 #endif
 #include "nsIAddonPolicyService.h"
 #include "nsIContentSecurityPolicy.h"
 #include "nsIObjectInputStream.h"
 #include "nsIObjectOutputStream.h"
-#include "nsIStandardURL.h"
 
 #include "ContentPrincipal.h"
 #include "nsNetUtil.h"
 #include "nsIURIWithPrincipal.h"
 #include "NullPrincipal.h"
 #include "nsScriptSecurityManager.h"
 #include "nsServiceManagerUtils.h"
 
@@ -364,41 +363,18 @@ BasePrincipal::AddonHasPermission(const 
 
   bool retval = false;
   nsresult rv = aps->AddonHasPermission(addonId, aPerm, &retval);
   NS_ENSURE_SUCCESS(rv, false);
   return retval;
 }
 
 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;
   nsresult rv = NS_URIChainHasFlags(aURI, nsIProtocolHandler::URI_INHERITS_SECURITY_CONTEXT,
                                     &inheritsPrincipal);
   if (NS_FAILED(rv) || inheritsPrincipal) {
     return NullPrincipal::Create(aAttrs);
   }
@@ -412,17 +388,17 @@ BasePrincipal::CreateCodebasePrincipal(n
       return NullPrincipal::Create(aAttrs);
     }
     RefPtr<BasePrincipal> concrete = Cast(principal);
     return concrete.forget();
   }
 
   // Mint a codebase principal.
   RefPtr<ContentPrincipal> codebase = new ContentPrincipal();
-  rv = codebase->Init(aURI, aAttrs, aOriginNoSuffix);
+  rv = codebase->Init(aURI, aAttrs);
   NS_ENSURE_SUCCESS(rv, nullptr);
   return codebase.forget();
 }
 
 already_AddRefed<BasePrincipal>
 BasePrincipal::CreateCodebasePrincipal(const nsACString& aOrigin)
 {
   MOZ_ASSERT(!StringBeginsWith(aOrigin, NS_LITERAL_CSTRING("[")),
--- a/caps/BasePrincipal.h
+++ b/caps/BasePrincipal.h
@@ -70,26 +70,19 @@ public:
   NS_IMETHOD GetUserContextId(uint32_t* aUserContextId) final;
   NS_IMETHOD GetPrivateBrowsingId(uint32_t* aPrivateBrowsingId) final;
 
   virtual bool AddonHasPermission(const nsAString& aPerm);
 
   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; }
   uint32_t UserContextId() const { return mOriginAttributes.mUserContextId; }
   uint32_t PrivateBrowsingId() const { return mOriginAttributes.mPrivateBrowsingId; }
   bool IsInIsolatedMozBrowserElement() const { return mOriginAttributes.mInIsolatedMozBrowser; }
 
   PrincipalKind Kind() const { return mKind; }
@@ -132,20 +125,16 @@ protected:
   // method of the child classes.
   void FinishInit(const nsACString& aOriginNoSuffix,
                   const OriginAttributes& aOriginAttributes);
 
   nsCOMPtr<nsIContentSecurityPolicy> mCSP;
   nsCOMPtr<nsIContentSecurityPolicy> mPreloadCSP;
 
 private:
-  static already_AddRefed<BasePrincipal>
-  CreateCodebasePrincipal(nsIURI* aURI, const OriginAttributes& aAttrs,
-                          const nsACString& aOriginNoSuffix);
-
   nsCOMPtr<nsIAtom> mOriginNoSuffix;
   nsCOMPtr<nsIAtom> mOriginSuffix;
 
   OriginAttributes mOriginAttributes;
   PrincipalKind mKind;
   bool mHasExplicitDomain;
   bool mInitialized;
 };
@@ -163,23 +152,30 @@ BasePrincipal::FastEquals(nsIPrincipal* 
   // If the two principals are codebase principals, their origin attributes
   // (aka the origin suffix) must also match.
   // If the two principals are null principals, they're only equal if they're
   // the same object.
   if (Kind() == eNullPrincipal || Kind() == eSystemPrincipal) {
     return this == other;
   }
 
-  if (Kind() == eCodebasePrincipal) {
-    return mOriginNoSuffix == other->mOriginNoSuffix &&
-           mOriginSuffix == other->mOriginSuffix;
+  if (mOriginNoSuffix) {
+    if (Kind() == eCodebasePrincipal) {
+      return mOriginNoSuffix == other->mOriginNoSuffix &&
+             mOriginSuffix == other->mOriginSuffix;
+    }
+
+    MOZ_ASSERT(Kind() == eExpandedPrincipal);
+    return mOriginNoSuffix == other->mOriginNoSuffix;
   }
 
-  MOZ_ASSERT(Kind() == eExpandedPrincipal);
-  return mOriginNoSuffix == other->mOriginNoSuffix;
+  // 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);
 }
 
 inline bool
 BasePrincipal::FastEqualsConsideringDomain(nsIPrincipal* aOther)
 {
   // If neither of the principals have document.domain set, we use the fast path
   // in Equals().  Otherwise, we fall back to the slow path below.
   auto other = Cast(aOther);
@@ -193,21 +189,24 @@ BasePrincipal::FastEqualsConsideringDoma
 
 inline bool
 BasePrincipal::FastSubsumes(nsIPrincipal* aOther)
 {
   // If two principals are equal, then they both subsume each other.
   // 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 (FastEquals(aOther)) {
+  if (mOriginNoSuffix && FastEquals(aOther)) {
     return true;
   }
 
   // Otherwise, fall back to the slow path.
   return Subsumes(aOther, DontConsiderDocumentDomain);
 }
 
 inline bool
--- a/caps/ContentPrincipal.cpp
+++ b/caps/ContentPrincipal.cpp
@@ -90,18 +90,17 @@ ContentPrincipal::~ContentPrincipal()
   // let's clear the principal within the csp to avoid a tangling pointer
   if (mCSP) {
     static_cast<nsCSPContext*>(mCSP.get())->clearLoadingPrincipal();
   }
 }
 
 nsresult
 ContentPrincipal::Init(nsIURI *aCodebase,
-                       const OriginAttributes& aOriginAttributes,
-                       const nsACString& aOriginNoSuffix)
+                       const OriginAttributes& aOriginAttributes)
 {
   NS_ENSURE_ARG(aCodebase);
 
   // Assert that the URI we get here isn't any of the schemes that we know we
   // should not get here.  These schemes always either inherit their principal
   // or fall back to a null principal.  These are schemes which return
   // URI_INHERITS_SECURITY_CONTEXT from their protocol handler's
   // GetProtocolFlags function.
@@ -111,17 +110,23 @@ ContentPrincipal::Init(nsIURI *aCodebase
       NS_SUCCEEDED(NS_URIChainHasFlags(aCodebase,
                                        nsIProtocolHandler::URI_INHERITS_SECURITY_CONTEXT,
                                        &hasFlag)) &&
       !hasFlag);
 
   mCodebase = NS_TryToMakeImmutable(aCodebase);
   mCodebaseImmutable = URIIsImmutable(mCodebase);
 
-  FinishInit(aOriginNoSuffix, aOriginAttributes);
+  nsAutoCString originNoSuffix;
+  nsresult rv = GenerateOriginNoSuffixFromURI(mCodebase, originNoSuffix);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
+
+  FinishInit(originNoSuffix, aOriginAttributes);
 
   return NS_OK;
 }
 
 nsresult
 ContentPrincipal::GetScriptLocation(nsACString &aStr)
 {
   return mCodebase->GetSpec(aStr);
@@ -475,21 +480,17 @@ ContentPrincipal::Read(nsIObjectInputStr
 
   OriginAttributes attrs;
   bool ok = attrs.PopulateFromSuffix(suffix);
   NS_ENSURE_TRUE(ok, NS_ERROR_FAILURE);
 
   rv = NS_ReadOptionalObject(aStream, true, getter_AddRefs(supports));
   NS_ENSURE_SUCCESS(rv, rv);
 
-  nsAutoCString originNoSuffix;
-  rv = GenerateOriginNoSuffixFromURI(codebase, originNoSuffix);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  rv = Init(codebase, attrs, originNoSuffix);
+  rv = Init(codebase, attrs);
   NS_ENSURE_SUCCESS(rv, rv);
 
   mCSP = do_QueryInterface(supports, &rv);
   // make sure setRequestContext is called after Init(),
   // to make sure  the principals URI been initalized.
   if (mCSP) {
     mCSP->SetRequestContext(nullptr, this);
   }
--- a/caps/ContentPrincipal.h
+++ b/caps/ContentPrincipal.h
@@ -27,43 +27,42 @@ public:
   NS_IMETHOD GetBaseDomain(nsACString& aBaseDomain) override;
   NS_IMETHOD GetAddonId(nsAString& aAddonId) override;
   bool IsCodebasePrincipal() const override { return true; }
 
   ContentPrincipal();
 
   // Init() must be called before the principal is in a usable state.
   nsresult Init(nsIURI* aCodebase,
-                const mozilla::OriginAttributes& aOriginAttributes,
-                const nsACString& aOriginNoSuffix);
+                const mozilla::OriginAttributes& aOriginAttributes);
 
   virtual nsresult GetScriptLocation(nsACString& aStr) override;
 
   /**
    * Called at startup to setup static data, e.g. about:config pref-observers.
    */
   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
   bool mCodebaseImmutable;
   bool mDomainImmutable;
 
 protected:
   virtual ~ContentPrincipal();
 
   bool SubsumesInternal(nsIPrincipal* aOther,
                         DocumentDomainConsideration aConsideration) override;
   bool MayLoadInternal(nsIURI* aURI) override;
 
 private:
+  static nsresult 
+  GenerateOriginNoSuffixFromURI(nsIURI* aURI, nsACString& aOriginNoSuffix);
+
   mozilla::Maybe<nsString> mAddonIdCache;
 };
 
 #define NS_PRINCIPAL_CONTRACTID "@mozilla.org/principal;1"
 #define NS_PRINCIPAL_CID \
 { 0x653e0e4d, 0x3ee4, 0x45fa, \
   { 0xb2, 0x72, 0x97, 0xc2, 0x0b, 0xc0, 0x1e, 0xb8 } }
 
--- a/caps/ExpandedPrincipal.cpp
+++ b/caps/ExpandedPrincipal.cpp
@@ -18,28 +18,28 @@ NS_IMPL_CI_INTERFACE_GETTER(ExpandedPrin
                             nsIPrincipal,
                             nsIExpandedPrincipal)
 
 struct OriginComparator
 {
   bool LessThan(nsIPrincipal* a, nsIPrincipal* b) const
   {
     nsAutoCString originA;
-    DebugOnly<nsresult> rv = a->GetOrigin(originA);
+    nsresult rv = a->GetOrigin(originA);
     MOZ_ASSERT(NS_SUCCEEDED(rv));
     nsAutoCString originB;
     rv = b->GetOrigin(originB);
     MOZ_ASSERT(NS_SUCCEEDED(rv));
     return originA < originB;
   }
 
   bool Equals(nsIPrincipal* a, nsIPrincipal* b) const
   {
     nsAutoCString originA;
-    DebugOnly<nsresult> rv = a->GetOrigin(originA);
+    nsresult rv = a->GetOrigin(originA);
     MOZ_ASSERT(NS_SUCCEEDED(rv));
     nsAutoCString originB;
     rv = b->GetOrigin(originB);
     MOZ_ASSERT(NS_SUCCEEDED(rv));
     return a == b;
   }
 };
 
@@ -66,17 +66,17 @@ ExpandedPrincipal::Create(nsTArray<nsCOM
   nsAutoCString origin;
   origin.AssignLiteral("[Expanded Principal [");
   for (size_t i = 0; i < ep->mPrincipals.Length(); ++i) {
     if (i != 0) {
       origin.AppendLiteral(", ");
     }
 
     nsAutoCString subOrigin;
-    DebugOnly<nsresult> rv = ep->mPrincipals.ElementAt(i)->GetOrigin(subOrigin);
+    nsresult rv = ep->mPrincipals.ElementAt(i)->GetOrigin(subOrigin);
     MOZ_ASSERT(NS_SUCCEEDED(rv));
     origin.Append(subOrigin);
   }
   origin.Append("]]");
 
   ep->FinishInit(origin, aAttrs);
   return ep.forget();
 }
--- a/caps/NullPrincipal.cpp
+++ b/caps/NullPrincipal.cpp
@@ -80,17 +80,17 @@ NullPrincipal::Init(const OriginAttribut
 
     mURI = aURI;
   } else {
     mURI = NullPrincipalURI::Create();
     NS_ENSURE_TRUE(mURI, NS_ERROR_NOT_AVAILABLE);
   }
 
   nsAutoCString originNoSuffix;
-  DebugOnly<nsresult> rv = mURI->GetSpec(originNoSuffix);
+  nsresult rv = mURI->GetSpec(originNoSuffix);
   MOZ_ASSERT(NS_SUCCEEDED(rv));
 
   FinishInit(originNoSuffix, aOriginAttributes);
 
   return NS_OK;
 }
 
 nsresult
--- a/ipc/glue/BackgroundUtils.cpp
+++ b/ipc/glue/BackgroundUtils.cpp
@@ -84,17 +84,18 @@ PrincipalInfoToPrincipal(const Principal
         return nullptr;
       }
 
       OriginAttributes attrs;
       if (info.attrs().mAppId != nsIScriptSecurityManager::UNKNOWN_APP_ID) {
         attrs = info.attrs();
       }
       principal = BasePrincipal::CreateCodebasePrincipal(uri, attrs);
-      if (NS_WARN_IF(!principal)) {
+      rv = principal ? NS_OK : NS_ERROR_FAILURE;
+      if (NS_WARN_IF(NS_FAILED(rv))) {
         return nullptr;
       }
 
       // When the principal is serialized, the origin is extract from it. This
       // can fail, and in case, here we will havea Tvoid_t. If we have a string,
       // it must match with what the_new_principal.getOrigin returns.
       if (info.originNoSuffix().type() == ContentPrincipalInfoOriginNoSuffix::TnsCString) {
         nsAutoCString originNoSuffix;