From ed4c44789a77f9bd629117051acc3519aaeb3a96 Mon Sep 17 00:00:00 2001 From: Dimi Lee Date: Mon, 25 May 2020 18:30:43 +0000 Subject: [PATCH] Bug 1637634 - Update IsOnContentBlockingAllowList in nsHttpChannel::AsyncOpen r=timhuang,necko-reviewers This patch removes TYPE_DOCUMENT test in test_shouldclassify because the testcase creates a non-top level channel with TYPE_DOCUMENT flag (this is wong!), which triggers the assertion in UpdateIsOnContentBlockingAllowList. File a follow-up bug 1640715 to add TYPE_DOCUMENT test back. Differential Revision: https://phabricator.services.mozilla.com/D76152 --- netwerk/cookie/CookieJarSettings.cpp | 11 +++++++---- netwerk/ipc/DocumentLoadListener.cpp | 10 ---------- netwerk/protocol/http/nsHttpChannel.cpp | 10 ++++++++-- .../tests/unit/test_shouldclassify.js | 13 ++++--------- 4 files changed, 19 insertions(+), 25 deletions(-) diff --git a/netwerk/cookie/CookieJarSettings.cpp b/netwerk/cookie/CookieJarSettings.cpp index 0fac2f48893a..135f45796183 100644 --- a/netwerk/cookie/CookieJarSettings.cpp +++ b/netwerk/cookie/CookieJarSettings.cpp @@ -431,19 +431,22 @@ void CookieJarSettings::UpdateIsOnContentBlockingAllowList( MOZ_ASSERT(bc->IsTop()); #endif - nsCOMPtr uriBeingLoaded = - AntiTrackingUtils::MaybeGetDocumentURIBeingLoaded(aChannel); - nsCOMPtr contentBlockingAllowListPrincipal; + nsCOMPtr uri; + nsresult rv = aChannel->GetURI(getter_AddRefs(uri)); + if (NS_WARN_IF(NS_FAILED(rv))) { + return; + } // We need to recompute the ContentBlockingAllowListPrincipal here for the // top level channel because we might navigate from the the initial // about:blank page or the existing page which may have a different origin // than the URI we are going to load here. Thus, we need to recompute the // prinicpal in order to get the correct ContentBlockingAllowListPrincipal. + nsCOMPtr contentBlockingAllowListPrincipal; OriginAttributes attrs; loadInfo->GetOriginAttributes(&attrs); ContentBlockingAllowList::RecomputePrincipal( - uriBeingLoaded, attrs, getter_AddRefs(contentBlockingAllowListPrincipal)); + uri, attrs, getter_AddRefs(contentBlockingAllowListPrincipal)); if (!contentBlockingAllowListPrincipal || !contentBlockingAllowListPrincipal->GetIsContentPrincipal()) { diff --git a/netwerk/ipc/DocumentLoadListener.cpp b/netwerk/ipc/DocumentLoadListener.cpp index 433b3a7a9959..f69dac902413 100644 --- a/netwerk/ipc/DocumentLoadListener.cpp +++ b/netwerk/ipc/DocumentLoadListener.cpp @@ -8,7 +8,6 @@ #include "DocumentLoadListener.h" #include "mozilla/AntiTrackingUtils.h" -#include "mozilla/ContentBlockingAllowList.h" #include "mozilla/LoadInfo.h" #include "mozilla/MozPromiseInlines.h" // For MozPromise::FromDomPromise #include "mozilla/StaticPrefs_fission.h" @@ -413,15 +412,6 @@ bool DocumentLoadListener::Open( // If this is for the top level loading, the top window URI should be the // URI which we are loading. topWindowURI = uriBeingLoaded; - - // Update the IsOnContentBlockingAllowList flag in the CookieJarSettings - // if this is a top level loading. For sub-document loading, this flag - // would inherit from the parent. - nsCOMPtr cookieJarSettings; - Unused << loadInfo->GetCookieJarSettings( - getter_AddRefs(cookieJarSettings)); - net::CookieJarSettings::Cast(cookieJarSettings) - ->UpdateIsOnContentBlockingAllowList(mChannel); } else if (RefPtr topWindow = AntiTrackingUtils:: GetTopWindowExcludingExtensionAccessibleContentFrames( browsingContext, uriBeingLoaded)) { diff --git a/netwerk/protocol/http/nsHttpChannel.cpp b/netwerk/protocol/http/nsHttpChannel.cpp index 60c15f4a39e7..075a4d649008 100644 --- a/netwerk/protocol/http/nsHttpChannel.cpp +++ b/netwerk/protocol/http/nsHttpChannel.cpp @@ -1863,14 +1863,20 @@ void nsHttpChannel::UpdateAntiTrackingInfo() { AntiTrackingUtils::ComputeIsThirdPartyToTopWindow(this); - // We only need to set FPD for top-level loads. FPD will automatically be - // propagated to non-top level loads via CookieJarSetting. if (mLoadInfo->GetExternalContentPolicyType() == nsIContentPolicy::TYPE_DOCUMENT) { nsCOMPtr cookieJarSettings; Unused << mLoadInfo->GetCookieJarSettings( getter_AddRefs(cookieJarSettings)); + // Update the IsOnContentBlockingAllowList flag in the CookieJarSettings + // if this is a top level loading. For sub-document loading, this flag + // would inherit from the parent. + mozilla::net::CookieJarSettings::Cast(cookieJarSettings) + ->UpdateIsOnContentBlockingAllowList(this); + + // We only need to set FPD for top-level loads. FPD will automatically be + // propagated to non-top level loads via CookieJarSetting. mozilla::net::CookieJarSettings::Cast(cookieJarSettings) ->SetFirstPartyDomain(mURI); } diff --git a/toolkit/components/url-classifier/tests/unit/test_shouldclassify.js b/toolkit/components/url-classifier/tests/unit/test_shouldclassify.js index 7faeb9117e4e..4921c93dbce0 100644 --- a/toolkit/components/url-classifier/tests/unit/test_shouldclassify.js +++ b/toolkit/components/url-classifier/tests/unit/test_shouldclassify.js @@ -16,16 +16,15 @@ var normalOrigin, trackingOrigin; // ShouldClassify algorithm uses the following parameters: // 1. Ci.nsIChannel.LOAD_ BYPASS_URL_CLASSIFIER loadflags -// 2. Content type +// 2. Content type // TODO: Bug 1640715 to test this // 3. triggering principal // 4. be Conservative // We test are the combinations here to make sure the algorithm is correct const PARAM_LOAD_BYPASS_URL_CLASSIFIER = 1 << 0; -const PARAM_CONTENT_POLICY_TYPE_DOCUMENT = 1 << 1; -const PARAM_TRIGGERING_PRINCIPAL_SYSTEM = 1 << 2; -const PARAM_CAP_BE_CONSERVATIVE = 1 << 3; -const PARAM_MAX = 1 << 4; +const PARAM_TRIGGERING_PRINCIPAL_SYSTEM = 1 << 1; +const PARAM_CAP_BE_CONSERVATIVE = 1 << 2; +const PARAM_MAX = 1 << 3; function getParameters(bitFlags) { var params = { @@ -39,10 +38,6 @@ function getParameters(bitFlags) { params.loadFlags = Ci.nsIChannel.LOAD_BYPASS_URL_CLASSIFIER; } - if (bitFlags & PARAM_CONTENT_POLICY_TYPE_DOCUMENT) { - params.contentType = Ci.nsIContentPolicy.TYPE_DOCUMENT; - } - if (bitFlags & PARAM_TRIGGERING_PRINCIPAL_SYSTEM) { params.system = true; }