Bug 1668083 - when redirecting load from post to get, load should become replacing load also in session history, r=peterv

Differential Revision: https://phabricator.services.mozilla.com/D93249

docshell/base/CanonicalBrowsingContext.cpp

@@ -389,6 +389,40 @@ CanonicalBrowsingContext::CreateLoadingSessionHistoryEntryForLoad(
   return loadingInfo;
 }
 
+UniquePtr<LoadingSessionHistoryInfo>
+CanonicalBrowsingContext::ReplaceLoadingSessionHistoryEntryForLoad(
+    LoadingSessionHistoryInfo* aInfo, nsIChannel* aChannel) {
+  MOZ_ASSERT(aInfo);
+  MOZ_ASSERT(aChannel);
+
+  UniquePtr<SessionHistoryInfo> newInfo = MakeUnique<SessionHistoryInfo>(
+      aChannel, aInfo->mInfo.LoadType(),
+      aInfo->mInfo.GetPartitionedPrincipalToInherit(), aInfo->mInfo.GetCsp());
+
+  RefPtr<SessionHistoryEntry> newEntry = new SessionHistoryEntry(newInfo.get());
+  if (IsTop()) {
+    // Only top level pages care about Get/SetPersist.
+    nsCOMPtr<nsIURI> uri;
+    aChannel->GetURI(getter_AddRefs(uri));
+    newEntry->SetPersist(nsDocShell::ShouldAddToSessionHistory(uri, aChannel));
+  }
+  newEntry->SetDocshellID(GetHistoryID());
+  newEntry->SetIsDynamicallyAdded(CreatedDynamically());
+  newEntry->SetForInitialLoad(true);
+
+  // Replacing the old entry.
+  SessionHistoryEntry::SetByLoadId(aInfo->mLoadId, newEntry);
+
+  for (size_t i = 0; i < mLoadingEntries.Length(); ++i) {
+    if (mLoadingEntries[i].mLoadId == aInfo->mLoadId) {
+      mLoadingEntries[i].mEntry = newEntry;
+      break;
+    }
+  }
+
+  return MakeUnique<LoadingSessionHistoryInfo>(newEntry, aInfo->mLoadId);
+}
+
 void CanonicalBrowsingContext::SessionHistoryCommit(uint64_t aLoadId,
                                                     const nsID& aChangeID,
                                                     uint32_t aLoadType) {

docshell/base/CanonicalBrowsingContext.h

@@ -108,6 +108,10 @@ class CanonicalBrowsingContext final : public BrowsingContext {
 
   UniquePtr<LoadingSessionHistoryInfo> CreateLoadingSessionHistoryEntryForLoad(
       nsDocShellLoadState* aLoadState, nsIChannel* aChannel);
+
+  UniquePtr<LoadingSessionHistoryInfo> ReplaceLoadingSessionHistoryEntryForLoad(
+      LoadingSessionHistoryInfo* aInfo, nsIChannel* aChannel);
+
   void SessionHistoryCommit(uint64_t aLoadId, const nsID& aChangeID,
                             uint32_t aLoadType);
 

docshell/base/nsDocShell.cpp

@@ -13273,7 +13273,12 @@ void nsDocShell::MoveLoadingToActiveEntry() {
       RefPtr<ChildSHistory> rootSH = GetRootSessionHistory();
       if (rootSH) {
         if (!loadingEntry->mLoadIsFromSessionHistory) {
-          changeID = rootSH->AddPendingHistoryChange();
+          // It is possible that this leads to wrong length temporarily, but
+          // so would not having the check for replace.
+          if (!LOAD_TYPE_HAS_FLAGS(
+                  mLoadType, nsIWebNavigation::LOAD_FLAGS_REPLACE_HISTORY)) {
+            changeID = rootSH->AddPendingHistoryChange();
+          }
         } else {
           // This is a load from session history, so we can update
           // index and length immediately.

docshell/shistory/SessionHistoryEntry.cpp

@@ -38,11 +38,6 @@ SessionHistoryInfo::SessionHistoryInfo(nsDocShellLoadState* aLoadState,
           /* FIXME Is this correct? */
           aLoadState->TypeHint())) {
   MaybeUpdateTitleFromURI();
-  bool isNoStore = false;
-  if (nsCOMPtr<nsIHttpChannel> httpChannel = do_QueryInterface(aChannel)) {
-    Unused << httpChannel->IsNoStoreResponse(&isNoStore);
-    mPersist = !isNoStore;
-  }
 }
 
 SessionHistoryInfo::SessionHistoryInfo(
@@ -70,6 +65,39 @@ SessionHistoryInfo::SessionHistoryInfo(
   MaybeUpdateTitleFromURI();
 }
 
+SessionHistoryInfo::SessionHistoryInfo(
+    nsIChannel* aChannel, uint32_t aLoadType,
+    nsIPrincipal* aPartitionedPrincipalToInherit,
+    nsIContentSecurityPolicy* aCsp) {
+  aChannel->GetURI(getter_AddRefs(mURI));
+  mLoadType = aLoadType;
+
+  nsCOMPtr<nsILoadInfo> loadInfo;
+  aChannel->GetLoadInfo(getter_AddRefs(loadInfo));
+
+  loadInfo->GetResultPrincipalURI(getter_AddRefs(mResultPrincipalURI));
+  loadInfo->GetTriggeringPrincipal(
+      getter_AddRefs(mSharedState.Get()->mTriggeringPrincipal));
+  loadInfo->GetPrincipalToInherit(
+      getter_AddRefs(mSharedState.Get()->mPrincipalToInherit));
+
+  mSharedState.Get()->mPartitionedPrincipalToInherit =
+      aPartitionedPrincipalToInherit;
+  mSharedState.Get()->mCsp = aCsp;
+  aChannel->GetContentType(mSharedState.Get()->mContentType);
+  aChannel->GetOriginalURI(getter_AddRefs(mOriginalURI));
+
+  uint32_t loadFlags;
+  aChannel->GetLoadFlags(&loadFlags);
+  mLoadReplace = !!(loadFlags & nsIChannel::LOAD_REPLACE);
+
+  MaybeUpdateTitleFromURI();
+
+  if (nsCOMPtr<nsIHttpChannel> httpChannel = do_QueryInterface(aChannel)) {
+    mReferrerInfo = httpChannel->GetReferrerInfo();
+  }
+}
+
 void SessionHistoryInfo::Reset(nsIURI* aURI, const nsID& aDocShellID,
                                bool aDynamicCreation,
                                nsIPrincipal* aTriggeringPrincipal,
@@ -291,13 +319,7 @@ nsDataHashtable<nsUint64HashKey, SessionHistoryEntry*>*
 LoadingSessionHistoryInfo::LoadingSessionHistoryInfo(
     SessionHistoryEntry* aEntry)
     : mInfo(aEntry->Info()), mLoadId(++gLoadingSessionHistoryInfoLoadId) {
-  if (!SessionHistoryEntry::sLoadIdToEntry) {
+  SessionHistoryEntry::SetByLoadId(mLoadId, aEntry);
-    SessionHistoryEntry::sLoadIdToEntry =
-        new nsDataHashtable<nsUint64HashKey, SessionHistoryEntry*>();
-  }
-  MOZ_LOG(gSHLog, LogLevel::Verbose,
-          ("SHEntry::AddLoadId(%" PRIu64 " - %p", mLoadId, aEntry));
-  SessionHistoryEntry::sLoadIdToEntry->Put(mLoadId, aEntry);
 }
 
 LoadingSessionHistoryInfo::LoadingSessionHistoryInfo(
@@ -330,6 +352,19 @@ SessionHistoryEntry* SessionHistoryEntry::GetByLoadId(uint64_t aLoadId) {
   return sLoadIdToEntry->Get(aLoadId);
 }
 
+void SessionHistoryEntry::SetByLoadId(uint64_t aLoadId,
+                                      SessionHistoryEntry* aEntry) {
+  if (!sLoadIdToEntry) {
+    sLoadIdToEntry =
+        new nsDataHashtable<nsUint64HashKey, SessionHistoryEntry*>();
+  }
+
+  MOZ_LOG(
+      gSHLog, LogLevel::Verbose,
+      ("SessionHistoryEntry::SetByLoadId(%" PRIu64 " - %p", aLoadId, aEntry));
+  sLoadIdToEntry->Put(aLoadId, aEntry);
+}
+
 void SessionHistoryEntry::RemoveLoadId(uint64_t aLoadId) {
   MOZ_ASSERT(XRE_IsParentProcess());
   if (!sLoadIdToEntry) {

docshell/shistory/SessionHistoryEntry.h

@@ -43,6 +43,9 @@ class SessionHistoryInfo {
                      nsIPrincipal* aPartitionedPrincipalToInherit,
                      nsIContentSecurityPolicy* aCsp,
                      const nsACString& aContentType);
+  SessionHistoryInfo(nsIChannel* aChannel, uint32_t aLoadType,
+                     nsIPrincipal* aPartitionedPrincipalToInherit,
+                     nsIContentSecurityPolicy* aCsp);
 
   void Reset(nsIURI* aURI, const nsID& aDocShellID, bool aDynamicCreation,
              nsIPrincipal* aTriggeringPrincipal,
@@ -122,6 +125,8 @@ class SessionHistoryInfo {
 
   void FillLoadInfo(nsDocShellLoadState& aLoadState) const;
 
+  uint32_t LoadType() { return mLoadType; }
+
  private:
   friend class SessionHistoryEntry;
   friend struct mozilla::ipc::IPDLParamTraits<SessionHistoryInfo>;
@@ -262,6 +267,7 @@ class SessionHistoryEntry : public nsISHEntry {
   // Get an entry based on LoadingSessionHistoryInfo's mLoadId. Parent process
   // only.
   static SessionHistoryEntry* GetByLoadId(uint64_t aLoadId);
+  static void SetByLoadId(uint64_t aLoadId, SessionHistoryEntry* aEntry);
   static void RemoveLoadId(uint64_t aLoadId);
 
   const nsTArray<RefPtr<SessionHistoryEntry>>& Children() { return mChildren; }

docshell/test/mochitest/file_redirect_history.html

@@ -0,0 +1,18 @@
+<html>
+  <head>
+    <script>
+      function loaded() {
+        addEventListener("message", ({ data }) => {
+          document.getElementById("form").action = data;
+          document.getElementById("button").click();
+        }, { once: true });
+        opener.postMessage("loaded", "*");
+      }
+    </script>
+  </head>
+  <body onload="loaded();">
+    <form id="form" method="POST">
+      <input id="button" type="submit" />
+    </form>
+  </body>
+</html>

docshell/test/mochitest/form_submit_redirect.sjs

@@ -0,0 +1,10 @@
+"use strict";
+
+async function handleRequest(request, response) {
+  if (request.method !== "POST") {
+    message = "bad";
+  } else {
+    response.setStatusLine(request.httpVersion, 302, "Moved Temporarily");
+    response.setHeader("Location", request.getHeader("referer"));
+  }
+}

docshell/test/mochitest/mochitest.ini

@@ -127,6 +127,10 @@ support-files =
   file_history_length_during_pageload_2.html
 [test_pushState_after_document_open.html]
 [test_navigate_after_pagehide.html]
+[test_redirect_history.html]
+support-files =
+  file_redirect_history.html
+  form_submit_redirect.sjs
 [test_windowedhistoryframes.html]
 skip-if = (!debug && os == 'android')
 [test_triggeringprincipal_location_seturi.html]

docshell/test/mochitest/test_redirect_history.html

@@ -0,0 +1,57 @@
+<!doctype html>
+<html>
+  <head>
+    <title>Test for redirect from POST</title>
+    <script src="/tests/SimpleTest/SimpleTest.js"></script>
+    <script src="/tests/SimpleTest/EventUtils.js"></script>
+    <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+  </head>
+  <body>
+    <script>
+      "use strict";
+
+      info("Starting tests");
+
+      let tests = new Map([
+        ["sameorigin", window.location.origin],
+        ["crossorigin", "http://test1.example.com"],
+      ]);
+      for (let [kind, origin] of tests) {
+        add_task(async function runTest() {
+          info(`Submitting to ${origin}`);
+
+          let win;
+          await new Promise(resolve => {
+            addEventListener("message", resolve, { once: true });
+            info("Loading file_redirect_history.html");
+            win = window.open("file_redirect_history.html");
+          });
+          info("Done loading file_redirect_history.html");
+
+          let length = win.history.length;
+          let loc = win.location.toString();
+
+          await new Promise(resolve => {
+            addEventListener("message", resolve, { once: true });
+            info("Posting");
+            win.postMessage(`${origin}/tests/docshell/test/mochitest/form_submit_redirect.sjs`, "*")
+          });
+          info("Done posting\n");
+          is(win.history.length, length, `Test ${kind}: history length should not change.`);
+          info(`Length=${win.history.length}`);
+          is(win.location.toString(), loc, `Test ${kind}: location should not change.`);
+
+          await new Promise(resolve => {
+            addEventListener("message", resolve, { once: true });
+            info("Reloading");
+            win.location.reload();
+          });
+          info("Done reloading\n");
+          is(win.location.toString(), loc, `Test ${kind}: location should not change after reload.`);
+
+          win.close();
+        });
+      }
+    </script>
+  </body>
+</html>

netwerk/ipc/DocumentLoadListener.cpp

@@ -2394,12 +2394,20 @@ DocumentLoadListener::AsyncOnChannelRedirect(
     return NS_OK;
   }
 
-  if (GetDocumentBrowsingContext() && !net::ChannelIsPost(aOldChannel)) {
+  if (GetDocumentBrowsingContext()) {
-    AddURIVisit(aOldChannel, 0);
+    if (mLoadingSessionHistoryInfo) {
+      mLoadingSessionHistoryInfo =
+          GetDocumentBrowsingContext()
+              ->ReplaceLoadingSessionHistoryEntryForLoad(
+                  mLoadingSessionHistoryInfo.get(), aNewChannel);
+    }
+    if (!net::ChannelIsPost(aOldChannel)) {
+      AddURIVisit(aOldChannel, 0);
 
-    nsCOMPtr<nsIURI> oldURI;
+      nsCOMPtr<nsIURI> oldURI;
-    aOldChannel->GetURI(getter_AddRefs(oldURI));
+      aOldChannel->GetURI(getter_AddRefs(oldURI));
-    nsDocShell::SaveLastVisit(aNewChannel, oldURI, aFlags);
+      nsDocShell::SaveLastVisit(aNewChannel, oldURI, aFlags);
+    }
   }
   mHaveVisibleRedirect |= true;