Bug 1754004 - Part 10: Clone PostData when writing into & reading fro…

…m SessionHistoryInfo, r=smaug

Previously the SessionHistoryInfo would hold onto and hand out the
original nsIInputStream objects which were provided by the
nsDocShellLoadState and used to create the underlying channel. This
could cause issues in edge cases, as input streams when serialized over
IPC have their logical owner transferred to the IPC layer so that it can
copy the data to the peer process.

This patch changes the logic to instead clone the input stream to and
from the history info. This means that the history info has its own
instance of the stream type and interacting with it shouldn't interfere
with other consumers of the post data stream.

The behaviour for non-SHIP session history is not changed, as it doesn't
serialize the relevant streams over IPC in the same way, and is on track to be
removed.

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

docshell/base/BrowsingContext.cpp

@@ -3508,7 +3508,7 @@ bool BrowsingContext::IsPopupAllowed() {
 bool BrowsingContext::ShouldAddEntryForRefresh(
     nsIURI* aCurrentURI, const SessionHistoryInfo& aInfo) {
   return ShouldAddEntryForRefresh(aCurrentURI, aInfo.GetURI(),
-                                  aInfo.GetPostData());
+                                  aInfo.HasPostData());
 }
 
 /* static */

docshell/base/CanonicalBrowsingContext.h

@@ -475,7 +475,7 @@ class CanonicalBrowsingContext final : public BrowsingContext {
 
   bool ShouldAddEntryForRefresh(const SessionHistoryEntry* aEntry) {
     return ShouldAddEntryForRefresh(aEntry->Info().GetURI(),
-                                    aEntry->Info().GetPostData());
+                                    aEntry->Info().HasPostData());
   }
   bool ShouldAddEntryForRefresh(nsIURI* aNewURI, bool aHasPostData) {
     nsCOMPtr<nsIURI> currentURI = GetCurrentURI();

docshell/base/nsDocShell.cpp

@@ -8800,19 +8800,6 @@ bool nsDocShell::IsSameDocumentNavigation(nsDocShellLoadState* aLoadState,
     }
   }
 
-#ifdef DEBUG
-  if (aState.mHistoryNavBetweenSameDoc) {
-    nsCOMPtr<nsIInputStream> currentPostData;
-    if (mozilla::SessionHistoryInParent()) {
-      currentPostData = mActiveEntry->GetPostData();
-    } else {
-      currentPostData = mOSHE->GetPostData();
-    }
-    NS_ASSERTION(currentPostData == aLoadState->PostDataStream(),
-                 "Different POST data for entries for the same page?");
-  }
-#endif
-
   // A same document navigation happens when we navigate between two SHEntries
   // for the same document. We do a same document navigation under two
   // circumstances. Either
@@ -10041,6 +10028,10 @@ nsIPrincipal* nsDocShell::GetInheritedPrincipal(
 
       // we really need to have a content type associated with this stream!!
       postChannel->SetUploadStream(aLoadState->PostDataStream(), ""_ns, -1);
+
+      // Ownership of the stream has transferred to the channel, clear our
+      // reference.
+      aLoadState->SetPostDataStream(nullptr);
     }
 
     /* If there is a valid postdata *and* it is a History Load,

docshell/shistory/SessionHistoryEntry.cpp

@@ -9,10 +9,13 @@
 #include "nsDocShell.h"
 #include "nsDocShellLoadState.h"
 #include "nsFrameLoader.h"
+#include "nsIFormPOSTActionChannel.h"
 #include "nsIHttpChannel.h"
+#include "nsIUploadChannel2.h"
 #include "nsIXULRuntime.h"
 #include "nsSHEntryShared.h"
 #include "nsSHistory.h"
+#include "nsStreamUtils.h"
 #include "nsStructuredCloneContainer.h"
 #include "nsXULAppAPI.h"
 #include "mozilla/PresState.h"
@@ -42,7 +45,6 @@ SessionHistoryInfo::SessionHistoryInfo(nsDocShellLoadState* aLoadState,
     : mURI(aLoadState->URI()),
       mOriginalURI(aLoadState->OriginalURI()),
       mResultPrincipalURI(aLoadState->ResultPrincipalURI()),
-      mPostData(aLoadState->PostDataStream()),
       mLoadType(aLoadState->LoadType()),
       mSrcdocData(aLoadState->SrcdocData().IsVoid()
                       ? Nothing()
@@ -56,6 +58,15 @@ SessionHistoryInfo::SessionHistoryInfo(nsDocShellLoadState* aLoadState,
           aLoadState->PartitionedPrincipalToInherit(), aLoadState->Csp(),
           /* FIXME Is this correct? */
           aLoadState->TypeHint())) {
+  // Pull the upload stream off of the channel instead of the load state, as
+  // ownership has already been transferred from the load state to the channel.
+  if (nsCOMPtr<nsIUploadChannel2> postChannel = do_QueryInterface(aChannel)) {
+    int64_t contentLength;
+    MOZ_ALWAYS_SUCCEEDS(postChannel->CloneUploadStream(
+        &contentLength, getter_AddRefs(mPostData)));
+    MOZ_ASSERT_IF(mPostData, NS_InputStreamIsCloneable(mPostData));
+  }
+
   if (nsCOMPtr<nsIHttpChannel> httpChannel = do_QueryInterface(aChannel)) {
     mReferrerInfo = httpChannel->GetReferrerInfo();
   }
@@ -163,6 +174,24 @@ void SessionHistoryInfo::MaybeUpdateTitleFromURI() {
   }
 }
 
+already_AddRefed<nsIInputStream> SessionHistoryInfo::GetPostData() const {
+  // Return a clone of our post data stream. Our caller will either be
+  // transferring this stream to a different SessionHistoryInfo, or passing it
+  // off to necko/another process which will consume it, and we want to preserve
+  // our local instance.
+  nsCOMPtr<nsIInputStream> postData;
+  if (mPostData) {
+    MOZ_ALWAYS_SUCCEEDS(
+        NS_CloneInputStream(mPostData, getter_AddRefs(postData)));
+  }
+  return postData.forget();
+}
+
+void SessionHistoryInfo::SetPostData(nsIInputStream* aPostData) {
+  MOZ_ASSERT_IF(aPostData, NS_InputStreamIsCloneable(aPostData));
+  mPostData = aPostData;
+}
+
 uint64_t SessionHistoryInfo::SharedId() const {
   return mSharedState.Get()->mId;
 }
@@ -217,7 +246,8 @@ void SessionHistoryInfo::FillLoadInfo(nsDocShellLoadState& aLoadState) const {
   aLoadState.SetOriginalURI(mOriginalURI);
   aLoadState.SetMaybeResultPrincipalURI(Some(mResultPrincipalURI));
   aLoadState.SetLoadReplace(mLoadReplace);
-  aLoadState.SetPostDataStream(mPostData);
+  nsCOMPtr<nsIInputStream> postData = GetPostData();
+  aLoadState.SetPostDataStream(postData);
   aLoadState.SetReferrerInfo(mReferrerInfo);
 
   aLoadState.SetTypeHint(mSharedState.Get()->mContentType);
@@ -653,14 +683,13 @@ SessionHistoryEntry::SetRefreshURIList(nsIMutableArray* aRefreshURIList) {
 
 NS_IMETHODIMP
 SessionHistoryEntry::GetPostData(nsIInputStream** aPostData) {
-  nsCOMPtr<nsIInputStream> postData = mInfo->mPostData;
-  postData.forget(aPostData);
+  *aPostData = mInfo->GetPostData().take();
   return NS_OK;
 }
 
 NS_IMETHODIMP
 SessionHistoryEntry::SetPostData(nsIInputStream* aPostData) {
-  mInfo->mPostData = aPostData;
+  mInfo->SetPostData(aPostData);
   return NS_OK;
 }
 
@@ -1467,6 +1496,8 @@ namespace ipc {
 void IPDLParamTraits<dom::SessionHistoryInfo>::Write(
     IPC::MessageWriter* aWriter, IProtocol* aActor,
     const dom::SessionHistoryInfo& aParam) {
+  nsCOMPtr<nsIInputStream> postData = aParam.GetPostData();
+
   Maybe<Tuple<uint32_t, dom::ClonedMessageData>> stateData;
   if (aParam.mStateData) {
     stateData.emplace();
@@ -1497,7 +1528,7 @@ void IPDLParamTraits<dom::SessionHistoryInfo>::Write(
   WriteIPDLParam(aWriter, aActor, aParam.mReferrerInfo);
   WriteIPDLParam(aWriter, aActor, aParam.mTitle);
   WriteIPDLParam(aWriter, aActor, aParam.mName);
-  WriteIPDLParam(aWriter, aActor, aParam.mPostData);
+  WriteIPDLParam(aWriter, aActor, postData);
   WriteIPDLParam(aWriter, aActor, aParam.mLoadType);
   WriteIPDLParam(aWriter, aActor, aParam.mScrollPositionX);
   WriteIPDLParam(aWriter, aActor, aParam.mScrollPositionY);
@@ -1570,6 +1601,17 @@ bool IPDLParamTraits<dom::SessionHistoryInfo>::Read(
     return false;
   }
 
+  // We should always see a cloneable input stream passed to SessionHistoryInfo.
+  // This is because it will be cloneable when first read in the parent process
+  // from the nsHttpChannel (which forces streams to be cloneable), and future
+  // streams in content will be wrapped in
+  // nsMIMEInputStream(RemoteLazyInputStream) which is also cloneable.
+  if (aResult->mPostData && !NS_InputStreamIsCloneable(aResult->mPostData)) {
+    aActor->FatalError(
+        "Unexpected non-cloneable postData for SessionHistoryInfo");
+    return false;
+  }
+
   dom::SHEntrySharedParentState* sharedState = nullptr;
   if (XRE_IsParentProcess()) {
     sharedState = dom::SHEntrySharedParentState::Lookup(sharedId);

docshell/shistory/SessionHistoryEntry.h

@@ -71,8 +71,9 @@ class SessionHistoryInfo {
     mResultPrincipalURI = aResultPrincipalURI;
   }
 
-  nsIInputStream* GetPostData() const { return mPostData; }
-  void SetPostData(nsIInputStream* aPostData) { mPostData = aPostData; }
+  bool HasPostData() const { return mPostData; }
+  already_AddRefed<nsIInputStream> GetPostData() const;
+  void SetPostData(nsIInputStream* aPostData);
 
   void GetScrollPosition(int32_t* aScrollPositionX, int32_t* aScrollPositionY) {
     *aScrollPositionX = mScrollPositionX;