Fixing bug 495112. Partially revert the fix for bug 455070 to fix session storage regressions. r+sr=jst@mozilla.org
authorhonzab.moz@firemni.cz
Thu, 28 May 2009 16:15:26 -0700
changeset 28820 363750f510ec5a5e3975391591f1c60f939f1a10
parent 28819 8bbbfa16fe349d87077506715223f7bbb0a23b5c
child 28821 6b5508397a29a68a8563ad5c514dbbd122420a1b
push idunknown
push userunknown
push dateunknown
bugs495112, 455070
milestone1.9.2a1pre
Fixing bug 495112. Partially revert the fix for bug 455070 to fix session storage regressions. r+sr=jst@mozilla.org
docshell/base/nsDocShell.cpp
dom/src/storage/nsDOMStorage.cpp
dom/src/storage/nsDOMStorage.h
dom/tests/mochitest/sessionstorage/Makefile.in
dom/tests/mochitest/sessionstorage/file_http.html
dom/tests/mochitest/sessionstorage/file_https.html
dom/tests/mochitest/sessionstorage/test_sessionStorageFromChrome.xhtml
dom/tests/mochitest/sessionstorage/test_sessionStorageHttpHttps.html
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -2135,16 +2135,44 @@ nsDocShell::HistoryPurged(PRInt32 aNumEn
         if (shell) {
             shell->HistoryPurged(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,
                                           nsIDOMStorage** aStorage)
 {
     NS_ENSURE_ARG_POINTER(aStorage);
     *aStorage = nsnull;
 
@@ -2161,36 +2189,38 @@ nsDocShell::GetSessionStorageForPrincipa
     if (!topItem)
         return NS_ERROR_FAILURE;
 
     nsDocShell* topDocShell = static_cast<nsDocShell*>(topItem.get());
     if (topDocShell != this)
         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 (!mStorages.Get(origin, aStorage) && aCreate) {
+    if (currentDomain.IsEmpty())
+        return NS_OK;
+
+    if (!mStorages.Get(currentDomain, aStorage) && aCreate) {
         nsCOMPtr<nsIDOMStorage> newstorage =
             do_CreateInstance("@mozilla.org/dom/storage;2");
         if (!newstorage)
             return NS_ERROR_OUT_OF_MEMORY;
 
         nsCOMPtr<nsPIDOMStorage> pistorage = do_QueryInterface(newstorage);
         if (!pistorage)
             return NS_ERROR_FAILURE;
-        pistorage->InitAsSessionStorage(aPrincipal);
-
-        if (!mStorages.Put(origin, newstorage))
+        rv = pistorage->InitAsSessionStorage(aPrincipal);
+        if (NS_FAILED(rv))
+            return rv;
+
+        if (!mStorages.Put(currentDomain, newstorage))
             return NS_ERROR_OUT_OF_MEMORY;
 
         newstorage.swap(*aStorage);
         return NS_OK;
     }
 
     nsCOMPtr<nsPIDOMStorage> piStorage = do_QueryInterface(*aStorage);
     if (piStorage) {
@@ -2251,29 +2281,29 @@ nsDocShell::AddSessionStorage(nsIPrincip
     nsCOMPtr<nsIDocShellTreeItem> topItem;
     nsresult rv = GetSameTypeRootTreeItem(getter_AddRefs(topItem));
     if (NS_FAILED(rv))
         return rv;
 
     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 {
             return topDocShell->AddSessionStorage(aPrincipal, aStorage);
         }
     }
 
     return NS_OK;
--- a/dom/src/storage/nsDOMStorage.cpp
+++ b/dom/src/storage/nsDOMStorage.cpp
@@ -530,65 +530,60 @@ NS_NewDOMStorage2(nsISupports* aOuter, R
 }
 
 nsDOMStorage::nsDOMStorage()
   : mUseDB(PR_FALSE)
   , mSessionOnly(PR_TRUE)
   , 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)
   , mLocalStorage(PR_FALSE) // Any clone is not a localStorage
   , mItemsCached(PR_FALSE)
   , mDomain(aThat.mDomain)
 #ifdef MOZ_STORAGE
   , mScopeDBKey(aThat.mScopeDBKey)
 #endif
 {
+  mSecurityChecker = this;
   mItems.Init(8);
-  aThat.mItems.EnumerateEntries(CopyStorageItems, this);
 
   if (nsDOMStorageManager::gStorageManager)
     nsDOMStorageManager::gStorageManager->AddToStoragesHash(this);
 }
 
 nsDOMStorage::~nsDOMStorage()
 {
   if (nsDOMStorageManager::gStorageManager)
     nsDOMStorageManager::gStorageManager->RemoveFromStoragesHash(this);
 }
 
 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.
   if (!uri)
     return NS_ERROR_NOT_AVAILABLE;
 
   nsCOMPtr<nsIURI> innerURI = NS_GetInnermostURI(uri);
   if (!innerURI)
@@ -597,17 +592,17 @@ GetDomainURI(nsIPrincipal *aPrincipal, n
 
   return NS_OK;
 }
 
 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
   // here as we call GetPrincipalURIAndHost (nsDOMStorage.cpp:88) from
   // nsDOMStorage::CanUseStorage before we query the storage manager for a new
   // sessionStorage. It calls GetAsciiHost on innermost URI. If it fails, we
   // won't get to InitAsSessionStorage.
   domainURI->GetAsciiHost(mDomain);
@@ -619,17 +614,17 @@ nsDOMStorage::InitAsSessionStorage(nsIPr
 #endif
   return NS_OK;
 }
 
 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
   // here as we call GetPrincipalURIAndHost (nsDOMStorage.cpp:88) from
   // nsDOMStorage::CanUseStorage before we query the storage manager for a new
   // localStorage. It calls GetAsciiHost on innermost URI. If it fails, we won't
   // get to InitAsLocalStorage. Actually, mDomain will get replaced with
   // mPrincipal in bug 455070. It is not even used for localStorage.
@@ -665,16 +660,37 @@ nsDOMStorage::InitAsGlobalStorage(const 
   if (!(mUseDB = !mScopeDBKey.IsEmpty()))
     mScopeDBKey.AppendLiteral(":");
 
   nsDOMStorageDBWrapper::CreateQuotaDomainDBKey(aDomainDemanded, PR_TRUE, mQuotaDomainDBKey);
 #endif
   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)
 {
   // check if the calling domain can use storage. Downgrade to session
   // only if only session storage may be used.
   NS_ASSERTION(aSessionOnly, "null session flag");
   *aSessionOnly = PR_FALSE;
@@ -745,17 +761,18 @@ nsDOMStorage::CacheStoragePermissions()
 
   nsIScriptSecurityManager* ssm = nsContentUtils::GetSecurityManager();
   if (!ssm)
     return PR_FALSE;
 
   nsCOMPtr<nsIPrincipal> subjectPrincipal;
   ssm->GetSubjectPrincipal(getter_AddRefs(subjectPrincipal));
 
-  return CanAccess(subjectPrincipal);
+  NS_ASSERTION(mSecurityChecker, "Has non-null mSecurityChecker");
+  return mSecurityChecker->CanAccess(subjectPrincipal);
 }
 
 
 class ItemCounterState
 {
 public:
   ItemCounterState(PRBool aIsCallerSecure)
     : mIsCallerSecure(aIsCallerSecure), mCount(0)
@@ -1399,37 +1416,41 @@ NS_INTERFACE_MAP_END
 
 nsDOMStorage2::nsDOMStorage2()
 {
 }
 
 nsDOMStorage2::nsDOMStorage2(nsDOMStorage2& aThat)
 {
   mStorage = new nsDOMStorage(*aThat.mStorage.get());
+  mStorage->mSecurityChecker = mStorage;
   mPrincipal = aThat.mPrincipal;
 }
 
 nsresult
 nsDOMStorage2::InitAsSessionStorage(nsIPrincipal *aPrincipal)
 {
   mStorage = new nsDOMStorage();
   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);
 }
 
 nsresult
 nsDOMStorage2::InitAsLocalStorage(nsIPrincipal *aPrincipal)
 {
   mStorage = new nsDOMStorage();
   if (!mStorage)
     return NS_ERROR_OUT_OF_MEMORY;
 
+  mStorage->mSecurityChecker = this;
   mPrincipal = aPrincipal;
   return mStorage->InitAsLocalStorage(aPrincipal);
 }
 
 nsresult
 nsDOMStorage2::InitAsGlobalStorage(const nsACString &aDomainDemanded)
 {
   NS_ASSERTION(PR_FALSE, "Should not initialize nsDOMStorage2 as global storage.");
@@ -1438,16 +1459,17 @@ nsDOMStorage2::InitAsGlobalStorage(const
 
 already_AddRefed<nsIDOMStorage>
 nsDOMStorage2::Clone()
 {
   nsDOMStorage2* storage = new nsDOMStorage2(*this);
   if (!storage)
     return nsnull;
 
+  storage->mStorage->CloneFrom(mStorage);
   NS_ADDREF(storage);
 
   return storage;
 }
 
 nsTArray<nsString> *
 nsDOMStorage2::GetKeys()
 {
@@ -1458,16 +1480,19 @@ nsIPrincipal*
 nsDOMStorage2::Principal()
 {
   return mPrincipal;
 }
 
 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;
 
   // Allow more powerful principals (e.g. system) to access the storage
   PRBool subsumes;
   nsresult rv = aPrincipal->Subsumes(mPrincipal, &subsumes);
   if (NS_FAILED(rv))
--- a/dom/src/storage/nsDOMStorage.h
+++ b/dom/src/storage/nsDOMStorage.h
@@ -188,16 +188,19 @@ public:
 
   // set the value corresponding to a key as secure.
   nsresult
   SetSecure(const nsAString& aKey, PRBool aSecure);
 
   // clear all values from the store
   void ClearAll();
 
+  nsresult
+  CloneFrom(nsDOMStorage* aThat);
+
   nsIDOMStorageItem* GetNamedItem(const nsAString& aKey, nsresult* aResult);
 
   static nsDOMStorage* FromSupports(nsISupports* aSupports)
   {
     return static_cast<nsDOMStorage*>(static_cast<nsIDOMStorageObsolete*>(aSupports));
   }
 
 protected:
@@ -239,16 +242,19 @@ protected:
   // the key->value item pairs
   nsTHashtable<nsSessionStorageEntry> mItems;
 
   // keys are used for database queries.
   // see comments of the getters bellow.
   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
   // an origin (localStorage).
   nsCString& GetScopeDBKey() {return mScopeDBKey;}
 
   // e.g. "moc.rab.%" - reversed eTLD+1 subpart of the domain or
   // (in future) reversed offline application allowed domain.
--- a/dom/tests/mochitest/sessionstorage/Makefile.in
+++ b/dom/tests/mochitest/sessionstorage/Makefile.in
@@ -44,19 +44,22 @@ relativesrcdir	= dom/tests/mochitest/ses
 include $(DEPTH)/config/autoconf.mk
 
 include $(topsrcdir)/config/rules.mk
 
 _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)
 
 _CHROME_FILES = \
     test_sessionStorageFromChrome.xhtml \
     $(NULL)
 
new file mode 100644
--- /dev/null
+++ b/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>
new file mode 100644
--- /dev/null
+++ b/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>
--- a/dom/tests/mochitest/sessionstorage/test_sessionStorageFromChrome.xhtml
+++ b/dom/tests/mochitest/sessionstorage/test_sessionStorageFromChrome.xhtml
@@ -12,21 +12,21 @@ function startTest()
 {
   // Check that we do not crash when we access the sessionStorage object from
   // chrome and that we throw.  See bug 404453.
   var exceptionCaught = false;
   try {
     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();
 }
 
 </script>
 
 </head>
 
new file mode 100644
--- /dev/null
+++ b/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>