Bug 1167100 - User originAttribute in ContentPrincipalInfo. r=bholley

dothq · Sep 23, 2015 · f97211a · f97211a
1 parent 5064006
commit f97211a
Show file tree

Hide file tree

Showing 9 changed files with 39 additions and 77 deletions.
diff --git a/caps/BasePrincipal.cpp b/caps/BasePrincipal.cpp
@@ -379,7 +379,7 @@ BasePrincipal::GetUnknownAppId(bool* aUnknownAppId)
 }
 
 already_AddRefed<BasePrincipal>
-BasePrincipal::CreateCodebasePrincipal(nsIURI* aURI, OriginAttributes& aAttrs)
+BasePrincipal::CreateCodebasePrincipal(nsIURI* aURI, const OriginAttributes& aAttrs)
 {
   // If the URI is supposed to inherit the security context of whoever loads it,
   // we shouldn't make a codebase principal for it.

diff --git a/caps/BasePrincipal.h b/caps/BasePrincipal.h
@@ -159,7 +159,8 @@ class BasePrincipal : public nsJSPrincipals
   virtual bool IsCodebasePrincipal() const { return false; };
 
   static BasePrincipal* Cast(nsIPrincipal* aPrin) { return static_cast<BasePrincipal*>(aPrin); }
-  static already_AddRefed<BasePrincipal> CreateCodebasePrincipal(nsIURI* aURI, OriginAttributes& aAttrs);
+  static already_AddRefed<BasePrincipal>
+  CreateCodebasePrincipal(nsIURI* aURI, const OriginAttributes& aAttrs);
   static already_AddRefed<BasePrincipal> CreateCodebasePrincipal(const nsACString& aOrigin);
 
   const OriginAttributes& OriginAttributesRef() { return mOriginAttributes; }

diff --git a/dom/base/StructuredCloneHolder.cpp b/dom/base/StructuredCloneHolder.cpp
@@ -426,16 +426,16 @@ StructuredCloneHolder::ReadFullySerializableObjects(JSContext* aCx,
     } else if (aTag == SCTAG_DOM_NULL_PRINCIPAL) {
       info = mozilla::ipc::NullPrincipalInfo();
     } else {
-      uint32_t appId = aIndex;
-
-      uint32_t isInBrowserElement, specLength;
-      if (!JS_ReadUint32Pair(aReader, &isInBrowserElement, &specLength)) {
+
+      uint32_t suffixLength, specLength;
+      if (!JS_ReadUint32Pair(aReader, &suffixLength, &specLength)) {
         return nullptr;
       }
 
-      uint32_t signedPkgLength, dummy;
-      if (!JS_ReadUint32Pair(aReader, &signedPkgLength, &dummy)) {
-        return nullptr;
+      nsAutoCString suffix;
+      suffix.SetLength(suffixLength);
+      if (!JS_ReadBytes(aReader, suffix.BeginWriting(), suffixLength)) {
+       return nullptr;
       }
 
       nsAutoCString spec;
@@ -444,14 +444,9 @@ StructuredCloneHolder::ReadFullySerializableObjects(JSContext* aCx,
         return nullptr;
       }
 
-      nsAutoCString signedPkg;
-      signedPkg.SetLength(signedPkgLength);
-      if (!JS_ReadBytes(aReader, signedPkg.BeginWriting(), signedPkgLength)) {
-        return nullptr;
-      }
-
-      info = mozilla::ipc::ContentPrincipalInfo(appId, isInBrowserElement,
-                                                spec, signedPkg);
+      OriginAttributes attrs;
+      attrs.PopulateFromSuffix(suffix);
+      info = mozilla::ipc::ContentPrincipalInfo(attrs, spec);
     }
 
     nsresult rv;
@@ -578,13 +573,12 @@ StructuredCloneHolder::WriteFullySerializableObjects(JSContext* aCx,
 
       MOZ_ASSERT(info.type() == mozilla::ipc::PrincipalInfo::TContentPrincipalInfo);
       const mozilla::ipc::ContentPrincipalInfo& cInfo = info;
-      return JS_WriteUint32Pair(aWriter, SCTAG_DOM_CONTENT_PRINCIPAL,
-                                cInfo.appId()) &&
-             JS_WriteUint32Pair(aWriter, cInfo.isInBrowserElement(),
-                                cInfo.spec().Length()) &&
-             JS_WriteUint32Pair(aWriter, cInfo.signedPkg().Length(), 0) &&
-             JS_WriteBytes(aWriter, cInfo.spec().get(), cInfo.spec().Length()) &&
-             JS_WriteBytes(aWriter, cInfo.signedPkg().get(), cInfo.signedPkg().Length());
+      nsAutoCString suffix;
+      cInfo.attrs().CreateSuffix(suffix);
+      return JS_WriteUint32Pair(aWriter, SCTAG_DOM_CONTENT_PRINCIPAL, 0) &&
+             JS_WriteUint32Pair(aWriter, suffix.Length(), cInfo.spec().Length()) &&
+             JS_WriteBytes(aWriter, suffix.get(), suffix.Length()) &&
+             JS_WriteBytes(aWriter, cInfo.spec().get(), cInfo.spec().Length());
     }
   }
 

diff --git a/dom/cache/CacheStorage.cpp b/dom/cache/CacheStorage.cpp
@@ -81,7 +81,7 @@ IsTrusted(const PrincipalInfo& aPrincipalInfo, bool aTestingPrefEnabled)
   // worker.  We require exact knowledge of this information before allowing
   // the caller to touch the disk using the Cache API.
   if (NS_WARN_IF(aPrincipalInfo.type() != PrincipalInfo::TContentPrincipalInfo ||
-                 aPrincipalInfo.get_ContentPrincipalInfo().appId() ==
+                 aPrincipalInfo.get_ContentPrincipalInfo().attrs().mAppId ==
                  nsIScriptSecurityManager::UNKNOWN_APP_ID)) {
     return false;
   }

diff --git a/dom/cache/DBSchema.cpp b/dom/cache/DBSchema.cpp
@@ -1740,10 +1740,8 @@ InsertEntry(mozIStorageConnection* aConn, CacheId aCacheId,
 
     serializedInfo.Append(cInfo.spec());
 
-    MOZ_ASSERT(cInfo.appId() != nsIScriptSecurityManager::UNKNOWN_APP_ID);
-    OriginAttributes attrs(cInfo.appId(), cInfo.isInBrowserElement());
     nsAutoCString suffix;
-    attrs.CreateSuffix(suffix);
+    cInfo.attrs().CreateSuffix(suffix);
     serializedInfo.Append(suffix);
   }
 
@@ -1913,9 +1911,8 @@ ReadResponse(mozIStorageConnection* aConn, EntryId aEntryId,
       return NS_ERROR_FAILURE;
     }
 
-    nsCString signedPkg = NS_ConvertUTF16toUTF8(attrs.mSignedPkg);
     aSavedResponseOut->mValue.principalInfo() =
-      mozilla::ipc::ContentPrincipalInfo(attrs.mAppId, attrs.mInBrowser, originNoSuffix, signedPkg);
+      mozilla::ipc::ContentPrincipalInfo(attrs, originNoSuffix);
   }
 
   int32_t redirected;

diff --git a/dom/workers/ServiceWorkerRegistrar.cpp b/dom/workers/ServiceWorkerRegistrar.cpp
@@ -332,9 +332,8 @@ ServiceWorkerRegistrar::ReadData()
     }
 
     GET_LINE(line);
-    nsCString signedPkg = NS_ConvertUTF16toUTF8(attrs.mSignedPkg);
     entry->principal() =
-      mozilla::ipc::ContentPrincipalInfo(attrs.mAppId, attrs.mInBrowser, line, signedPkg);
+      mozilla::ipc::ContentPrincipalInfo(attrs, line);
 
     GET_LINE(entry->scope());
     GET_LINE(entry->scriptSpec());
@@ -549,9 +548,8 @@ ServiceWorkerRegistrar::WriteData()
     const mozilla::ipc::ContentPrincipalInfo& cInfo =
       info.get_ContentPrincipalInfo();
 
-    OriginAttributes attrs(cInfo.appId(), cInfo.isInBrowserElement());
     nsAutoCString suffix;
-    attrs.CreateSuffix(suffix);
+    cInfo.attrs().CreateSuffix(suffix);
 
     buffer.Truncate();
     buffer.Append(suffix.get());

diff --git a/dom/workers/test/gtest/TestReadWrite.cpp b/dom/workers/test/gtest/TestReadWrite.cpp
@@ -163,9 +163,8 @@ TEST(ServiceWorkerRegistrar, TestReadData)
   ASSERT_EQ(info0.type(), mozilla::ipc::PrincipalInfo::TContentPrincipalInfo) << "First principal must be content";
   const mozilla::ipc::ContentPrincipalInfo& cInfo0 = data[0].principal();
 
-  mozilla::OriginAttributes attrs0(cInfo0.appId(), cInfo0.isInBrowserElement());
   nsAutoCString suffix0;
-  attrs0.CreateSuffix(suffix0);
+  cInfo0.attrs().CreateSuffix(suffix0);
 
   ASSERT_STREQ("^appId=123&inBrowser=1", suffix0.get());
   ASSERT_STREQ("spec 0", cInfo0.spec().get());
@@ -179,9 +178,8 @@ TEST(ServiceWorkerRegistrar, TestReadData)
   ASSERT_EQ(info1.type(), mozilla::ipc::PrincipalInfo::TContentPrincipalInfo) << "First principal must be content";
   const mozilla::ipc::ContentPrincipalInfo& cInfo1 = data[1].principal();
 
-  mozilla::OriginAttributes attrs1(cInfo1.appId(), cInfo1.isInBrowserElement());
   nsAutoCString suffix1;
-  attrs1.CreateSuffix(suffix1);
+  cInfo1.attrs().CreateSuffix(suffix1);
 
   ASSERT_STREQ("", suffix1.get());
   ASSERT_STREQ("spec 1", cInfo1.spec().get());
@@ -221,7 +219,7 @@ TEST(ServiceWorkerRegistrar, TestWriteData)
 
       nsAutoCString spec;
       spec.AppendPrintf("spec write %d", i);
-      d->principal() = mozilla::ipc::ContentPrincipalInfo(i, i % 2, spec, EmptyCString());
+      d->principal() = mozilla::ipc::ContentPrincipalInfo(mozilla::OriginAttributes(i, i % 2), spec);
       d->scope().AppendPrintf("scope write %d", i);
       d->scriptSpec().AppendPrintf("scriptSpec write %d", i);
       d->currentWorkerURL().AppendPrintf("currentWorkerURL write %d", i);
@@ -247,8 +245,12 @@ TEST(ServiceWorkerRegistrar, TestWriteData)
     ASSERT_EQ(data[i].principal().type(), mozilla::ipc::PrincipalInfo::TContentPrincipalInfo);
     const mozilla::ipc::ContentPrincipalInfo& cInfo = data[i].principal();
 
-    ASSERT_EQ((uint32_t)i, cInfo.appId());
-    ASSERT_EQ((uint32_t)(i % 2), (uint32_t)cInfo.isInBrowserElement());
+    mozilla::OriginAttributes attrs(i, i % 2);
+    nsAutoCString suffix, expectSuffix;
+    attrs.CreateSuffix(expectSuffix);
+    cInfo.attrs().CreateSuffix(suffix);
+
+    ASSERT_STREQ(expectSuffix.get(), suffix.get());
 
     test.AppendPrintf("spec write %d", i);
     ASSERT_STREQ(test.get(), cInfo.spec().get());

diff --git a/ipc/glue/BackgroundUtils.cpp b/ipc/glue/BackgroundUtils.cpp
@@ -25,7 +25,6 @@ class OptionalLoadInfoArgs;
 }
 
 using mozilla::BasePrincipal;
-using mozilla::OriginAttributes;
 using namespace mozilla::net;
 
 namespace ipc {
@@ -77,13 +76,10 @@ PrincipalInfoToPrincipal(const PrincipalInfo& aPrincipalInfo,
         return nullptr;
       }
 
-      if (info.appId() == nsIScriptSecurityManager::UNKNOWN_APP_ID) {
+      if (info.attrs().mAppId == nsIScriptSecurityManager::UNKNOWN_APP_ID) {
         rv = secMan->GetSimpleCodebasePrincipal(uri, getter_AddRefs(principal));
       } else {
-        // TODO: Bug 1167100 - User nsIPrincipal.originAttribute in ContentPrincipalInfo
-        OriginAttributes attrs(info.appId(), info.isInBrowserElement());
-        attrs.mSignedPkg = NS_ConvertUTF8toUTF16(info.signedPkg());
-        principal = BasePrincipal::CreateCodebasePrincipal(uri, attrs);
+        principal = BasePrincipal::CreateCodebasePrincipal(uri, info.attrs());
         rv = principal ? NS_OK : NS_ERROR_FAILURE;
       }
       if (NS_WARN_IF(NS_FAILED(rv))) {
@@ -203,33 +199,8 @@ PrincipalToPrincipalInfo(nsIPrincipal* aPrincipal,
     return rv;
   }
 
-  const mozilla::OriginAttributes& attr =
-	mozilla::BasePrincipal::Cast(aPrincipal)->OriginAttributesRef();
-  nsCString signedPkg = NS_ConvertUTF16toUTF8(attr.mSignedPkg);
-
-  bool isUnknownAppId;
-  rv = aPrincipal->GetUnknownAppId(&isUnknownAppId);
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    return rv;
-  }
-
-  uint32_t appId;
-  if (isUnknownAppId) {
-    appId = nsIScriptSecurityManager::UNKNOWN_APP_ID;
-  } else {
-    rv = aPrincipal->GetAppId(&appId);
-    if (NS_WARN_IF(NS_FAILED(rv))) {
-      return rv;
-    }
-  }
-
-  bool isInBrowserElement;
-  rv = aPrincipal->GetIsInBrowserElement(&isInBrowserElement);
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    return rv;
-  }
-
-  *aPrincipalInfo = ContentPrincipalInfo(appId, isInBrowserElement, spec, signedPkg);
+  *aPrincipalInfo = ContentPrincipalInfo(BasePrincipal::Cast(aPrincipal)->OriginAttributesRef(),
+                                         spec);
   return NS_OK;
 }
 

diff --git a/ipc/glue/PBackgroundSharedTypes.ipdlh b/ipc/glue/PBackgroundSharedTypes.ipdlh
@@ -2,17 +2,16 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+using mozilla::OriginAttributes from "mozilla/ipc/BackgroundUtils.h";
 using struct mozilla::void_t from "ipc/IPCMessageUtils.h";
 
 namespace mozilla {
 namespace ipc {
 
 struct ContentPrincipalInfo
 {
-  uint32_t appId;
-  bool isInBrowserElement;
+  OriginAttributes attrs;
   nsCString spec;
-  nsCString signedPkg;
 };
 
 struct SystemPrincipalInfo