Bug 1164977 - Hoist app attributes into a struct on BasePrincipal and refer to them as 'origin attributes'. r=gabor
authorBobby Holley <bobbyholley@gmail.com>
Thu, 14 May 2015 12:24:03 -0700
changeset 275235 d4e6af40c5e87de3f2e91c6eeca96954d92c0166
parent 275234 2493c405317f9369d9cc573f0394bc67ae237380
child 275236 e18c56c6ade50594df27b3926df2b4f30eb00505
push id4932
push userjlund@mozilla.com
push dateMon, 10 Aug 2015 18:23:06 +0000
treeherdermozilla-beta@6dd5a4f5f745 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgabor
bugs1164977
milestone41.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 1164977 - Hoist app attributes into a struct on BasePrincipal and refer to them as 'origin attributes'. r=gabor This sets the stage for the upcoming work for signed apps.
caps/BasePrincipal.cpp
caps/BasePrincipal.h
caps/nsNullPrincipal.cpp
caps/nsNullPrincipal.h
caps/nsPrincipal.cpp
caps/nsPrincipal.h
caps/nsScriptSecurityManager.cpp
caps/nsScriptSecurityManager.h
--- a/caps/BasePrincipal.cpp
+++ b/caps/BasePrincipal.cpp
@@ -71,55 +71,55 @@ BasePrincipal::GetIsNullPrincipal(bool* 
 {
   *aIsNullPrincipal = false;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 BasePrincipal::GetJarPrefix(nsACString& aJarPrefix)
 {
-  MOZ_ASSERT(mAppId != nsIScriptSecurityManager::UNKNOWN_APP_ID);
+  MOZ_ASSERT(AppId() != nsIScriptSecurityManager::UNKNOWN_APP_ID);
 
-  mozilla::GetJarPrefix(mAppId, mIsInBrowserElement, aJarPrefix);
+  mozilla::GetJarPrefix(AppId(), IsInBrowserElement(), aJarPrefix);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 BasePrincipal::GetAppStatus(uint16_t* aAppStatus)
 {
-  if (mAppId == nsIScriptSecurityManager::UNKNOWN_APP_ID) {
+  if (AppId() == nsIScriptSecurityManager::UNKNOWN_APP_ID) {
     NS_WARNING("Asking for app status on a principal with an unknown app id");
     *aAppStatus = nsIPrincipal::APP_STATUS_NOT_INSTALLED;
     return NS_OK;
   }
 
   *aAppStatus = nsScriptSecurityManager::AppStatusForPrincipal(this);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 BasePrincipal::GetAppId(uint32_t* aAppId)
 {
-  if (mAppId == nsIScriptSecurityManager::UNKNOWN_APP_ID) {
+  if (AppId() == nsIScriptSecurityManager::UNKNOWN_APP_ID) {
     MOZ_ASSERT(false);
     *aAppId = nsIScriptSecurityManager::NO_APP_ID;
     return NS_OK;
   }
 
-  *aAppId = mAppId;
+  *aAppId = AppId();
   return NS_OK;
 }
 
 NS_IMETHODIMP
 BasePrincipal::GetIsInBrowserElement(bool* aIsInBrowserElement)
 {
-  *aIsInBrowserElement = mIsInBrowserElement;
+  *aIsInBrowserElement = IsInBrowserElement();
   return NS_OK;
 }
 
 NS_IMETHODIMP
 BasePrincipal::GetUnknownAppId(bool* aUnknownAppId)
 {
-  *aUnknownAppId = mAppId == nsIScriptSecurityManager::UNKNOWN_APP_ID;
+  *aUnknownAppId = AppId() == nsIScriptSecurityManager::UNKNOWN_APP_ID;
   return NS_OK;
 }
 
 } // namespace mozilla
--- a/caps/BasePrincipal.h
+++ b/caps/BasePrincipal.h
@@ -18,20 +18,17 @@ namespace mozilla {
  * default implementations and other commonalities between principal
  * implementations.
  *
  * We should merge nsJSPrincipals into this class at some point.
  */
 class BasePrincipal : public nsJSPrincipals
 {
 public:
-  BasePrincipal()
-    : mAppId(nsIScriptSecurityManager::NO_APP_ID)
-    , mIsInBrowserElement(false)
-  {}
+  BasePrincipal() {}
 
   enum DocumentDomainConsideration { DontConsiderDocumentDomain, ConsiderDocumentDomain};
   bool Subsumes(nsIPrincipal* aOther, DocumentDomainConsideration aConsideration);
 
   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;
@@ -43,21 +40,42 @@ public:
   NS_IMETHOD GetAppId(uint32_t* aAppStatus) final;
   NS_IMETHOD GetIsInBrowserElement(bool* aIsInBrowserElement) final;
   NS_IMETHOD GetUnknownAppId(bool* aUnknownAppId) final;
 
   virtual bool IsOnCSSUnprefixingWhitelist() override { return false; }
 
   static BasePrincipal* Cast(nsIPrincipal* aPrin) { return static_cast<BasePrincipal*>(aPrin); }
 
+  struct OriginAttributes {
+    uint32_t mAppId;
+    bool mIsInBrowserElement;
+
+    OriginAttributes() : mAppId(nsIScriptSecurityManager::NO_APP_ID), mIsInBrowserElement(false) {}
+    OriginAttributes(uint32_t aAppId, bool aIsInBrowserElement)
+      : mAppId(aAppId), mIsInBrowserElement(aIsInBrowserElement) {}
+    bool operator==(const OriginAttributes& aOther) const
+    {
+      return mAppId == aOther.mAppId &&
+             mIsInBrowserElement == aOther.mIsInBrowserElement;
+    }
+    bool operator!=(const OriginAttributes& aOther) const
+    {
+      return !(*this == aOther);
+    }
+  };
+
+  const OriginAttributes& OriginAttributesRef() { return mOriginAttributes; }
+  uint32_t AppId() const { return mOriginAttributes.mAppId; }
+  bool IsInBrowserElement() const { return mOriginAttributes.mIsInBrowserElement; }
+
 protected:
   virtual ~BasePrincipal() {}
 
   virtual bool SubsumesInternal(nsIPrincipal* aOther, DocumentDomainConsideration aConsider) = 0;
 
   nsCOMPtr<nsIContentSecurityPolicy> mCSP;
-  uint32_t mAppId;
-  bool mIsInBrowserElement;
+  OriginAttributes mOriginAttributes;
 };
 
 } // namespace mozilla
 
 #endif /* mozilla_BasePrincipal_h */
--- a/caps/nsNullPrincipal.cpp
+++ b/caps/nsNullPrincipal.cpp
@@ -36,37 +36,35 @@ NS_IMPL_QUERY_INTERFACE_CI(nsNullPrincip
 NS_IMPL_CI_INTERFACE_GETTER(nsNullPrincipal,
                             nsIPrincipal,
                             nsISerializable)
 
 /* static */ already_AddRefed<nsNullPrincipal>
 nsNullPrincipal::CreateWithInheritedAttributes(nsIPrincipal* aInheritFrom)
 {
   nsRefPtr<nsNullPrincipal> nullPrin = new nsNullPrincipal();
-  nsresult rv = nullPrin->Init(aInheritFrom->GetAppId(),
-                               aInheritFrom->GetIsInBrowserElement());
+  nsresult rv = nullPrin->Init(Cast(aInheritFrom)->OriginAttributesRef());
   return NS_SUCCEEDED(rv) ? nullPrin.forget() : nullptr;
 }
 
 /* static */ already_AddRefed<nsNullPrincipal>
-nsNullPrincipal::Create(uint32_t aAppId, bool aInMozBrowser)
+nsNullPrincipal::Create(const OriginAttributes& aOriginAttributes)
 {
   nsRefPtr<nsNullPrincipal> nullPrin = new nsNullPrincipal();
-  nsresult rv = nullPrin->Init(aAppId, aInMozBrowser);
+  nsresult rv = nullPrin->Init(aOriginAttributes);
   NS_ENSURE_SUCCESS(rv, nullptr);
 
   return nullPrin.forget();
 }
 
 nsresult
-nsNullPrincipal::Init(uint32_t aAppId, bool aInMozBrowser)
+nsNullPrincipal::Init(const OriginAttributes& aOriginAttributes)
 {
-  MOZ_ASSERT(aAppId != nsIScriptSecurityManager::UNKNOWN_APP_ID);
-  mAppId = aAppId;
-  mIsInBrowserElement = aInMozBrowser;
+  mOriginAttributes = aOriginAttributes;
+  MOZ_ASSERT(AppId() != nsIScriptSecurityManager::UNKNOWN_APP_ID);
 
   mURI = nsNullPrincipalURI::Create();
   NS_ENSURE_TRUE(mURI, NS_ERROR_NOT_AVAILABLE);
 
   return NS_OK;
 }
 
 void
@@ -159,25 +157,25 @@ nsNullPrincipal::GetBaseDomain(nsACStrin
  */
 NS_IMETHODIMP
 nsNullPrincipal::Read(nsIObjectInputStream* aStream)
 {
   // Note - nsNullPrincipal use NS_GENERIC_FACTORY_CONSTRUCTOR_INIT, which means
   // that the Init() method has already been invoked by the time we deserialize.
   // This is in contrast to nsPrincipal, which uses NS_GENERIC_FACTORY_CONSTRUCTOR,
   // in which case ::Read needs to invoke Init().
-  nsresult rv = aStream->Read32(&mAppId);
+  nsresult rv = aStream->Read32(&mOriginAttributes.mAppId);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  rv = aStream->ReadBoolean(&mIsInBrowserElement);
+  rv = aStream->ReadBoolean(&mOriginAttributes.mIsInBrowserElement);
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsNullPrincipal::Write(nsIObjectOutputStream* aStream)
 {
-  aStream->Write32(mAppId);
-  aStream->WriteBoolean(mIsInBrowserElement);
+  aStream->Write32(AppId());
+  aStream->WriteBoolean(IsInBrowserElement());
   return NS_OK;
 }
 
--- a/caps/nsNullPrincipal.h
+++ b/caps/nsNullPrincipal.h
@@ -49,21 +49,19 @@ public:
   NS_IMETHOD GetIsNullPrincipal(bool* aIsNullPrincipal) override;
   NS_IMETHOD GetBaseDomain(nsACString& aBaseDomain) override;
 
   // Returns null on failure.
   static already_AddRefed<nsNullPrincipal> CreateWithInheritedAttributes(nsIPrincipal *aInheritFrom);
 
   // Returns null on failure.
   static already_AddRefed<nsNullPrincipal>
-    Create(uint32_t aAppId = nsIScriptSecurityManager::NO_APP_ID,
-           bool aInMozBrowser = false);
+    Create(const OriginAttributes& aOriginAttributes = OriginAttributes());
 
-  nsresult Init(uint32_t aAppId = nsIScriptSecurityManager::NO_APP_ID,
-                bool aInMozBrowser = false);
+  nsresult Init(const OriginAttributes& aOriginAttributes = OriginAttributes());
 
   virtual void GetScriptLocation(nsACString &aStr) override;
 
  protected:
   virtual ~nsNullPrincipal() {}
 
   bool SubsumesInternal(nsIPrincipal* aOther, DocumentDomainConsideration aConsideration) override
   {
--- a/caps/nsPrincipal.cpp
+++ b/caps/nsPrincipal.cpp
@@ -73,30 +73,26 @@ nsPrincipal::nsPrincipal()
   , mDomainImmutable(false)
   , mInitialized(false)
 { }
 
 nsPrincipal::~nsPrincipal()
 { }
 
 nsresult
-nsPrincipal::Init(nsIURI *aCodebase,
-                  uint32_t aAppId,
-                  bool aInMozBrowser)
+nsPrincipal::Init(nsIURI *aCodebase, const OriginAttributes& aOriginAttributes)
 {
   NS_ENSURE_STATE(!mInitialized);
   NS_ENSURE_ARG(aCodebase);
 
   mInitialized = true;
 
   mCodebase = NS_TryToMakeImmutable(aCodebase);
   mCodebaseImmutable = URIIsImmutable(mCodebase);
-
-  mAppId = aAppId;
-  mIsInBrowserElement = aInMozBrowser;
+  mOriginAttributes = aOriginAttributes;
 
   return NS_OK;
 }
 
 void
 nsPrincipal::GetScriptLocation(nsACString &aStr)
 {
   mCodebase->GetSpec(aStr);
@@ -167,18 +163,18 @@ nsPrincipal::SubsumesInternal(nsIPrincip
 {
   MOZ_ASSERT(aOther);
 
   // For nsPrincipal, Subsumes is equivalent to Equals.
   if (aOther == this) {
     return true;
   }
 
-  if (!nsScriptSecurityManager::AppAttributesEqual(this, aOther)) {
-      return false;
+  if (OriginAttributesRef() != Cast(aOther)->OriginAttributesRef()) {
+    return false;
   }
 
   // If either the subject or the object has changed its principal by
   // explicitly setting document.domain then the other must also have
   // done so in order to be considered the same origin. This prevents
   // DNS spoofing based on document.domain (154930)
   nsresult rv;
   if (aConsideration == ConsiderDocumentDomain) {
@@ -375,17 +371,18 @@ nsPrincipal::Read(nsIObjectInputStream* 
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = NS_ReadOptionalObject(aStream, true, getter_AddRefs(supports));
   NS_ENSURE_SUCCESS(rv, rv);
 
   // This may be null.
   nsCOMPtr<nsIContentSecurityPolicy> csp = do_QueryInterface(supports, &rv);
 
-  rv = Init(codebase, appId, inMozBrowser);
+  OriginAttributes attrs(appId, inMozBrowser);
+  rv = Init(codebase, attrs);
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = SetCsp(csp);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // need to link in the CSP context here (link in the URI of the protected
   // resource).
   if (csp) {
@@ -409,18 +406,18 @@ nsPrincipal::Write(nsIObjectOutputStream
   }
 
   rv = NS_WriteOptionalCompoundObject(aStream, mDomain, NS_GET_IID(nsIURI),
                                       true);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
-  aStream->Write32(mAppId);
-  aStream->WriteBoolean(mIsInBrowserElement);
+  aStream->Write32(AppId());
+  aStream->WriteBoolean(IsInBrowserElement());
 
   rv = NS_WriteOptionalCompoundObject(aStream, mCSP,
                                       NS_GET_IID(nsIContentSecurityPolicy),
                                       true);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
--- a/caps/nsPrincipal.h
+++ b/caps/nsPrincipal.h
@@ -29,19 +29,17 @@ public:
   NS_IMETHOD GetOrigin(nsACString& aOrigin) override;
   NS_IMETHOD CheckMayLoad(nsIURI* uri, bool report, bool allowIfInheritsPrincipal) override;
   NS_IMETHOD GetBaseDomain(nsACString& aBaseDomain) override;
   virtual bool IsOnCSSUnprefixingWhitelist() override;
 
   nsPrincipal();
 
   // Init() must be called before the principal is in a usable state.
-  nsresult Init(nsIURI* aCodebase,
-                uint32_t aAppId,
-                bool aInMozBrowser);
+  nsresult Init(nsIURI* aCodebase, const 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;
--- a/caps/nsScriptSecurityManager.cpp
+++ b/caps/nsScriptSecurityManager.cpp
@@ -482,36 +482,16 @@ nsScriptSecurityManager::HashPrincipalBy
 {
     nsCOMPtr<nsIURI> uri;
     aPrincipal->GetDomain(getter_AddRefs(uri));
     if (!uri)
         aPrincipal->GetURI(getter_AddRefs(uri));
     return SecurityHashURI(uri);
 }
 
-/* static */ bool
-nsScriptSecurityManager::AppAttributesEqual(nsIPrincipal* aFirst,
-                                            nsIPrincipal* aSecond)
-{
-    MOZ_ASSERT(aFirst && aSecond, "Don't pass null pointers!");
-
-    uint32_t firstAppId = nsIScriptSecurityManager::UNKNOWN_APP_ID;
-    if (!aFirst->GetUnknownAppId()) {
-        firstAppId = aFirst->GetAppId();
-    }
-
-    uint32_t secondAppId = nsIScriptSecurityManager::UNKNOWN_APP_ID;
-    if (!aSecond->GetUnknownAppId()) {
-        secondAppId = aSecond->GetAppId();
-    }
-
-    return ((firstAppId == secondAppId) &&
-            (aFirst->GetIsInBrowserElement() == aSecond->GetIsInBrowserElement()));
-}
-
 NS_IMETHODIMP
 nsScriptSecurityManager::CheckLoadURIFromScript(JSContext *cx, nsIURI *aURI)
 {
     // Get principal of currently executing script.
     MOZ_ASSERT(cx == nsContentUtils::GetCurrentJSContext());
     nsIPrincipal* principal = nsContentUtils::SubjectPrincipal();
     nsresult rv = CheckLoadURIWithPrincipal(principal, aURI,
                                             nsIScriptSecurityManager::STANDARD);
@@ -1017,19 +997,19 @@ nsScriptSecurityManager::CreateCodebaseP
             NS_ENSURE_TRUE(principal, NS_ERROR_FAILURE);
         }
 
         principal.forget(result);
 
         return NS_OK;
     }
 
+    BasePrincipal::OriginAttributes attrs(aAppId, aInMozBrowser);
     nsRefPtr<nsPrincipal> codebase = new nsPrincipal();
-
-    nsresult rv = codebase->Init(aURI, aAppId, aInMozBrowser);
+    nsresult rv = codebase->Init(aURI, attrs);
     if (NS_FAILED(rv))
         return rv;
 
     NS_ADDREF(*result = codebase);
 
     return NS_OK;
 }
 
--- a/caps/nsScriptSecurityManager.h
+++ b/caps/nsScriptSecurityManager.h
@@ -75,32 +75,16 @@ public:
     HashPrincipalByOrigin(nsIPrincipal* aPrincipal);
 
     static bool
     GetStrictFileOriginPolicy()
     {
         return sStrictFileOriginPolicy;
     }
 
-    /**
-     * Returns true if the two principals share the same app attributes.
-     *
-     * App attributes are appId and the inBrowserElement flag.
-     * Two principals have the same app attributes if those information are
-     * equals.
-     * This method helps keeping principals from different apps isolated from
-     * each other. Also, it helps making sure mozbrowser (web views) and their
-     * parent are isolated from each other. All those entities do not share the
-     * same data (cookies, IndexedDB, localStorage, etc.) so we shouldn't allow
-     * violating that principle.
-     */
-    static bool
-    AppAttributesEqual(nsIPrincipal* aFirst,
-                       nsIPrincipal* aSecond);
-
     void DeactivateDomainPolicy();
 
 private:
 
     // GetScriptSecurityManager is the only call that can make one
     nsScriptSecurityManager();
     virtual ~nsScriptSecurityManager();