Fixing bug 495112. Partially revert the fix for bug 455070 to fix session storage regressions. r+sr=jst@mozilla.org

docshell/base/nsDocShell.cpp

@@ -2140,6 +2140,34 @@ nsDocShell::HistoryPurged(PRInt32 aNumEntries)
     return NS_OK;
 }
 
+static
+nsresult
+GetPrincipalDomain(nsIPrincipal* aPrincipal, nsACString& aDomain)
+{
+  aDomain.Truncate();
+
+  nsCOMPtr<nsIURI> codebaseURI;
+  nsresult rv = aPrincipal->GetDomain(getter_AddRefs(codebaseURI));
+  NS_ENSURE_SUCCESS(rv, rv);
+  if (!codebaseURI) {
+     rv = aPrincipal->GetURI(getter_AddRefs(codebaseURI));
+     NS_ENSURE_SUCCESS(rv, rv);
+  }
+
+  if (!codebaseURI)
+     return NS_OK;
+
+  nsCOMPtr<nsIURI> innerURI = NS_GetInnermostURI(codebaseURI);
+  NS_ASSERTION(innerURI, "Failed to get innermost URI");
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  rv = innerURI->GetAsciiHost(aDomain);
+  if (NS_FAILED(rv))
+      return rv;
+
+  return NS_OK;
+}
+
 NS_IMETHODIMP
 nsDocShell::GetSessionStorageForPrincipal(nsIPrincipal* aPrincipal,
                                           PRBool aCreate,
@@ -2166,15 +2194,15 @@ nsDocShell::GetSessionStorageForPrincipal(nsIPrincipal* aPrincipal,
         return topDocShell->GetSessionStorageForPrincipal(aPrincipal, aCreate,
                                                           aStorage);
 
-    nsXPIDLCString origin;
-    rv = aPrincipal->GetOrigin(getter_Copies(origin));
+    nsCAutoString currentDomain;
+    rv = GetPrincipalDomain(aPrincipal, currentDomain);
     if (NS_FAILED(rv))
         return rv;
 
-    if (origin.IsEmpty())
-        return NS_ERROR_FAILURE;
+    if (currentDomain.IsEmpty())
+        return NS_OK;
 
-    if (!mStorages.Get(origin, aStorage) && aCreate) {
+    if (!mStorages.Get(currentDomain, aStorage) && aCreate) {
         nsCOMPtr<nsIDOMStorage> newstorage =
             do_CreateInstance("@mozilla.org/dom/storage;2");
         if (!newstorage)
@@ -2183,9 +2211,11 @@ nsDocShell::GetSessionStorageForPrincipal(nsIPrincipal* aPrincipal,
         nsCOMPtr<nsPIDOMStorage> pistorage = do_QueryInterface(newstorage);
         if (!pistorage)
             return NS_ERROR_FAILURE;
-        pistorage->InitAsSessionStorage(aPrincipal);
+        rv = pistorage->InitAsSessionStorage(aPrincipal);
+        if (NS_FAILED(rv))
+            return rv;
 
-        if (!mStorages.Put(origin, newstorage))
+        if (!mStorages.Put(currentDomain, newstorage))
             return NS_ERROR_OUT_OF_MEMORY;
 
         newstorage.swap(*aStorage);
@@ -2256,19 +2286,19 @@ nsDocShell::AddSessionStorage(nsIPrincipal* aPrincipal,
     if (topItem) {
         nsCOMPtr<nsIDocShell> topDocShell = do_QueryInterface(topItem);
         if (topDocShell == this) {
-            nsXPIDLCString origin;
-            rv = aPrincipal->GetOrigin(getter_Copies(origin));
+            nsCAutoString currentDomain;
+            rv = GetPrincipalDomain(aPrincipal, currentDomain);
             if (NS_FAILED(rv))
                 return rv;
 
-            if (origin.IsEmpty())
+            if (currentDomain.IsEmpty())
                 return NS_ERROR_FAILURE;
 
             // Do not replace an existing session storage.
-            if (mStorages.GetWeak(origin))
+            if (mStorages.GetWeak(currentDomain))
                 return NS_ERROR_NOT_AVAILABLE;
 
-            if (!mStorages.Put(origin, aStorage))
+            if (!mStorages.Put(currentDomain, aStorage))
                 return NS_ERROR_OUT_OF_MEMORY;
         }
         else {

dom/src/storage/nsDOMStorage.cpp

@@ -535,25 +535,12 @@ nsDOMStorage::nsDOMStorage()
   , mLocalStorage(PR_FALSE)
   , mItemsCached(PR_FALSE)
 {
+  mSecurityChecker = this;
   mItems.Init(8);
   if (nsDOMStorageManager::gStorageManager)
     nsDOMStorageManager::gStorageManager->AddToStoragesHash(this);
 }
 
-static PLDHashOperator
-CopyStorageItems(nsSessionStorageEntry* aEntry, void* userArg)
-{
-  nsDOMStorage* newstorage = static_cast<nsDOMStorage*>(userArg);
-
-  newstorage->SetItem(aEntry->GetKey(), aEntry->mItem->GetValueInternal());
-
-  if (aEntry->mItem->IsSecure()) {
-    newstorage->SetSecure(aEntry->GetKey(), PR_TRUE);
-  }
-
-  return PL_DHASH_NEXT;
-}
-
 nsDOMStorage::nsDOMStorage(nsDOMStorage& aThat)
   : mUseDB(PR_FALSE) // Any clone is not using the database
   , mSessionOnly(PR_TRUE)
@@ -564,8 +551,8 @@ nsDOMStorage::nsDOMStorage(nsDOMStorage& aThat)
   , mScopeDBKey(aThat.mScopeDBKey)
 #endif
 {
+  mSecurityChecker = this;
   mItems.Init(8);
-  aThat.mItems.EnumerateEntries(CopyStorageItems, this);
 
   if (nsDOMStorageManager::gStorageManager)
     nsDOMStorageManager::gStorageManager->AddToStoragesHash(this);
@@ -579,11 +566,19 @@ nsDOMStorage::~nsDOMStorage()
 
 static
 nsresult
-GetDomainURI(nsIPrincipal *aPrincipal, nsIURI **_domain)
+GetDomainURI(nsIPrincipal *aPrincipal, PRBool aIncludeDomain, nsIURI **_domain)
 {
   nsCOMPtr<nsIURI> uri;
-  nsresult rv = aPrincipal->GetURI(getter_AddRefs(uri));
-  NS_ENSURE_SUCCESS(rv, rv);
+
+  if (aIncludeDomain) {
+    nsresult rv = aPrincipal->GetDomain(getter_AddRefs(uri));
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
+
+  if (!uri) {
+    nsresult rv = aPrincipal->GetURI(getter_AddRefs(uri));
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
 
   // Check if we really got any URI. System principal doesn't return a URI
   // instance and we would crash in NS_GetInnermostURI below.
@@ -602,7 +597,7 @@ nsresult
 nsDOMStorage::InitAsSessionStorage(nsIPrincipal *aPrincipal)
 {
   nsCOMPtr<nsIURI> domainURI;
-  nsresult rv = GetDomainURI(aPrincipal, getter_AddRefs(domainURI));
+  nsresult rv = GetDomainURI(aPrincipal, PR_TRUE, getter_AddRefs(domainURI));
   NS_ENSURE_SUCCESS(rv, rv);
 
   // No need to check for a return value. If this would fail we would not get
@@ -624,7 +619,7 @@ nsresult
 nsDOMStorage::InitAsLocalStorage(nsIPrincipal *aPrincipal)
 {
   nsCOMPtr<nsIURI> domainURI;
-  nsresult rv = GetDomainURI(aPrincipal, getter_AddRefs(domainURI));
+  nsresult rv = GetDomainURI(aPrincipal, PR_FALSE, getter_AddRefs(domainURI));
   NS_ENSURE_SUCCESS(rv, rv);
 
   // No need to check for a return value. If this would fail we would not get
@@ -670,6 +665,27 @@ nsDOMStorage::InitAsGlobalStorage(const nsACString &aDomainDemanded)
   return NS_OK;
 }
 
+static PLDHashOperator
+CopyStorageItems(nsSessionStorageEntry* aEntry, void* userArg)
+{
+  nsDOMStorage* newstorage = static_cast<nsDOMStorage*>(userArg);
+
+  newstorage->SetItem(aEntry->GetKey(), aEntry->mItem->GetValueInternal());
+
+  if (aEntry->mItem->IsSecure()) {
+    newstorage->SetSecure(aEntry->GetKey(), PR_TRUE);
+  }
+
+  return PL_DHASH_NEXT;
+}
+
+nsresult
+nsDOMStorage::CloneFrom(nsDOMStorage* aThat)
+{
+  aThat->mItems.EnumerateEntries(CopyStorageItems, this);
+  return NS_OK;
+}
+
 //static
 PRBool
 nsDOMStorage::CanUseStorage(PRPackedBool* aSessionOnly)
@@ -750,7 +766,8 @@ nsDOMStorage::CacheStoragePermissions()
   nsCOMPtr<nsIPrincipal> subjectPrincipal;
   ssm->GetSubjectPrincipal(getter_AddRefs(subjectPrincipal));
 
-  return CanAccess(subjectPrincipal);
+  NS_ASSERTION(mSecurityChecker, "Has non-null mSecurityChecker");
+  return mSecurityChecker->CanAccess(subjectPrincipal);
 }
 
 
@@ -1404,6 +1421,7 @@ nsDOMStorage2::nsDOMStorage2()
 nsDOMStorage2::nsDOMStorage2(nsDOMStorage2& aThat)
 {
   mStorage = new nsDOMStorage(*aThat.mStorage.get());
+  mStorage->mSecurityChecker = mStorage;
   mPrincipal = aThat.mPrincipal;
 }
 
@@ -1414,6 +1432,8 @@ nsDOMStorage2::InitAsSessionStorage(nsIPrincipal *aPrincipal)
   if (!mStorage)
     return NS_ERROR_OUT_OF_MEMORY;
 
+  // Leave security checks only for domain (nsDOMStorage implementation)
+  mStorage->mSecurityChecker = mStorage;
   mPrincipal = aPrincipal;
   return mStorage->InitAsSessionStorage(aPrincipal);
 }
@@ -1425,6 +1445,7 @@ nsDOMStorage2::InitAsLocalStorage(nsIPrincipal *aPrincipal)
   if (!mStorage)
     return NS_ERROR_OUT_OF_MEMORY;
 
+  mStorage->mSecurityChecker = this;
   mPrincipal = aPrincipal;
   return mStorage->InitAsLocalStorage(aPrincipal);
 }
@@ -1443,6 +1464,7 @@ nsDOMStorage2::Clone()
   if (!storage)
     return nsnull;
 
+  storage->mStorage->CloneFrom(mStorage);
   NS_ADDREF(storage);
 
   return storage;
@@ -1463,6 +1485,9 @@ nsDOMStorage2::Principal()
 PRBool
 nsDOMStorage2::CanAccess(nsIPrincipal *aPrincipal)
 {
+  if (mStorage->mSecurityChecker != this)
+    return mStorage->mSecurityChecker->CanAccess(aPrincipal);
+
   // Allow C++ callers to access the storage
   if (!aPrincipal)
     return PR_TRUE;

dom/src/storage/nsDOMStorage.h

@@ -193,6 +193,9 @@ public:
   // clear all values from the store
   void ClearAll();
 
+  nsresult
+  CloneFrom(nsDOMStorage* aThat);
+
   nsIDOMStorageItem* GetNamedItem(const nsAString& aKey, nsresult* aResult);
 
   static nsDOMStorage* FromSupports(nsISupports* aSupports)
@@ -244,6 +247,9 @@ protected:
   nsCString mScopeDBKey;
   nsCString mQuotaDomainDBKey;
 
+  friend class nsIDOMStorage2;
+  nsPIDOMStorage* mSecurityChecker;
+
 public:
   // e.g. "moc.rab.oof.:" or "moc.rab.oof.:http:80" depending
   // on association with a domain (globalStorage) or

dom/tests/mochitest/sessionstorage/Makefile.in

@@ -49,9 +49,12 @@ _TEST_FILES	= \
     frameReplace.html \
     frameEqual.html \
     frameNotEqual.html \
+    file_http.html \
+    file_https.html \
     test_sessionStorageBase.html \
     test_sessionStorageClone.html \
     test_sessionStorageReplace.html \
+    test_sessionStorageHttpHttps.html \
     interOriginSlave.js \
     interOriginTest.js \
     $(NULL)

dom/tests/mochitest/sessionstorage/file_http.html

@@ -0,0 +1,40 @@
+<html>
+<head>
+<script>
+
+window.addEventListener("message", onMessageReceived, false);
+
+function postMsg(msg)
+{
+  parent.postMessage(msg, "http://localhost:8888");
+}
+
+function onMessageReceived(event)
+{
+  if (event.data == "check") {
+    postMsg(sessionStorage.getItem("foo"));
+
+    var gotValue = "threw";
+    try {
+      gotValue = sessionStorage.getItem("foo-https");
+    } catch (e) {
+    }
+
+    postMsg(gotValue);
+
+    postMsg("the end");
+  }
+}
+
+function start()
+{
+  sessionStorage.setItem("foo", "insecure");
+  postMsg(sessionStorage.getItem("foo"));
+}
+
+</script>
+</head>
+<body onload="start();">
+insecure
+</body>
+</html>

dom/tests/mochitest/sessionstorage/file_https.html

@@ -0,0 +1,15 @@
+<html>
+<head>
+<script>
+function start()
+{
+  sessionStorage.setItem("foo-https", "secure");
+  parent.postMessage(sessionStorage.getItem("foo-https"),
+                     "http://localhost:8888");
+}
+</script>
+</head>
+<body onload="start();">
+secure
+</body>
+</html>

dom/tests/mochitest/sessionstorage/test_sessionStorageFromChrome.xhtml

@@ -17,11 +17,11 @@ function startTest()
     sessionStorage;
   }
   catch (e) {
-    is(e.result, Components.results.NS_ERROR_NOT_AVAILABLE,
+    is(e.result, 2152923145,
        "Testing that we get the expected exception.");
     exceptionCaught = true;
   }
-  todo_is(exceptionCaught, true, "Testing that an exception was thrown.");
+  is(exceptionCaught, true, "Testing that an exception was thrown.");
 
   SimpleTest.finish();
 }

dom/tests/mochitest/sessionstorage/test_sessionStorageHttpHttps.html

@@ -0,0 +1,60 @@
+<html xmlns="http://www.w3.org/1999/xhtml">
+<head>
+<title>sessionStorage replace test</title>
+
+<!--
+  This test checks that sessionStorage values set in an https page
+  are not readable from a non-https page from the same domain.
+-->
+
+<script type="text/javascript" src="/MochiKit/packed.js"></script>
+<script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+<link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+
+<script type="text/javascript">
+
+window.addEventListener("message", onMessageReceived, false);
+
+var messages = [];
+
+function onMessageReceived(event)
+{
+  if (event.data == "the end") {
+    is(messages.length, 4, "Wrong number of messages.");
+    is(messages[0], "insecure", "Wrong message from insecure page");
+    is(messages[1], "secure", "Wrong message from secure page");
+    is(messages[2], "insecure", "Wrong second message from insecure page");
+    is(messages[3], null, "Insecure page got secure message?");
+
+    SimpleTest.finish();
+
+    return;
+  }
+
+  messages.push(event.data);
+
+  if (event.data == "insecure" && messages.length == 1) {
+    window.httpsframe.location = "https://test1.example.com/tests/dom/tests/mochitest/sessionstorage/file_https.html";
+  }
+
+  if (event.data == "secure") {
+    window.httpframe.postMessage("check", "http://test1.example.com");
+  }
+}
+
+function startTest()
+{
+  window.httpframe.location = "http://test1.example.com/tests/dom/tests/mochitest/sessionstorage/file_http.html";
+}
+
+SimpleTest.waitForExplicitFinish();
+
+</script>
+
+</head>
+
+<body onload="startTest();">
+  <iframe src="" name="httpframe"></iframe>
+  <iframe src="" name="httpsframe"></iframe>
+</body>
+</html>