From 2cc4039e44a82b28993ef112b4c5583c9cfec000 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sat, 15 Mar 2025 15:36:25 +0000 Subject: [PATCH] Bug 1954134 - Simplify ImageCacheKey partitioning. r=necko-reviewers,valentin,arai Use the same scheme as the JS and CSS caches: https://searchfox.org/mozilla-central/rev/5c2888b35d56928d252acf84e8816fa89a8a6a61/layout/style/Loader.cpp#2341 https://searchfox.org/mozilla-central/rev/5c2888b35d56928d252acf84e8816fa89a8a6a61/dom/script/ScriptLoader.cpp#329 (And fonts, and plenty of others) Differential Revision: https://phabricator.services.mozilla.com/D241664 --- dom/script/SharedScriptCache.cpp | 4 +- dom/script/SharedScriptCache.h | 8 +- image/ImageCacheKey.cpp | 115 ++++++-------------------- image/ImageCacheKey.h | 19 ++--- image/imgLoader.cpp | 85 +++++-------------- layout/style/Loader.cpp | 16 ++-- layout/style/Loader.h | 10 +-- layout/style/SharedSubResourceCache.h | 5 +- 8 files changed, 74 insertions(+), 188 deletions(-) diff --git a/dom/script/SharedScriptCache.cpp b/dom/script/SharedScriptCache.cpp index cb63a9e94443..2fd271214fb9 100644 --- a/dom/script/SharedScriptCache.cpp +++ b/dom/script/SharedScriptCache.cpp @@ -24,7 +24,7 @@ ScriptHashKey::ScriptHashKey(ScriptLoader* aLoader, const JS::loader::ScriptLoadRequest* aRequest) : PLDHashEntryHdr(), mURI(aRequest->mURI), - mPrincipal(aRequest->TriggeringPrincipal()), + mTriggeringPrincipal(aRequest->TriggeringPrincipal()), mLoaderPrincipal(aLoader->LoaderPrincipal()), mPartitionPrincipal(aLoader->PartitionedPrincipal()), mCORSMode(aRequest->CORSMode()), @@ -56,7 +56,7 @@ bool ScriptHashKey::KeyEquals(const ScriptHashKey& aKey) const { } } - if (!mPrincipal->Equals(aKey.mPrincipal)) { + if (!mTriggeringPrincipal->Equals(aKey.mTriggeringPrincipal)) { return false; } diff --git a/dom/script/SharedScriptCache.h b/dom/script/SharedScriptCache.h index 99c67ff97881..2625b5cd3177 100644 --- a/dom/script/SharedScriptCache.h +++ b/dom/script/SharedScriptCache.h @@ -38,7 +38,7 @@ class ScriptHashKey : public PLDHashEntryHdr { explicit ScriptHashKey(const ScriptHashKey& aKey) : PLDHashEntryHdr(), mURI(aKey.mURI), - mPrincipal(aKey.mPrincipal), + mTriggeringPrincipal(aKey.mTriggeringPrincipal), mLoaderPrincipal(aKey.mLoaderPrincipal), mPartitionPrincipal(aKey.mPartitionPrincipal), mCORSMode(aKey.mCORSMode), @@ -55,7 +55,7 @@ class ScriptHashKey : public PLDHashEntryHdr { ScriptHashKey(ScriptHashKey&& aKey) : PLDHashEntryHdr(), mURI(std::move(aKey.mURI)), - mPrincipal(std::move(aKey.mPrincipal)), + mTriggeringPrincipal(std::move(aKey.mTriggeringPrincipal)), mLoaderPrincipal(std::move(aKey.mLoaderPrincipal)), mPartitionPrincipal(std::move(aKey.mPartitionPrincipal)), mCORSMode(std::move(aKey.mCORSMode)), @@ -87,7 +87,7 @@ class ScriptHashKey : public PLDHashEntryHdr { return nsURIHashKey::HashKey(aKey->mURI); } - nsIPrincipal* Principal() const { return mPrincipal; } + nsIPrincipal* TriggeringPrincipal() const { return mTriggeringPrincipal; } nsIPrincipal* LoaderPrincipal() const { return mLoaderPrincipal; } nsIPrincipal* PartitionPrincipal() const { return mPartitionPrincipal; } @@ -97,7 +97,7 @@ class ScriptHashKey : public PLDHashEntryHdr { protected: const nsCOMPtr mURI; - const nsCOMPtr mPrincipal; + const nsCOMPtr mTriggeringPrincipal; const nsCOMPtr mLoaderPrincipal; const nsCOMPtr mPartitionPrincipal; const CORSMode mCORSMode; diff --git a/image/ImageCacheKey.cpp b/image/ImageCacheKey.cpp index 7317daf11d3f..3dfc968554b9 100644 --- a/image/ImageCacheKey.cpp +++ b/image/ImageCacheKey.cpp @@ -13,15 +13,12 @@ #include "mozilla/StoragePrincipalHelper.h" #include "mozilla/Unused.h" #include "mozilla/dom/Document.h" -#include "mozilla/dom/File.h" #include "mozilla/dom/ServiceWorkerManager.h" #include "mozilla/StaticPrefs_privacy.h" #include "mozilla/StorageAccess.h" #include "nsContentUtils.h" #include "nsHashKeys.h" #include "nsLayoutUtils.h" -#include "nsPrintfCString.h" -#include "nsString.h" namespace mozilla { @@ -29,33 +26,33 @@ using namespace dom; namespace image { +static nsIPrincipal* GetLoaderPrincipal(Document* aDocument) { + return aDocument ? aDocument->NodePrincipal() + : nsContentUtils::GetSystemPrincipal(); +} + +static nsIPrincipal* GetPartitionPrincipal(Document* aDocument) { + if (aDocument && StaticPrefs::privacy_partition_network_state()) { + return aDocument->PartitionedPrincipal(); + } + return GetLoaderPrincipal(aDocument); +} + ImageCacheKey::ImageCacheKey(nsIURI* aURI, CORSMode aCORSMode, const OriginAttributes& aAttrs, Document* aDocument) : mURI(aURI), - mOriginAttributes(aAttrs), mControlledDocument(GetSpecialCaseDocumentToken(aDocument)), - mIsolationKey(GetIsolationKey(aDocument, aURI)), + mLoaderPrincipal(GetLoaderPrincipal(aDocument)), + mPartitionPrincipal(GetPartitionPrincipal(aDocument)), mCORSMode(aCORSMode), - mAppType(GetAppType(aDocument)) {} + mAppType(GetAppType(aDocument)) { + MOZ_ASSERT(mLoaderPrincipal); + MOZ_ASSERT(mPartitionPrincipal); +} -ImageCacheKey::ImageCacheKey(const ImageCacheKey& aOther) - : mURI(aOther.mURI), - mOriginAttributes(aOther.mOriginAttributes), - mControlledDocument(aOther.mControlledDocument), - mIsolationKey(aOther.mIsolationKey), - mHash(aOther.mHash), - mCORSMode(aOther.mCORSMode), - mAppType(aOther.mAppType) {} - -ImageCacheKey::ImageCacheKey(ImageCacheKey&& aOther) - : mURI(std::move(aOther.mURI)), - mOriginAttributes(aOther.mOriginAttributes), - mControlledDocument(aOther.mControlledDocument), - mIsolationKey(aOther.mIsolationKey), - mHash(aOther.mHash), - mCORSMode(aOther.mCORSMode), - mAppType(aOther.mAppType) {} +ImageCacheKey::ImageCacheKey(const ImageCacheKey& aOther) = default; +ImageCacheKey::ImageCacheKey(ImageCacheKey&& aOther) = default; bool ImageCacheKey::operator==(const ImageCacheKey& aOther) const { // Don't share the image cache between a controlled document and anything @@ -63,14 +60,8 @@ bool ImageCacheKey::operator==(const ImageCacheKey& aOther) const { if (mControlledDocument != aOther.mControlledDocument) { return false; } - // Don't share the image cache between two top-level documents of different - // base domains. - if (!mIsolationKey.Equals(aOther.mIsolationKey, - nsCaseInsensitiveCStringComparator)) { - return false; - } - // The origin attributes always have to match. - if (mOriginAttributes != aOther.mOriginAttributes) { + + if (!mPartitionPrincipal->Equals(aOther.mPartitionPrincipal)) { return false; } @@ -90,22 +81,11 @@ bool ImageCacheKey::operator==(const ImageCacheKey& aOther) const { void ImageCacheKey::EnsureHash() const { MOZ_ASSERT(mHash.isNothing()); - PLDHashNumber hash = 0; - - // Since we frequently call Hash() several times in a row on the same - // ImageCacheKey, as an optimization we compute our hash once and store it. - - nsPrintfCString ptr("%p", mControlledDocument); - nsAutoCString suffix; - mOriginAttributes.CreateSuffix(suffix); nsAutoCString spec; Unused << mURI->GetSpec(spec); - hash = HashString(spec); - - hash = AddToHash(hash, HashString(suffix), HashString(mIsolationKey), - HashString(ptr), mAppType); - mHash.emplace(hash); + mHash.emplace(AddToHash(mPartitionPrincipal->GetHashValue(), HashString(spec), + mControlledDocument, mAppType, mCORSMode)); } /* static */ @@ -127,53 +107,6 @@ void* ImageCacheKey::GetSpecialCaseDocumentToken(Document* aDocument) { return nullptr; } -/* static */ -nsCString ImageCacheKey::GetIsolationKey(Document* aDocument, nsIURI* aURI) { - if (!aDocument || !aDocument->GetInnerWindow()) { - return ""_ns; - } - - // Network-state isolation - if (StaticPrefs::privacy_partition_network_state()) { - OriginAttributes oa; - StoragePrincipalHelper::GetOriginAttributesForNetworkState(aDocument, oa); - - nsAutoCString suffix; - oa.CreateSuffix(suffix); - - return std::move(suffix); - } - - // If the window is 3rd party resource, let's see if first-party storage - // access is granted for this image. - if (AntiTrackingUtils::IsThirdPartyWindow(aDocument->GetInnerWindow(), - nullptr)) { - uint32_t rejectedReason = 0; - Unused << rejectedReason; - return ShouldAllowAccessFor(aDocument->GetInnerWindow(), aURI, true, - &rejectedReason) - ? ""_ns - : aDocument->GetBaseDomain(); - } - - // Another scenario is if this image is a 3rd party resource loaded by a - // first party context. In this case, we should check if the nsIChannel has - // been marked as tracking resource, but we don't have the channel yet at - // this point. The best approach here is to be conservative: if we are sure - // that the permission is granted, let's return 0. Otherwise, let's make a - // unique image cache per the top-level document eTLD+1. - if (!ApproximateAllowAccessForWithoutChannel(aDocument->GetInnerWindow(), - aURI)) { - // If we are here, the image is a 3rd-party resource loaded by a first-party - // context. We can just use the document's base domain as the key because it - // should be the same as the top-level document's base domain. - return aDocument - ->GetBaseDomain(); // because we don't have anything better! - } - - return ""_ns; -} - /* static */ nsIDocShell::AppType ImageCacheKey::GetAppType(Document* aDocument) { if (!aDocument) { diff --git a/image/ImageCacheKey.h b/image/ImageCacheKey.h index b4910a9f86a1..5f55537db5cb 100644 --- a/image/ImageCacheKey.h +++ b/image/ImageCacheKey.h @@ -12,7 +12,6 @@ #include "mozilla/BasePrincipal.h" #include "mozilla/Maybe.h" -#include "mozilla/RefPtr.h" #include "PLDHashTable.h" #include "nsIDocShell.h" @@ -50,14 +49,11 @@ class ImageCacheKey final { /// A weak pointer to the URI. nsIURI* URI() const { return mURI; } + nsIPrincipal* PartitionPrincipal() const { return mPartitionPrincipal; } + nsIPrincipal* LoaderPrincipal() const { return mLoaderPrincipal; } + CORSMode GetCORSMode() const { return mCORSMode; } - const OriginAttributes& OriginAttributesRef() const { - return mOriginAttributes; - } - - const nsCString& IsolationKeyRef() const { return mIsolationKey; } - /// A token indicating which service worker controlled document this entry /// belongs to, if any. void* ControlledDocument() const { return mControlledDocument; } @@ -67,11 +63,6 @@ class ImageCacheKey final { // token for the key. All those exceptions are handled by this method. static void* GetSpecialCaseDocumentToken(dom::Document* aDocument); - // For anti-tracking we need to use an isolation key. It can be the suffix of - // the PatitionedPrincipal (see StoragePrincipalHelper.h) or the top-level - // document's base domain. This is handled by this method. - static nsCString GetIsolationKey(dom::Document* aDocument, nsIURI* aURI); - // The AppType of the docshell an image is loaded in can influence whether the // image is allowed to load. The specific AppType is fetched by this method. static nsIDocShell::AppType GetAppType(dom::Document* aDocument); @@ -79,9 +70,9 @@ class ImageCacheKey final { void EnsureHash() const; nsCOMPtr mURI; - const OriginAttributes mOriginAttributes; void* mControlledDocument; - nsCString mIsolationKey; + nsCOMPtr mLoaderPrincipal; + nsCOMPtr mPartitionPrincipal; mutable Maybe mHash; const CORSMode mCORSMode; const nsIDocShell::AppType mAppType; diff --git a/image/imgLoader.cpp b/image/imgLoader.cpp index d4c69cfafbab..5f7437f31b6b 100644 --- a/image/imgLoader.cpp +++ b/image/imgLoader.cpp @@ -1376,8 +1376,7 @@ imgLoader::ClearCache(JS::Handle chrome) { return ClearCache(chrome.isBoolean() ? Some(chrome.toBoolean()) : Nothing()); } -nsresult -imgLoader::ClearCache(mozilla::Maybe chrome) { +nsresult imgLoader::ClearCache(mozilla::Maybe chrome) { if (XRE_IsParentProcess()) { bool privateLoader = this == gPrivateBrowsingLoader; for (auto* cp : ContentParent::AllProcesses(ContentParent::eLive)) { @@ -1447,90 +1446,46 @@ nsresult imgLoader::RemoveEntriesInternal( return NS_ERROR_INVALID_ARG; } - nsCOMPtr tldService; AutoTArray, 128> entriesToBeRemoved; - Maybe patternWithPartitionKey = Nothing(); - if (aPattern) { - // Used for checking for cache entries partitioned under aSchemelessSite. - OriginAttributesPattern pattern(aPattern.ref()); - pattern.mPartitionKeyPattern.Construct(); - pattern.mPartitionKeyPattern.Value().mBaseDomain.Construct( - NS_ConvertUTF8toUTF16(aSchemelessSite.ref())); - - patternWithPartitionKey.emplace(std::move(pattern)); - } - // For base domain we only clear the non-chrome cache. for (const auto& entry : mCache) { const auto& key = entry.GetKey(); + // TODO(emilio): Deduplicate this with SharedSubresourceCache. const bool shouldRemove = [&] { - // The isolation key is either just the site, or an origin suffix - // which contains the partitionKey holding the baseDomain. - if (aPrincipal) { - nsCOMPtr keyPrincipal = - BasePrincipal::CreateContentPrincipal(key.URI(), - key.OriginAttributesRef()); - return keyPrincipal->Equals(aPrincipal.ref()); + return key.LoaderPrincipal()->Equals(aPrincipal.ref()); } if (!aSchemelessSite) { return false; } - // Clear by site and pattern. - nsAutoCString host; - nsresult rv = key.URI()->GetHost(host); - if (NS_FAILED(rv) || host.IsEmpty()) { - return false; - } - - if (!tldService) { - tldService = do_GetService(NS_EFFECTIVETLDSERVICE_CONTRACTID); - } - if (NS_WARN_IF(!tldService)) { - return false; - } - - bool hasRootDomain = false; - rv = tldService->HasRootDomain(host, aSchemelessSite.ref(), - &hasRootDomain); + // Clear by site + nsIPrincipal* partitionPrincipal = key.PartitionPrincipal(); + nsAutoCString principalBaseDomain; + nsresult rv = partitionPrincipal->GetBaseDomain(principalBaseDomain); if (NS_WARN_IF(NS_FAILED(rv))) { return false; } - if (hasRootDomain && aPattern->Matches(key.OriginAttributesRef())) { + if (principalBaseDomain.Equals(aSchemelessSite.ref()) && + aPattern.ref().Matches(partitionPrincipal->OriginAttributesRef())) { return true; } - // Attempt to parse isolation key into origin attributes. - Maybe originAttributesWithPartitionKey; - { - OriginAttributes attrs; - if (attrs.PopulateFromSuffix(key.IsolationKeyRef())) { - OriginAttributes attrsWithPartitionKey(key.OriginAttributesRef()); - attrsWithPartitionKey.mPartitionKey = attrs.mPartitionKey; - originAttributesWithPartitionKey.emplace( - std::move(attrsWithPartitionKey)); - } - } + // Clear entries partitioned under aSchemelessSite. We need to add the + // partition key filter to aPattern so that we include any OA filtering + // specified by the caller. For example the caller may pass aPattern = { + // privateBrowsingId: 1 } which means we may only clear partitioned + // private browsing data. + OriginAttributesPattern patternWithPartitionKey(aPattern.ref()); + patternWithPartitionKey.mPartitionKeyPattern.Construct(); + patternWithPartitionKey.mPartitionKeyPattern.Value() + .mBaseDomain.Construct(NS_ConvertUTF8toUTF16(aSchemelessSite.ref())); - // Match it against the pattern that contains the partition key and any - // fields set by the caller pattern. - if (originAttributesWithPartitionKey.isSome()) { - nsAutoCString oaSuffixForPrinting; - originAttributesWithPartitionKey->CreateSuffix(oaSuffixForPrinting); - - nsAutoString patternForPrinting; - patternWithPartitionKey->ToJSON(patternForPrinting); - - return patternWithPartitionKey.ref().Matches( - originAttributesWithPartitionKey.ref()); - } - - // The isolation key is the site. - return aSchemelessSite->Equals(key.IsolationKeyRef()); + return patternWithPartitionKey.Matches( + partitionPrincipal->OriginAttributesRef()); }(); if (shouldRemove) { diff --git a/layout/style/Loader.cpp b/layout/style/Loader.cpp index 070e8889756f..6111bd8855e5 100644 --- a/layout/style/Loader.cpp +++ b/layout/style/Loader.cpp @@ -144,7 +144,7 @@ namespace mozilla { SheetLoadDataHashKey::SheetLoadDataHashKey(const css::SheetLoadData& aLoadData) : mURI(aLoadData.mURI), - mPrincipal(aLoadData.mTriggeringPrincipal), + mTriggeringPrincipal(aLoadData.mTriggeringPrincipal), mLoaderPrincipal(aLoadData.mLoader->LoaderPrincipal()), mPartitionPrincipal(aLoadData.mLoader->PartitionedPrincipal()), mEncodingGuess(aLoadData.mGuessedEncoding), @@ -154,7 +154,7 @@ SheetLoadDataHashKey::SheetLoadDataHashKey(const css::SheetLoadData& aLoadData) mIsLinkRelPreloadOrEarlyHint(aLoadData.IsLinkRelPreloadOrEarlyHint()) { MOZ_COUNT_CTOR(SheetLoadDataHashKey); MOZ_ASSERT(mURI); - MOZ_ASSERT(mPrincipal); + MOZ_ASSERT(mTriggeringPrincipal); MOZ_ASSERT(mLoaderPrincipal); MOZ_ASSERT(mPartitionPrincipal); aLoadData.mSheet->GetIntegrity(mSRIMetadata); @@ -180,16 +180,20 @@ bool SheetLoadDataHashKey::KeyEquals(const SheetLoadDataHashKey& aKey) const { return true; } - if (!mPrincipal->Equals(aKey.mPrincipal)) { - LOG((" > Principal mismatch\n")); + if (!mTriggeringPrincipal->Equals(aKey.mTriggeringPrincipal)) { + LOG((" > Triggering principal mismatch\n")); return false; } // We only check for partition principal equality if any of the loads are // triggered by a document rather than e.g. an extension (which have different // origins than the loader principal). - if (mPrincipal->Equals(mLoaderPrincipal) || - aKey.mPrincipal->Equals(aKey.mLoaderPrincipal)) { + // + // TODO(emilio): The idea of this code is allowing sharing extension + // stylesheets on different origins, see bug 1645987 comment 5, but I think + // this is less important (maybe completely irrelevant?) with Fission. + if (mTriggeringPrincipal->Equals(mLoaderPrincipal) || + aKey.mTriggeringPrincipal->Equals(aKey.mLoaderPrincipal)) { if (!mPartitionPrincipal->Equals(aKey.mPartitionPrincipal)) { LOG((" > Partition principal mismatch\n")); return false; diff --git a/layout/style/Loader.h b/layout/style/Loader.h index 5414634903ca..39523f2ead5d 100644 --- a/layout/style/Loader.h +++ b/layout/style/Loader.h @@ -55,7 +55,7 @@ class SheetLoadDataHashKey : public PLDHashEntryHdr { explicit SheetLoadDataHashKey(const SheetLoadDataHashKey* aKey) : mURI(aKey->mURI), - mPrincipal(aKey->mPrincipal), + mTriggeringPrincipal(aKey->mTriggeringPrincipal), mLoaderPrincipal(aKey->mLoaderPrincipal), mPartitionPrincipal(aKey->mPartitionPrincipal), mEncodingGuess(aKey->mEncodingGuess), @@ -76,7 +76,7 @@ class SheetLoadDataHashKey : public PLDHashEntryHdr { const dom::SRIMetadata& aSRIMetadata, css::StylePreloadKind aPreloadKind) : mURI(aURI), - mPrincipal(aPrincipal), + mTriggeringPrincipal(aPrincipal), mLoaderPrincipal(aLoaderPrincipal), mPartitionPrincipal(aPartitionPrincipal), mEncodingGuess(aEncodingGuess), @@ -94,7 +94,7 @@ class SheetLoadDataHashKey : public PLDHashEntryHdr { SheetLoadDataHashKey(SheetLoadDataHashKey&& toMove) : mURI(std::move(toMove.mURI)), - mPrincipal(std::move(toMove.mPrincipal)), + mTriggeringPrincipal(std::move(toMove.mTriggeringPrincipal)), mLoaderPrincipal(std::move(toMove.mLoaderPrincipal)), mPartitionPrincipal(std::move(toMove.mPartitionPrincipal)), mEncodingGuess(std::move(toMove.mEncodingGuess)), @@ -130,7 +130,7 @@ class SheetLoadDataHashKey : public PLDHashEntryHdr { nsIURI* URI() const { return mURI; } - nsIPrincipal* Principal() const { return mPrincipal; } + nsIPrincipal* TriggeringPrincipal() const { return mTriggeringPrincipal; } nsIPrincipal* LoaderPrincipal() const { return mLoaderPrincipal; } @@ -142,7 +142,7 @@ class SheetLoadDataHashKey : public PLDHashEntryHdr { protected: const nsCOMPtr mURI; - const nsCOMPtr mPrincipal; + const nsCOMPtr mTriggeringPrincipal; const nsCOMPtr mLoaderPrincipal; const nsCOMPtr mPartitionPrincipal; // The encoding guess is the encoding the sheet would get if the request diff --git a/layout/style/SharedSubResourceCache.h b/layout/style/SharedSubResourceCache.h index c128cdf0f077..adee2e252dd7 100644 --- a/layout/style/SharedSubResourceCache.h +++ b/layout/style/SharedSubResourceCache.h @@ -295,7 +295,10 @@ void SharedSubResourceCache::ClearInProcess( } } - if (aPrincipal && iter.Key().Principal()->Equals(aPrincipal.ref())) { + if (aPrincipal && + iter.Key().TriggeringPrincipal()->Equals(aPrincipal.ref())) { + // FIXME(emilio): I don't think we want the triggering principal, but + // the loader principal (or the partition principal), most likely? return true; } if (!aSchemelessSite) {