From 3303fb66bd7a36e1e4873e19bf5d96c5e1fe1e3f Mon Sep 17 00:00:00 2001 From: Peter Van der Beken Date: Tue, 14 Dec 2021 10:13:58 +0000 Subject: [PATCH] Bug 1742865 - Handle meta refresh correctly with session history in the parent. r=smaug Differential Revision: https://phabricator.services.mozilla.com/D132837 --- docshell/base/BrowsingContext.cpp | 26 ++++- docshell/base/BrowsingContext.h | 12 +- docshell/base/CanonicalBrowsingContext.cpp | 16 ++- docshell/base/CanonicalBrowsingContext.h | 6 + docshell/base/nsDocShell.cpp | 14 ++- docshell/test/mochitest/file_bug1742865.sjs | 65 +++++++++++ .../test/mochitest/file_bug1742865_outer.sjs | 25 +++++ docshell/test/mochitest/mochitest.ini | 4 + docshell/test/mochitest/test_bug1742865.html | 103 ++++++++++++++++++ 9 files changed, 258 insertions(+), 13 deletions(-) create mode 100644 docshell/test/mochitest/file_bug1742865.sjs create mode 100644 docshell/test/mochitest/file_bug1742865_outer.sjs create mode 100644 docshell/test/mochitest/test_bug1742865.html diff --git a/docshell/base/BrowsingContext.cpp b/docshell/base/BrowsingContext.cpp index d35bc39359d3..1cebebb4ada6 100644 --- a/docshell/base/BrowsingContext.cpp +++ b/docshell/base/BrowsingContext.cpp @@ -3482,10 +3482,24 @@ bool BrowsingContext::IsPopupAllowed() { return false; } +/* static */ +bool BrowsingContext::ShouldAddEntryForRefresh( + nsIURI* aCurrentURI, const SessionHistoryInfo& aInfo) { + if (aInfo.GetPostData()) { + return true; + } + + bool equalsURI = false; + if (aCurrentURI) { + aCurrentURI->Equals(aInfo.GetURI(), &equalsURI); + } + return !equalsURI; +} + void BrowsingContext::SessionHistoryCommit( const LoadingSessionHistoryInfo& aInfo, uint32_t aLoadType, - bool aHadActiveEntry, bool aPersist, bool aCloneEntryChildren, - bool aChannelExpired) { + nsIURI* aCurrentURI, bool aHadActiveEntry, bool aPersist, + bool aCloneEntryChildren, bool aChannelExpired) { nsID changeID = {}; if (XRE_IsContentProcess()) { RefPtr rootSH = Top()->GetChildSessionHistory(); @@ -3495,13 +3509,17 @@ void BrowsingContext::SessionHistoryCommit( // CanonicalBrowsingContext::SessionHistoryCommit. We'll be // incrementing the session history length if we're not replacing, // this is a top-level load or it's not the initial load in an iframe, - // and ShouldUpdateSessionHistory(loadType) returns true. + // ShouldUpdateSessionHistory(loadType) returns true and it's not a + // refresh for which ShouldAddEntryForRefresh returns false. // It is possible that this leads to wrong length temporarily, but // so would not having the check for replace. if (!LOAD_TYPE_HAS_FLAGS( aLoadType, nsIWebNavigation::LOAD_FLAGS_REPLACE_HISTORY) && (IsTop() || aHadActiveEntry) && - ShouldUpdateSessionHistory(aLoadType)) { + ShouldUpdateSessionHistory(aLoadType) && + (!LOAD_TYPE_HAS_FLAGS(aLoadType, + nsIWebNavigation::LOAD_FLAGS_IS_REFRESH) || + ShouldAddEntryForRefresh(aCurrentURI, aInfo.mInfo))) { changeID = rootSH->AddPendingHistoryChange(); } } else { diff --git a/docshell/base/BrowsingContext.h b/docshell/base/BrowsingContext.h index eeb742c95e51..7d11fb7edf6a 100644 --- a/docshell/base/BrowsingContext.h +++ b/docshell/base/BrowsingContext.h @@ -791,10 +791,13 @@ class BrowsingContext : public nsILoadContext, public nsWrapperCache { // context or any of its ancestors. bool IsPopupAllowed(); + // aCurrentURI is only required to be non-null if the load type contains the + // nsIWebNavigation::LOAD_FLAGS_IS_REFRESH flag and aInfo is for a refresh to + // the current URI. void SessionHistoryCommit(const LoadingSessionHistoryInfo& aInfo, - uint32_t aLoadType, bool aHadActiveEntry, - bool aPersist, bool aCloneEntryChildren, - bool aChannelExpired); + uint32_t aLoadType, nsIURI* aCurrentURI, + bool aHadActiveEntry, bool aPersist, + bool aCloneEntryChildren, bool aChannelExpired); // Set a new active entry on this browsing context. This is used for // implementing history.pushState/replaceState and same document navigations. @@ -898,6 +901,9 @@ class BrowsingContext : public nsILoadContext, public nsWrapperCache { return mChildSessionHistory.forget(); } + static bool ShouldAddEntryForRefresh(nsIURI* aCurrentURI, + const SessionHistoryInfo& aInfo); + private: void Attach(bool aFromIPC, ContentParent* aOriginProcess); diff --git a/docshell/base/CanonicalBrowsingContext.cpp b/docshell/base/CanonicalBrowsingContext.cpp index 12736d22c833..d995cc9afbe9 100644 --- a/docshell/base/CanonicalBrowsingContext.cpp +++ b/docshell/base/CanonicalBrowsingContext.cpp @@ -797,7 +797,6 @@ void CanonicalBrowsingContext::SessionHistoryCommit( RemoveDynEntriesFromActiveSessionHistoryEntry(); } } - mActiveEntry = newActiveEntry; if (LOAD_TYPE_HAS_FLAGS(aLoadType, nsIWebNavigation::LOAD_FLAGS_REPLACE_HISTORY)) { @@ -808,8 +807,16 @@ void CanonicalBrowsingContext::SessionHistoryCommit( // should append instead. addEntry = index < 0; if (!addEntry) { - shistory->ReplaceEntry(index, mActiveEntry); + shistory->ReplaceEntry(index, newActiveEntry); } + mActiveEntry = newActiveEntry; + } else if (LOAD_TYPE_HAS_FLAGS( + aLoadType, nsIWebNavigation::LOAD_FLAGS_IS_REFRESH) && + !ShouldAddEntryForRefresh(newActiveEntry)) { + addEntry = false; + mActiveEntry->ReplaceWith(*newActiveEntry); + } else { + mActiveEntry = newActiveEntry; } if (loadFromSessionHistory) { @@ -841,7 +848,10 @@ void CanonicalBrowsingContext::SessionHistoryCommit( } else if (addEntry) { if (mActiveEntry) { if (LOAD_TYPE_HAS_FLAGS( - aLoadType, nsIWebNavigation::LOAD_FLAGS_REPLACE_HISTORY)) { + aLoadType, nsIWebNavigation::LOAD_FLAGS_REPLACE_HISTORY) || + (LOAD_TYPE_HAS_FLAGS(aLoadType, + nsIWebNavigation::LOAD_FLAGS_IS_REFRESH) && + !ShouldAddEntryForRefresh(newActiveEntry))) { // FIXME We need to make sure that when we create the info we // make a copy of the shared state. mActiveEntry->ReplaceWith(*newActiveEntry); diff --git a/docshell/base/CanonicalBrowsingContext.h b/docshell/base/CanonicalBrowsingContext.h index 949d4d4062da..18b8f5bfe329 100644 --- a/docshell/base/CanonicalBrowsingContext.h +++ b/docshell/base/CanonicalBrowsingContext.h @@ -459,6 +459,12 @@ class CanonicalBrowsingContext final : public BrowsingContext { void RemovePendingDiscard(); + bool ShouldAddEntryForRefresh(const SessionHistoryEntry* aEntry) { + nsCOMPtr currentURI = GetCurrentURI(); + return BrowsingContext::ShouldAddEntryForRefresh(currentURI, + aEntry->Info()); + } + // XXX(farre): Store a ContentParent pointer here rather than mProcessId? // Indicates which process owns the docshell. uint64_t mProcessId; diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp index 9de55a78eb60..4aacece423e9 100644 --- a/docshell/base/nsDocShell.cpp +++ b/docshell/base/nsDocShell.cpp @@ -8940,8 +8940,11 @@ nsresult nsDocShell::HandleSameDocumentNavigation( this, mLoadingEntry->mInfo.GetURI()->GetSpecOrDefault().get())); bool hadActiveEntry = !!mActiveEntry; mActiveEntry = MakeUnique(mLoadingEntry->mInfo); + // We're passing in mCurrentURI, which could be null. SessionHistoryCommit + // does require a non-null uri if this is for a refresh load of the same + // URI, but in that case mCurrentURI won't be null here. mBrowsingContext->SessionHistoryCommit( - *mLoadingEntry, mLoadType, hadActiveEntry, true, true, + *mLoadingEntry, mLoadType, mCurrentURI, hadActiveEntry, true, true, /* No expiration update on the same document loads*/ false); // FIXME Need to set postdata. @@ -13472,8 +13475,13 @@ void nsDocShell::MoveLoadingToActiveEntry(bool aPersist, bool aExpired) { MOZ_ASSERT(loadingEntry); uint32_t loadType = mLoadType == LOAD_ERROR_PAGE ? mFailedLoadType : mLoadType; - mBrowsingContext->SessionHistoryCommit( - *loadingEntry, loadType, hadActiveEntry, aPersist, false, aExpired); + + // We're passing in mCurrentURI, which could be null. SessionHistoryCommit + // does require a non-null uri if this is for a refresh load of the same + // URI, but in that case mCurrentURI won't be null here. + mBrowsingContext->SessionHistoryCommit(*loadingEntry, loadType, mCurrentURI, + hadActiveEntry, aPersist, false, + aExpired); } } diff --git a/docshell/test/mochitest/file_bug1742865.sjs b/docshell/test/mochitest/file_bug1742865.sjs new file mode 100644 index 000000000000..42c8e6367ea0 --- /dev/null +++ b/docshell/test/mochitest/file_bug1742865.sjs @@ -0,0 +1,65 @@ +Cu.importGlobalProperties(["URLSearchParams"]); + +function handleRequest(request, response) { + if (request.queryString == "reset") { + setState("index", "0"); + response.setStatusLine(request.httpVersion, 200, "Ok"); + response.write("Reset"); + return; + } + + let refresh = ""; + let index = Number(getState("index")); + // index == 0 First load, returns first meta refresh + // index == 1 Second load, caused by first meta refresh, returns second meta refresh + // index == 2 Third load, caused by second meta refresh, doesn't return a meta refresh + if (index < 2) { + let query = new URLSearchParams(request.queryString); + refresh = query.get("seconds"); + if (query.get("crossorigin") == "true") { + const hosts = ["example.org", "example.com"]; + + let url = `${request.scheme}://${hosts[index]}${request.path}?${request.queryString}`; + refresh += `; url=${url}`; + } + refresh = ``; + } + setState("index", String(index + 1)); + + response.write( + ` + + + + + ${refresh} + + + + + +` + ); +} diff --git a/docshell/test/mochitest/file_bug1742865_outer.sjs b/docshell/test/mochitest/file_bug1742865_outer.sjs new file mode 100644 index 000000000000..24d696415bea --- /dev/null +++ b/docshell/test/mochitest/file_bug1742865_outer.sjs @@ -0,0 +1,25 @@ +Cu.importGlobalProperties(["URLSearchParams"]); + +function handleRequest(request, response) { + response.write( + ` + + + + + + + + +` + ); +} diff --git a/docshell/test/mochitest/mochitest.ini b/docshell/test/mochitest/mochitest.ini index 8978e859014a..c63031fed234 100644 --- a/docshell/test/mochitest/mochitest.ini +++ b/docshell/test/mochitest/mochitest.ini @@ -125,6 +125,10 @@ support-files = support-files = file_bug1741132.html skip-if = toolkit == "android" && !sessionHistoryInParent +[test_bug1742865.html] +support-files = + file_bug1742865.sjs + file_bug1742865_outer.sjs [test_bug1743353.html] support-files = file_bug1743353.html diff --git a/docshell/test/mochitest/test_bug1742865.html b/docshell/test/mochitest/test_bug1742865.html new file mode 100644 index 000000000000..8beee9bb3b6c --- /dev/null +++ b/docshell/test/mochitest/test_bug1742865.html @@ -0,0 +1,103 @@ + + + + + Auto refreshing pages shouldn't add an entry to session history + + + + + +

+ +

+
+