From 87ae6eae9fe132a84223e5b8876e88a05fedc108 Mon Sep 17 00:00:00 2001 From: Andreea Pavel Date: Thu, 20 May 2021 18:59:30 +0300 Subject: [PATCH] Backed out 3 changesets (bug 1695911, bug 1648825) for bc failures at browser_navigation.js on a CLOSED TREE Backed out changeset 1d7e78cac600 (bug 1695911) Backed out changeset d90566e41269 (bug 1648825) Backed out changeset 5aece2a17f5d (bug 1648825) --- docshell/base/nsDocShell.cpp | 12 +- docshell/shistory/SessionHistoryEntry.cpp | 19 +- docshell/shistory/SessionHistoryEntry.h | 1 - docshell/shistory/nsISHEntry.idl | 10 +- docshell/shistory/nsSHEntry.cpp | 25 +-- docshell/shistory/nsSHEntry.h | 1 - dom/security/SecFetch.cpp | 5 - dom/security/test/moz.build | 1 - dom/security/test/sec-fetch/.eslintrc.js | 6 +- dom/security/test/sec-fetch/browser.ini | 4 - .../test/sec-fetch/browser_navigation.js | 181 ------------------ dom/security/test/sec-fetch/file_no_cache.sjs | 28 --- dom/security/test/sec-fetch/mochitest.ini | 2 - .../test_iframe_history_manipulation.html | 90 --------- modules/libpref/init/StaticPrefList.yaml | 2 +- testing/marionette/navigate.js | 2 - 16 files changed, 10 insertions(+), 379 deletions(-) delete mode 100644 dom/security/test/sec-fetch/browser.ini delete mode 100644 dom/security/test/sec-fetch/browser_navigation.js delete mode 100644 dom/security/test/sec-fetch/file_no_cache.sjs delete mode 100644 dom/security/test/sec-fetch/test_iframe_history_manipulation.html diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp index 61da1ff539d9..1da3e53d959f 100644 --- a/docshell/base/nsDocShell.cpp +++ b/docshell/base/nsDocShell.cpp @@ -11662,8 +11662,6 @@ nsresult nsDocShell::AddToSessionHistory( bool expired = false; // by default the page is not expired bool discardLayoutState = false; nsCOMPtr cacheChannel; - bool userActivation = false; - if (aChannel) { cacheChannel = do_QueryInterface(aChannel); @@ -11704,8 +11702,6 @@ nsresult nsDocShell::AddToSessionHistory( loadInfo->GetResultPrincipalURI(getter_AddRefs(resultPrincipalURI)); - userActivation = loadInfo->GetHasValidUserGestureActivation(); - // For now keep storing just the principal in the SHEntry. if (!principalToInherit) { if (loadInfo->GetLoadingSandboxed()) { @@ -11774,7 +11770,7 @@ nsresult nsDocShell::AddToSessionHistory( principalToInherit, partitionedPrincipalToInherit, csp, HistoryID(), GetCreatedDynamically(), originalURI, resultPrincipalURI, loadReplace, referrerInfo, srcdoc, - srcdocEntry, baseURI, saveLayoutState, expired, userActivation); + srcdocEntry, baseURI, saveLayoutState, expired); if (mBrowsingContext->IsTop() && GetSessionHistory()) { bool shouldPersist = ShouldAddToSessionHistory(aURI, aChannel); @@ -11887,8 +11883,7 @@ nsresult nsDocShell::LoadHistoryEntry(nsISHEntry* aEntry, uint32_t aLoadType, // in case. nsCOMPtr kungFuDeathGrip(aEntry); - loadState->SetHasValidUserGestureActivation( - loadState->HasValidUserGestureActivation() || aUserActivation); + loadState->SetHasValidUserGestureActivation(aUserActivation); return LoadHistoryEntry(loadState, aLoadType, aEntry == mOSHE); } @@ -11897,8 +11892,7 @@ nsresult nsDocShell::LoadHistoryEntry(const LoadingSessionHistoryInfo& aEntry, uint32_t aLoadType, bool aUserActivation) { RefPtr loadState = aEntry.CreateLoadInfo(); - loadState->SetHasValidUserGestureActivation( - loadState->HasValidUserGestureActivation() || aUserActivation); + loadState->SetHasValidUserGestureActivation(aUserActivation); return LoadHistoryEntry(loadState, aLoadType, aEntry.mLoadingCurrentActiveEntry); diff --git a/docshell/shistory/SessionHistoryEntry.cpp b/docshell/shistory/SessionHistoryEntry.cpp index 61ec28bbb9be..4450a68c0220 100644 --- a/docshell/shistory/SessionHistoryEntry.cpp +++ b/docshell/shistory/SessionHistoryEntry.cpp @@ -48,7 +48,6 @@ SessionHistoryInfo::SessionHistoryInfo(nsDocShellLoadState* aLoadState, /* FIXME Should this be aLoadState->IsSrcdocLoad()? */ mIsSrcdocEntry(!aLoadState->SrcdocData().IsEmpty()), mHasUserInteraction(false), - mHasUserActivation(aLoadState->HasValidUserGestureActivation()), mSharedState(SharedState::Create( aLoadState->TriggeringPrincipal(), aLoadState->PrincipalToInherit(), aLoadState->PartitionedPrincipalToInherit(), aLoadState->Csp(), @@ -141,7 +140,6 @@ void SessionHistoryInfo::Reset(nsIURI* aURI, const nsID& aDocShellID, mScrollRestorationIsManual = false; mPersist = false; mHasUserInteraction = false; - mHasUserActivation = false; mSharedState.Get()->mTriggeringPrincipal = aTriggeringPrincipal; mSharedState.Get()->mPrincipalToInherit = aPrincipalToInherit; @@ -554,18 +552,6 @@ SessionHistoryEntry::SetHasUserInteraction(bool aFlag) { return NS_OK; } -NS_IMETHODIMP -SessionHistoryEntry::GetHasUserActivation(bool* aFlag) { - *aFlag = mInfo->mHasUserActivation; - return NS_OK; -} - -NS_IMETHODIMP -SessionHistoryEntry::SetHasUserActivation(bool aFlag) { - mInfo->mHasUserActivation = aFlag; - return NS_OK; -} - NS_IMETHODIMP SessionHistoryEntry::GetReferrerInfo(nsIReferrerInfo** aReferrerInfo) { nsCOMPtr referrerInfo = mInfo->mReferrerInfo; @@ -993,8 +979,7 @@ SessionHistoryEntry::Create( nsIContentSecurityPolicy* aCsp, const nsID& aDocshellID, bool aDynamicCreation, nsIURI* aOriginalURI, nsIURI* aResultPrincipalURI, bool aLoadReplace, nsIReferrerInfo* aReferrerInfo, const nsAString& aSrcdoc, - bool aSrcdocEntry, nsIURI* aBaseURI, bool aSaveLayoutState, bool aExpired, - bool aUserActivation) { + bool aSrcdocEntry, nsIURI* aBaseURI, bool aSaveLayoutState, bool aExpired) { MOZ_CRASH("Might need to implement this"); return NS_ERROR_NOT_IMPLEMENTED; } @@ -1448,7 +1433,6 @@ void IPDLParamTraits::Write( WriteIPDLParam(aMsg, aActor, aParam.mScrollRestorationIsManual); WriteIPDLParam(aMsg, aActor, aParam.mPersist); WriteIPDLParam(aMsg, aActor, aParam.mHasUserInteraction); - WriteIPDLParam(aMsg, aActor, aParam.mHasUserActivation); WriteIPDLParam(aMsg, aActor, aParam.mSharedState.Get()->mId); WriteIPDLParam(aMsg, aActor, aParam.mSharedState.Get()->mTriggeringPrincipal); WriteIPDLParam(aMsg, aActor, aParam.mSharedState.Get()->mPrincipalToInherit); @@ -1486,7 +1470,6 @@ bool IPDLParamTraits::Read( &aResult->mScrollRestorationIsManual) || !ReadIPDLParam(aMsg, aIter, aActor, &aResult->mPersist) || !ReadIPDLParam(aMsg, aIter, aActor, &aResult->mHasUserInteraction) || - !ReadIPDLParam(aMsg, aIter, aActor, &aResult->mHasUserActivation) || !ReadIPDLParam(aMsg, aIter, aActor, &sharedId)) { aActor->FatalError("Error reading fields for SessionHistoryInfo"); return false; diff --git a/docshell/shistory/SessionHistoryEntry.h b/docshell/shistory/SessionHistoryEntry.h index e74311578368..3b158b227c04 100644 --- a/docshell/shistory/SessionHistoryEntry.h +++ b/docshell/shistory/SessionHistoryEntry.h @@ -169,7 +169,6 @@ class SessionHistoryInfo { bool mScrollRestorationIsManual = false; bool mPersist = true; bool mHasUserInteraction = false; - bool mHasUserActivation = false; union SharedState { SharedState(); diff --git a/docshell/shistory/nsISHEntry.idl b/docshell/shistory/nsISHEntry.idl index 73ac40551d4e..4b28726788a3 100644 --- a/docshell/shistory/nsISHEntry.idl +++ b/docshell/shistory/nsISHEntry.idl @@ -97,13 +97,6 @@ interface nsISHEntry : nsISupports */ [infallible] attribute boolean hasUserInteraction; - /** - * Whether the load that created this entry was triggered by user activation. - * (e.g.: The user clicked a link) - * Remembering this flag enables replaying the sec-fetch-* headers. - */ - [infallible] attribute boolean hasUserActivation; - /** Referrer Info*/ [infallible] attribute nsIReferrerInfo referrerInfo; @@ -328,8 +321,7 @@ interface nsISHEntry : nsISupports in bool srcdocEntry, in nsIURI baseURI, in bool saveLayoutState, - in bool expired, - in bool userActivation); + in bool expired); nsISHEntry clone(); diff --git a/docshell/shistory/nsSHEntry.cpp b/docshell/shistory/nsSHEntry.cpp index 1e4000eacd2b..45f33549fa58 100644 --- a/docshell/shistory/nsSHEntry.cpp +++ b/docshell/shistory/nsSHEntry.cpp @@ -43,8 +43,7 @@ nsSHEntry::nsSHEntry() mScrollRestorationIsManual(false), mLoadedInThisProcess(false), mPersist(true), - mHasUserInteraction(false), - mHasUserActivation(false) {} + mHasUserInteraction(false) {} nsSHEntry::nsSHEntry(const nsSHEntry& aOther) : mShared(aOther.mShared), @@ -71,8 +70,7 @@ nsSHEntry::nsSHEntry(const nsSHEntry& aOther) mScrollRestorationIsManual(false), mLoadedInThisProcess(aOther.mLoadedInThisProcess), mPersist(aOther.mPersist), - mHasUserInteraction(false), - mHasUserActivation(aOther.mHasUserActivation) {} + mHasUserInteraction(false) {} nsSHEntry::~nsSHEntry() { // Null out the mParent pointers on all our kids. @@ -326,18 +324,6 @@ nsSHEntry::SetHasUserInteraction(bool aFlag) { return NS_OK; } -NS_IMETHODIMP -nsSHEntry::GetHasUserActivation(bool* aFlag) { - *aFlag = mHasUserActivation; - return NS_OK; -} - -NS_IMETHODIMP -nsSHEntry::SetHasUserActivation(bool aFlag) { - mHasUserActivation = aFlag; - return NS_OK; -} - NS_IMETHODIMP nsSHEntry::GetCacheKey(uint32_t* aResult) { *aResult = mShared->mCacheKey; @@ -374,7 +360,7 @@ nsSHEntry::Create(nsIURI* aURI, const nsAString& aTitle, nsIURI* aResultPrincipalURI, bool aLoadReplace, nsIReferrerInfo* aReferrerInfo, const nsAString& aSrcdocData, bool aSrcdocEntry, nsIURI* aBaseURI, bool aSaveLayoutState, - bool aExpired, bool aUserActivation) { + bool aExpired) { MOZ_ASSERT( aTriggeringPrincipal, "need a valid triggeringPrincipal to create a session history entry"); @@ -416,8 +402,6 @@ nsSHEntry::Create(nsIURI* aURI, const nsAString& aTitle, mLoadReplace = aLoadReplace; mReferrerInfo = aReferrerInfo; - mHasUserActivation = aUserActivation; - mShared->mLayoutHistoryState = nullptr; mShared->mSaveLayoutState = aSaveLayoutState; @@ -934,9 +918,6 @@ nsSHEntry::CreateLoadInfo(nsDocShellLoadState** aLoadState) { loadState->SetInternalLoadFlags(flags); loadState->SetFirstParty(true); - - loadState->SetHasValidUserGestureActivation(GetHasUserActivation()); - loadState->SetSHEntry(this); loadState.forget(aLoadState); diff --git a/docshell/shistory/nsSHEntry.h b/docshell/shistory/nsSHEntry.h index 326b0092cf94..8c94726f0966 100644 --- a/docshell/shistory/nsSHEntry.h +++ b/docshell/shistory/nsSHEntry.h @@ -65,7 +65,6 @@ class nsSHEntry : public nsISHEntry { bool mLoadedInThisProcess; bool mPersist; bool mHasUserInteraction; - bool mHasUserActivation; }; #endif /* nsSHEntry_h */ diff --git a/dom/security/SecFetch.cpp b/dom/security/SecFetch.cpp index 0ce1581b58f9..6f30007f333f 100644 --- a/dom/security/SecFetch.cpp +++ b/dom/security/SecFetch.cpp @@ -217,11 +217,6 @@ bool IsUserTriggeredForSecFetchSite(nsIHttpChannel* aHTTPChannel) { return false; } - // sec-fetch-site can only be user triggered if the load was user triggered. - if (!loadInfo->GetHasValidUserGestureActivation()) { - return false; - } - // We can assert that the navigation must be "webby" if the load was triggered // by a meta refresh. See also Bug 1647128. if (loadInfo->GetIsMetaRefresh()) { diff --git a/dom/security/test/moz.build b/dom/security/test/moz.build index b50e6359ed01..0fb27de75208 100644 --- a/dom/security/test/moz.build +++ b/dom/security/test/moz.build @@ -38,5 +38,4 @@ BROWSER_CHROME_MANIFESTS += [ "https-first/browser.ini", "https-only/browser.ini", "mixedcontentblocker/browser.ini", - "sec-fetch/browser.ini", ] diff --git a/dom/security/test/sec-fetch/.eslintrc.js b/dom/security/test/sec-fetch/.eslintrc.js index 317abe7b487a..845ed3f013b8 100644 --- a/dom/security/test/sec-fetch/.eslintrc.js +++ b/dom/security/test/sec-fetch/.eslintrc.js @@ -1,9 +1,5 @@ "use strict"; module.exports = { - extends: [ - "plugin:mozilla/browser-test", - "plugin:mozilla/chrome-test", - "plugin:mozilla/mochitest-test", - ], + extends: ["plugin:mozilla/mochitest-test"], }; diff --git a/dom/security/test/sec-fetch/browser.ini b/dom/security/test/sec-fetch/browser.ini deleted file mode 100644 index b9642b438e04..000000000000 --- a/dom/security/test/sec-fetch/browser.ini +++ /dev/null @@ -1,4 +0,0 @@ -[DEFAULT] -support-files = file_no_cache.sjs - -[browser_navigation.js] diff --git a/dom/security/test/sec-fetch/browser_navigation.js b/dom/security/test/sec-fetch/browser_navigation.js deleted file mode 100644 index 7c93b4441f84..000000000000 --- a/dom/security/test/sec-fetch/browser_navigation.js +++ /dev/null @@ -1,181 +0,0 @@ -"use strict"; - -const REQUEST_URL = - "https://example.com/browser/dom/security/test/sec-fetch/file_no_cache.sjs"; - -let gTestCounter = 0; -let gExpectedHeader = {}; - -async function setup() { - waitForExplicitFinish(); - - await SpecialPowers.pushPrefEnv({ - set: [["dom.security.secFetch.enabled", true]], - }); -} - -function checkSecFetchUser(subject, topic, data) { - let channel = subject.QueryInterface(Ci.nsIHttpChannel); - if (!channel.URI.spec.startsWith("https://example.com/")) { - return; - } - - info(`testing headers for load of ${channel.URI.spec}`); - - const secFetchHeaders = [ - "sec-fetch-mode", - "sec-fetch-dest", - "sec-fetch-user", - "sec-fetch-site", - ]; - - secFetchHeaders.forEach(header => { - const expectedValue = gExpectedHeader[header]; - try { - is( - channel.getRequestHeader(header), - expectedValue, - `${header} is set to ${expectedValue}` - ); - } catch (e) { - if (expectedValue) { - ok(false, "required headers are set"); - } else { - ok(true, `${header} should not be set`); - } - } - }); - - gTestCounter++; -} - -async function testNavigations() { - gTestCounter = 0; - - // Load initial site - let loaded = BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser); - BrowserTestUtils.loadURI(gBrowser, REQUEST_URL + "?test1"); - await loaded; - - // Load another site - loaded = BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser); - await SpecialPowers.spawn(gBrowser.selectedBrowser, [], async function() { - content.document.notifyUserGestureActivation(); // simulate user activation - let test2Button = content.document.getElementById("test2_button"); - test2Button.click(); - content.document.clearUserGestureActivation(); - }); - await loaded; - // Load another site - loaded = BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser); - await SpecialPowers.spawn(gBrowser.selectedBrowser, [], async function() { - content.document.notifyUserGestureActivation(); // simulate user activation - let test3Button = content.document.getElementById("test3_button"); - test3Button.click(); - content.document.clearUserGestureActivation(); - }); - await loaded; - - gExpectedHeader = { - "sec-fetch-mode": "navigate", - "sec-fetch-dest": "document", - "sec-fetch-site": "same-origin", - "sec-fetch-user": "?1", - }; - - // Register the http request observer. - // All following actions should cause requests with the sec-fetch-user header - // set. - Services.obs.addObserver(checkSecFetchUser, "http-on-stop-request"); - - // Go back one site by clicking the back button - info("Clicking back button"); - loaded = BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser); - document.notifyUserGestureActivation(); // simulate user activation - let backButton = document.getElementById("back-button"); - backButton.click(); - document.clearUserGestureActivation(); - await loaded; - - // Reload the site by clicking the reload button - info("Clicking reload button"); - loaded = BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser); - document.notifyUserGestureActivation(); // simulate user activation - let reloadButton = document.getElementById("reload-button"); - reloadButton.click(); - document.clearUserGestureActivation(); - await loaded; - - // Go forward one site by clicking the forward button - info("Clicking forward button"); - loaded = BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser); - document.notifyUserGestureActivation(); // simulate user activation - let forwardButton = document.getElementById("forward-button"); - forwardButton.click(); - document.clearUserGestureActivation(); - await loaded; - - // Testing history.back/forward... - - info("going back with history.back"); - loaded = BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser); - await SpecialPowers.spawn(gBrowser.selectedBrowser, [], async function() { - content.document.notifyUserGestureActivation(); // simulate user activation - content.history.back(); - content.document.clearUserGestureActivation(); - }); - await loaded; - - info("going forward with history.forward"); - loaded = BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser); - await SpecialPowers.spawn(gBrowser.selectedBrowser, [], async function() { - content.document.notifyUserGestureActivation(); // simulate user activation - content.history.forward(); - content.document.clearUserGestureActivation(); - }); - await loaded; - - gExpectedHeader = { - "sec-fetch-mode": "navigate", - "sec-fetch-dest": "document", - "sec-fetch-site": "same-origin", - }; - - info("going back with history.back without user activation"); - loaded = BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser); - await SpecialPowers.spawn(gBrowser.selectedBrowser, [], async function() { - content.history.back(); - }); - await loaded; - - info("going forward with history.forward without user activation"); - loaded = BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser); - await SpecialPowers.spawn(gBrowser.selectedBrowser, [], async function() { - content.history.forward(); - }); - await loaded; - - ok(gTestCounter === 7, "testing that all five actions have been tested."); - - Services.obs.removeObserver(checkSecFetchUser, "http-on-stop-request"); -} - -add_task(async function() { - waitForExplicitFinish(); - - await SpecialPowers.pushPrefEnv({ - set: [["dom.security.secFetch.enabled", true]], - }); - - await testNavigations(); - - if (SpecialPowers.getBoolPref("fission.autostart")) { - await SpecialPowers.pushPrefEnv({ - set: [["fission.bfcacheInParent", true]], - }); - - await testNavigations(); - } - - finish(); -}); diff --git a/dom/security/test/sec-fetch/file_no_cache.sjs b/dom/security/test/sec-fetch/file_no_cache.sjs deleted file mode 100644 index abc20797f846..000000000000 --- a/dom/security/test/sec-fetch/file_no_cache.sjs +++ /dev/null @@ -1,28 +0,0 @@ -const MESSAGE_PAGE = function (msg) { - return ` - - - - - Click me - Click me - - -`; -}; - -function handleRequest(request, response) { - response.setHeader("Cache-Control", "no-store"); - response.setHeader("Content-Type", "text/html"); - - response.write(MESSAGE_PAGE(request.queryString)); -} diff --git a/dom/security/test/sec-fetch/mochitest.ini b/dom/security/test/sec-fetch/mochitest.ini index 8f42c94286bd..305bf7402b2c 100644 --- a/dom/security/test/sec-fetch/mochitest.ini +++ b/dom/security/test/sec-fetch/mochitest.ini @@ -1,6 +1,5 @@ [DEFAULT] support-files = - file_no_cache.sjs file_redirect.sjs [test_websocket.html] @@ -9,4 +8,3 @@ support-files = file_websocket_wsh.py [test_iframe_src_metaRedirect.html] [test_iframe_srcdoc_metaRedirect.html] [test_iframe_window_open_metaRedirect.html] -[test_iframe_history_manipulation.html] diff --git a/dom/security/test/sec-fetch/test_iframe_history_manipulation.html b/dom/security/test/sec-fetch/test_iframe_history_manipulation.html deleted file mode 100644 index 978b95ba7776..000000000000 --- a/dom/security/test/sec-fetch/test_iframe_history_manipulation.html +++ /dev/null @@ -1,90 +0,0 @@ - - - - Bug 1648825 - Fetch Metadata Headers contain invalid value for Sec-Fetch-Site for history manipulation - - - - - - - - - - diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml index 4d86f4671823..264d87b358da 100644 --- a/modules/libpref/init/StaticPrefList.yaml +++ b/modules/libpref/init/StaticPrefList.yaml @@ -2884,7 +2884,7 @@ # request headers to be set. - name: dom.security.secFetch.enabled type: RelaxedAtomicBool - value: true + value: @IS_NIGHTLY_BUILD@ mirror: always # This pref enables the featurePolicy header support. diff --git a/testing/marionette/navigate.js b/testing/marionette/navigate.js index ff856eb79dba..4a9e9d5459ae 100644 --- a/testing/marionette/navigate.js +++ b/testing/marionette/navigate.js @@ -166,8 +166,6 @@ navigate.navigateTo = async function(browsingContext, url) { const opts = { loadFlags: Ci.nsIWebNavigation.LOAD_FLAGS_IS_LINK, triggeringPrincipal: Services.scriptSecurityManager.getSystemPrincipal(), - // Fake user activation. - hasValidUserGestureActivation: true, }; browsingContext.loadURI(url, opts); };