diff options
Diffstat (limited to 'dom/quota')
-rw-r--r-- | dom/quota/ActorsParent.cpp | 39 | ||||
-rw-r--r-- | dom/quota/CachingDatabaseConnection.cpp | 12 | ||||
-rw-r--r-- | dom/quota/CachingDatabaseConnection.h | 12 | ||||
-rw-r--r-- | dom/quota/Config.h | 10 | ||||
-rw-r--r-- | dom/quota/OriginParser.cpp | 6 | ||||
-rw-r--r-- | dom/quota/OriginParser.h | 2 | ||||
-rw-r--r-- | dom/quota/QuotaCommon.cpp | 98 | ||||
-rw-r--r-- | dom/quota/QuotaCommon.h | 4 | ||||
-rw-r--r-- | dom/quota/QuotaManager.h | 3 | ||||
-rw-r--r-- | dom/quota/ScopedLogExtraInfo.cpp | 37 | ||||
-rw-r--r-- | dom/quota/ScopedLogExtraInfo.h | 21 | ||||
-rw-r--r-- | dom/quota/SerializationHelpers.h | 2 | ||||
-rw-r--r-- | dom/quota/StorageOriginAttributes.cpp | 50 | ||||
-rw-r--r-- | dom/quota/StorageOriginAttributes.h | 22 | ||||
-rw-r--r-- | dom/quota/test/gtest/TestScopedLogExtraInfo.cpp | 31 | ||||
-rw-r--r-- | dom/quota/test/gtest/TestStorageOriginAttributes.cpp | 75 | ||||
-rw-r--r-- | dom/quota/test/modules/system/worker/ModuleLoader.js | 2 | ||||
-rw-r--r-- | dom/quota/test/xpcshell/telemetry/test_dom_quota_try.js | 180 | ||||
-rw-r--r-- | dom/quota/test/xpcshell/telemetry/xpcshell.toml | 3 |
19 files changed, 509 insertions, 100 deletions
diff --git a/dom/quota/ActorsParent.cpp b/dom/quota/ActorsParent.cpp index 3f8a1a316b..451d55a450 100644 --- a/dom/quota/ActorsParent.cpp +++ b/dom/quota/ActorsParent.cpp @@ -82,6 +82,7 @@ #include "mozilla/Variant.h" #include "mozilla/dom/FileSystemQuotaClientFactory.h" #include "mozilla/dom/FlippedOnce.h" +#include "mozilla/dom/IndexedDatabaseManager.h" #include "mozilla/dom/LocalStorageCommon.h" #include "mozilla/dom/StorageDBUpdater.h" #include "mozilla/dom/cache/QuotaClient.h" @@ -108,6 +109,7 @@ #include "mozilla/ipc/PBackgroundSharedTypes.h" #include "mozilla/ipc/ProtocolUtils.h" #include "mozilla/net/ExtensionProtocolHandler.h" +#include "mozilla/StorageOriginAttributes.h" #include "nsAppDirectoryServiceDefs.h" #include "nsBaseHashtable.h" #include "nsCOMPtr.h" @@ -1377,6 +1379,9 @@ void InitializeQuotaManager() { RefPtr<net::ExtensionProtocolHandler> extensionProtocolHandler = net::ExtensionProtocolHandler::GetSingleton(); QM_WARNONLY_TRY(MOZ_TO_RESULT(extensionProtocolHandler)); + + IndexedDatabaseManager* mgr = IndexedDatabaseManager::GetOrCreate(); + QM_WARNONLY_TRY(MOZ_TO_RESULT(mgr)); } QM_WARNONLY_TRY(QM_TO_RESULT(QuotaManager::Initialize())); @@ -2267,7 +2272,7 @@ void QuotaManager::Shutdown() { quotaManager->mQuotaManagerShutdownSteps.get()); } - CrashReporter::AnnotateCrashReport( + CrashReporter::RecordAnnotationNSCString( CrashReporter::Annotation::QuotaManagerShutdownTimeout, annotation); MOZ_CRASH("Quota manager shutdown timed out"); @@ -2372,7 +2377,7 @@ void QuotaManager::Shutdown() { // Body of the function - ScopedLogExtraInfo scope{ScopedLogExtraInfo::kTagContext, + ScopedLogExtraInfo scope{ScopedLogExtraInfo::kTagContextTainted, "dom::quota::QuotaManager::Shutdown"_ns}; // This must be called before `flagShutdownStarted`, it would fail otherwise. @@ -2707,6 +2712,10 @@ nsresult QuotaManager::LoadQuota() { fullOriginMetadata.mStorageOrigin = fullOriginMetadata.mOrigin; + const auto extraInfo = + ScopedLogExtraInfo{ScopedLogExtraInfo::kTagStorageOriginTainted, + fullOriginMetadata.mStorageOrigin}; + fullOriginMetadata.mIsPrivate = false; QM_TRY_INSPECT(const auto& clientUsagesText, @@ -3203,7 +3212,7 @@ nsresult QuotaManager::CreateDirectoryMetadata( const OriginMetadata& aOriginMetadata) { AssertIsOnIOThread(); - OriginAttributes groupAttributes; + StorageOriginAttributes groupAttributes; nsCString groupNoSuffix; QM_TRY(OkIf(groupAttributes.PopulateFromOrigin(aOriginMetadata.mGroup, @@ -3211,11 +3220,11 @@ nsresult QuotaManager::CreateDirectoryMetadata( NS_ERROR_FAILURE); nsCString groupPrefix; - GetJarPrefix(groupAttributes.mInIsolatedMozBrowser, groupPrefix); + GetJarPrefix(groupAttributes.InIsolatedMozBrowser(), groupPrefix); nsCString group = groupPrefix + groupNoSuffix; - OriginAttributes originAttributes; + StorageOriginAttributes originAttributes; nsCString originNoSuffix; QM_TRY(OkIf(originAttributes.PopulateFromOrigin(aOriginMetadata.mOrigin, @@ -3223,7 +3232,7 @@ nsresult QuotaManager::CreateDirectoryMetadata( NS_ERROR_FAILURE); nsCString originPrefix; - GetJarPrefix(originAttributes.mInIsolatedMozBrowser, originPrefix); + GetJarPrefix(originAttributes.InIsolatedMozBrowser(), originPrefix); nsCString origin = originPrefix + originNoSuffix; @@ -3588,6 +3597,10 @@ nsresult QuotaManager::InitializeRepository(PersistenceType aPersistenceType, MOZ_ASSERT(metadata.mPersistenceType == aPersistenceType); + const auto extraInfo = ScopedLogExtraInfo{ + ScopedLogExtraInfo::kTagStorageOriginTainted, + metadata.mStorageOrigin}; + // FIXME(tt): The check for origin name consistency can // be removed once we have an upgrade to traverse origin // directories and check through the directory metadata @@ -3679,6 +3692,10 @@ nsresult QuotaManager::InitializeRepository(PersistenceType aPersistenceType, QM_TRY(([&]() -> Result<Ok, nsresult> { QM_TRY(([&directory, &info, this, aPersistenceType, &aOriginFunc]() -> Result<Ok, nsresult> { + const auto extraInfo = ScopedLogExtraInfo{ + ScopedLogExtraInfo::kTagStorageOriginTainted, + info.mFullOriginMetadata.mStorageOrigin}; + const auto originDirName = MakeSanitizedOriginString(info.mFullOriginMetadata.mOrigin); @@ -3728,6 +3745,10 @@ nsresult QuotaManager::InitializeOrigin(PersistenceType aPersistenceType, nsIFile* aDirectory) { AssertIsOnIOThread(); + // The ScopedLogExtraInfo is not set here on purpose, so the callers can + // decide if they want to set it. The extra info can be set sooner this way + // as well. + const bool trackQuota = aPersistenceType != PERSISTENCE_TYPE_PERSISTENT; // We need to initialize directories of all clients if they exists and also @@ -5211,6 +5232,10 @@ QuotaManager::EnsurePersistentOriginIsInitialized( const auto innerFunc = [&aOriginMetadata, this](const auto& firstInitializationAttempt) -> mozilla::Result<std::pair<nsCOMPtr<nsIFile>, bool>, nsresult> { + const auto extraInfo = + ScopedLogExtraInfo{ScopedLogExtraInfo::kTagStorageOriginTainted, + aOriginMetadata.mStorageOrigin}; + QM_TRY_UNWRAP(auto directory, GetOriginDirectory(aOriginMetadata)); if (mInitializedOrigins.Contains(aOriginMetadata.mOrigin)) { @@ -7612,7 +7637,7 @@ Result<bool, nsresult> UpgradeStorageFrom1_0To2_0Helper::MaybeRemoveAppsData( MOZ_ASSERT(originalSuffix[0] == '^'); if (!URLParams::Parse( - Substring(originalSuffix, 1, originalSuffix.Length() - 1), + Substring(originalSuffix, 1, originalSuffix.Length() - 1), true, [](const nsAString& aName, const nsAString& aValue) { if (aName.EqualsLiteral("appId")) { return false; diff --git a/dom/quota/CachingDatabaseConnection.cpp b/dom/quota/CachingDatabaseConnection.cpp index 504c0b5ded..dfdf0f71da 100644 --- a/dom/quota/CachingDatabaseConnection.cpp +++ b/dom/quota/CachingDatabaseConnection.cpp @@ -14,16 +14,16 @@ namespace mozilla::dom::quota { CachingDatabaseConnection::CachingDatabaseConnection( MovingNotNull<nsCOMPtr<mozIStorageConnection>> aStorageConnection) : -#ifdef CACHING_DB_CONNECTION_CHECK_THREAD_OWNERSHIP - mOwningThread{nsAutoOwningThread{}}, +#ifdef MOZ_THREAD_SAFETY_OWNERSHIP_CHECKS_SUPPORTED + mOwningEventTarget{nsAutoOwningEventTarget{}}, #endif mStorageConnection(std::move(aStorageConnection)) { } void CachingDatabaseConnection::LazyInit( MovingNotNull<nsCOMPtr<mozIStorageConnection>> aStorageConnection) { -#ifdef CACHING_DB_CONNECTION_CHECK_THREAD_OWNERSHIP - mOwningThread.init(); +#ifdef MOZ_THREAD_SAFETY_OWNERSHIP_CHECKS_SUPPORTED + mOwningEventTarget.init(); #endif mStorageConnection.init(std::move(aStorageConnection)); } @@ -40,8 +40,8 @@ CachingDatabaseConnection::GetCachedStatement(const nsACString& aQuery) { auto stmt, mCachedStatements.TryLookupOrInsertWith( aQuery, [&]() -> Result<nsCOMPtr<mozIStorageStatement>, nsresult> { - const auto extraInfo = - ScopedLogExtraInfo{ScopedLogExtraInfo::kTagQuery, aQuery}; + const auto extraInfo = ScopedLogExtraInfo{ + ScopedLogExtraInfo::kTagQueryTainted, aQuery}; QM_TRY_RETURN( MOZ_TO_RESULT_INVOKE_MEMBER_TYPED( diff --git a/dom/quota/CachingDatabaseConnection.h b/dom/quota/CachingDatabaseConnection.h index ea94035a34..c9d7dbdd6b 100644 --- a/dom/quota/CachingDatabaseConnection.h +++ b/dom/quota/CachingDatabaseConnection.h @@ -14,6 +14,7 @@ #include "nscore.h" #include "nsHashKeys.h" #include "nsInterfaceHashtable.h" +#include "nsISupportsImpl.h" #include "nsString.h" #include "mozilla/Assertions.h" #include "mozilla/Attributes.h" @@ -55,7 +56,7 @@ class CachingDatabaseConnection { BorrowedStatement(NotNull<mozIStorageStatement*> aStatement, const nsACString& aQuery) : mozStorageStatementScoper(aStatement), - mExtraInfo{ScopedLogExtraInfo::kTagQuery, aQuery} {} + mExtraInfo{ScopedLogExtraInfo::kTagQueryTainted, aQuery} {} ScopedLogExtraInfo mExtraInfo; #else @@ -67,8 +68,9 @@ class CachingDatabaseConnection { class LazyStatement; void AssertIsOnConnectionThread() const { -#ifdef CACHING_DB_CONNECTION_CHECK_THREAD_OWNERSHIP - mOwningThread->AssertOwnership("CachingDatabaseConnection not thread-safe"); +#ifdef MOZ_THREAD_SAFETY_OWNERSHIP_CHECKS_SUPPORTED + mOwningEventTarget->AssertOwnership( + "CachingDatabaseConnection not thread-safe"); #endif } @@ -125,8 +127,8 @@ class CachingDatabaseConnection { void Close(); private: -#ifdef CACHING_DB_CONNECTION_CHECK_THREAD_OWNERSHIP - LazyInitializedOnce<const nsAutoOwningThread> mOwningThread; +#ifdef MOZ_THREAD_SAFETY_OWNERSHIP_CHECKS_SUPPORTED + LazyInitializedOnce<const nsAutoOwningEventTarget> mOwningEventTarget; #endif LazyInitializedOnceEarlyDestructible< diff --git a/dom/quota/Config.h b/dom/quota/Config.h index 3ec16ab95d..10025883e1 100644 --- a/dom/quota/Config.h +++ b/dom/quota/Config.h @@ -33,14 +33,4 @@ # define QM_COLLECTING_OPERATION_TELEMETRY #endif -/** - * The thread ownership checks in CachingDatabaseConnection assumes that the - * object lives on a single thread, not any serial event target. - * Defining CACHING_DB_CONNECTION_CHECK_THREAD_OWNERSHIP restores the checks. - * See bug 1858989. - */ -#if 0 -# define CACHING_DB_CONNECTION_CHECK_THREAD_OWNERSHIP 1 -#endif - #endif // DOM_QUOTA_CONFIG_H_ diff --git a/dom/quota/OriginParser.cpp b/dom/quota/OriginParser.cpp index 0335b1e2c0..96e2da56fc 100644 --- a/dom/quota/OriginParser.cpp +++ b/dom/quota/OriginParser.cpp @@ -216,11 +216,7 @@ void OriginParser::HandleToken(const nsDependentCSubstring& aToken) { return; } - if (aToken.First() == 't') { - mInIsolatedMozBrowser = true; - } else if (aToken.First() == 'f') { - mInIsolatedMozBrowser = false; - } else { + if ((aToken.First() != 't') && (aToken.First() != 'f')) { QM_WARNING("'%s' is not a valid value for the inMozBrowser flag!", nsCString(aToken).get()); diff --git a/dom/quota/OriginParser.h b/dom/quota/OriginParser.h index cc4d0d1a11..f4fecf1c0a 100644 --- a/dom/quota/OriginParser.h +++ b/dom/quota/OriginParser.h @@ -65,7 +65,6 @@ class MOZ_STACK_CLASS OriginParser final { SchemeType mSchemeType; State mState; - bool mInIsolatedMozBrowser; bool mUniversalFileOrigin; bool mMaybeDriveLetter; bool mError; @@ -80,7 +79,6 @@ class MOZ_STACK_CLASS OriginParser final { mTokenizer(aOrigin, '+'), mSchemeType(eNone), mState(eExpectingAppIdOrScheme), - mInIsolatedMozBrowser(false), mUniversalFileOrigin(false), mMaybeDriveLetter(false), mError(false), diff --git a/dom/quota/QuotaCommon.cpp b/dom/quota/QuotaCommon.cpp index e2df8a1082..71b6186d00 100644 --- a/dom/quota/QuotaCommon.cpp +++ b/dom/quota/QuotaCommon.cpp @@ -374,14 +374,15 @@ void LogError(const nsACString& aExpr, const Maybe<nsresult> aMaybeRv, return; } - nsAutoCString context; + const Tainted<nsCString>* contextTaintedPtr = nullptr; # ifdef QM_SCOPED_LOG_EXTRA_INFO_ENABLED const auto& extraInfoMap = ScopedLogExtraInfo::GetExtraInfoMap(); - if (const auto contextIt = extraInfoMap.find(ScopedLogExtraInfo::kTagContext); + if (const auto contextIt = + extraInfoMap.find(ScopedLogExtraInfo::kTagContextTainted); contextIt != extraInfoMap.cend()) { - context = *contextIt->second; + contextTaintedPtr = contextIt->second; } # endif @@ -444,37 +445,46 @@ void LogError(const nsACString& aExpr, const Maybe<nsresult> aMaybeRv, } # endif - nsAutoCString extraInfosString; + auto extraInfosStringTainted = Tainted<nsAutoCString>([&] { + nsAutoCString extraInfosString; - if (!rvCode.IsEmpty()) { - extraInfosString.Append(" failed with resultCode "_ns + rvCode); - } + if (!rvCode.IsEmpty()) { + extraInfosString.Append(" failed with resultCode "_ns + rvCode); + } - if (!rvName.IsEmpty()) { - extraInfosString.Append(", resultName "_ns + rvName); - } + if (!rvName.IsEmpty()) { + extraInfosString.Append(", resultName "_ns + rvName); + } # ifdef QM_ERROR_STACKS_ENABLED - if (!frameIdString.IsEmpty()) { - extraInfosString.Append(", frameId "_ns + frameIdString); - } + if (!frameIdString.IsEmpty()) { + extraInfosString.Append(", frameId "_ns + frameIdString); + } - if (!stackIdString.IsEmpty()) { - extraInfosString.Append(", stackId "_ns + stackIdString); - } + if (!stackIdString.IsEmpty()) { + extraInfosString.Append(", stackId "_ns + stackIdString); + } - if (!processIdString.IsEmpty()) { - extraInfosString.Append(", processId "_ns + processIdString); - } + if (!processIdString.IsEmpty()) { + extraInfosString.Append(", processId "_ns + processIdString); + } # endif # ifdef QM_SCOPED_LOG_EXTRA_INFO_ENABLED - for (const auto& item : extraInfoMap) { - extraInfosString.Append(", "_ns + nsDependentCString(item.first) + " "_ns + - *item.second); - } + for (const auto& item : extraInfoMap) { + const auto& valueTainted = *item.second; + + extraInfosString.Append( + ", "_ns + nsDependentCString(item.first) + " "_ns + + MOZ_NO_VALIDATE(valueTainted, + "It's okay to append any `extraInfoMap` value to " + "`extraInfosString`.")); + } # endif + return extraInfosString; + }()); + const auto sourceFileRelativePath = detail::MakeSourceFileRelativePath(aSourceFilePath); @@ -482,9 +492,14 @@ void LogError(const nsACString& aExpr, const Maybe<nsresult> aMaybeRv, NS_DebugBreak( NS_DEBUG_WARNING, nsAutoCString("QM_TRY failure ("_ns + severityString + ")"_ns).get(), - (extraInfosString.IsEmpty() ? nsPromiseFlatCString(aExpr) - : static_cast<const nsCString&>(nsAutoCString( - aExpr + extraInfosString))) + (MOZ_NO_VALIDATE(extraInfosStringTainted, + "It's okay to check if `extraInfosString` is empty.") + .IsEmpty() + ? nsPromiseFlatCString(aExpr) + : static_cast<const nsCString&>(nsAutoCString( + aExpr + MOZ_NO_VALIDATE(extraInfosStringTainted, + "It's okay to log `extraInfosString` " + "to stdout/console.")))) .get(), nsPromiseFlatCString(sourceFileRelativePath).get(), aSourceFileLine); # endif @@ -496,13 +511,16 @@ void LogError(const nsACString& aExpr, const Maybe<nsresult> aMaybeRv, // reporting (instead of the browsing console). // Another option is to keep the current check and rely on MOZ_LOG reporting // in future once that's available. - if (!context.IsEmpty()) { + if (contextTaintedPtr) { nsCOMPtr<nsIConsoleService> console = do_GetService(NS_CONSOLESERVICE_CONTRACTID); if (console) { NS_ConvertUTF8toUTF16 message( "QM_TRY failure ("_ns + severityString + ")"_ns + ": '"_ns + aExpr + - extraInfosString + "', file "_ns + sourceFileRelativePath + ":"_ns + + MOZ_NO_VALIDATE( + extraInfosStringTainted, + "It's okay to log `extraInfosString` to the browser console.") + + "', file "_ns + sourceFileRelativePath + ":"_ns + IntToCString(aSourceFileLine)); // The concatenation above results in a message like: @@ -517,14 +535,34 @@ void LogError(const nsACString& aExpr, const Maybe<nsresult> aMaybeRv, # endif # ifdef QM_LOG_ERROR_TO_TELEMETRY_ENABLED - if (!context.IsEmpty()) { + // The context tag is special because it's used to enable logging to + // telemetry (besides carrying information). Other tags (like query) don't + // enable logging to telemetry. + + if (contextTaintedPtr) { + const auto& contextTainted = *contextTaintedPtr; + + // Do NOT CHANGE this if you don't know what you're doing. + + // `extraInfoString` is not included in the telemetry event on purpose + // since it can contain sensitive information. + // For now, we don't include aExpr in the telemetry event. It might help to // match locations across versions, but they might be large. + + // New extra entries (with potentially sensitive content) can't be easily + // (accidentally) added because they would have to be added to Events.yaml + // under "dom.quota.try" which would require a data review. + auto extra = Some([&] { auto res = CopyableTArray<EventExtraEntry>{}; res.SetCapacity(9); - res.AppendElement(EventExtraEntry{"context"_ns, nsCString{context}}); + res.AppendElement(EventExtraEntry{ + "context"_ns, + MOZ_NO_VALIDATE( + contextTainted, + "Context has been data-reviewed for telemetry transmission.")}); # ifdef QM_ERROR_STACKS_ENABLED if (!frameIdString.IsEmpty()) { diff --git a/dom/quota/QuotaCommon.h b/dom/quota/QuotaCommon.h index b672150b96..74855dd16b 100644 --- a/dom/quota/QuotaCommon.h +++ b/dom/quota/QuotaCommon.h @@ -1712,8 +1712,8 @@ auto ExecuteInitialization( const auto maybeScopedLogExtraInfo = firstInitializationAttempt.Recorded() ? Nothing{} - : Some(ScopedLogExtraInfo{ScopedLogExtraInfo::kTagContext, - aContext}); + : Some(ScopedLogExtraInfo{ + ScopedLogExtraInfo::kTagContextTainted, aContext}); #endif return std::forward<Func>(aFunc)(firstInitializationAttempt); diff --git a/dom/quota/QuotaManager.h b/dom/quota/QuotaManager.h index 42e5de961a..354977166a 100644 --- a/dom/quota/QuotaManager.h +++ b/dom/quota/QuotaManager.h @@ -727,7 +727,8 @@ class QuotaManager final : public BackgroundThreadObject { nsCOMPtr<mozIStorageConnection> mStorageConnection; - EnumeratedArray<Client::Type, Client::TYPE_MAX, nsCString> mShutdownSteps; + EnumeratedArray<Client::Type, nsCString, size_t(Client::TYPE_MAX)> + mShutdownSteps; LazyInitializedOnce<const TimeStamp> mShutdownStartedAt; // Accesses to mQuotaManagerShutdownSteps must be protected by mQuotaMutex. diff --git a/dom/quota/ScopedLogExtraInfo.cpp b/dom/quota/ScopedLogExtraInfo.cpp index e9ddf84a8d..1c38c58f88 100644 --- a/dom/quota/ScopedLogExtraInfo.cpp +++ b/dom/quota/ScopedLogExtraInfo.cpp @@ -9,19 +9,27 @@ namespace mozilla::dom::quota { #ifdef QM_SCOPED_LOG_EXTRA_INFO_ENABLED -MOZ_THREAD_LOCAL(const nsACString*) ScopedLogExtraInfo::sQueryValue; -MOZ_THREAD_LOCAL(const nsACString*) ScopedLogExtraInfo::sContextValue; +MOZ_THREAD_LOCAL(const Tainted<nsCString>*) +ScopedLogExtraInfo::sQueryValueTainted; +MOZ_THREAD_LOCAL(const Tainted<nsCString>*) +ScopedLogExtraInfo::sContextValueTainted; +MOZ_THREAD_LOCAL(const Tainted<nsCString>*) +ScopedLogExtraInfo::sStorageOriginValueTainted; /* static */ auto ScopedLogExtraInfo::FindSlot(const char* aTag) { // XXX For now, don't use a real map but just allow the known tag values. - if (aTag == kTagQuery) { - return &sQueryValue; + if (aTag == kTagQueryTainted) { + return &sQueryValueTainted; } - if (aTag == kTagContext) { - return &sContextValue; + if (aTag == kTagContextTainted) { + return &sContextValueTainted; + } + + if (aTag == kTagStorageOriginTainted) { + return &sStorageOriginValueTainted; } MOZ_CRASH("Unknown tag!"); @@ -51,20 +59,25 @@ ScopedLogExtraInfo::GetExtraInfoMap() { // the caller(s). ScopedLogExtraInfoMap map; - if (sQueryValue.get()) { - map.emplace(kTagQuery, sQueryValue.get()); + if (sQueryValueTainted.get()) { + map.emplace(kTagQueryTainted, sQueryValueTainted.get()); + } + + if (sContextValueTainted.get()) { + map.emplace(kTagContextTainted, sContextValueTainted.get()); } - if (sContextValue.get()) { - map.emplace(kTagContext, sContextValue.get()); + if (sStorageOriginValueTainted.get()) { + map.emplace(kTagStorageOriginTainted, sStorageOriginValueTainted.get()); } return map; } /* static */ void ScopedLogExtraInfo::Initialize() { - MOZ_ALWAYS_TRUE(sQueryValue.init()); - MOZ_ALWAYS_TRUE(sContextValue.init()); + MOZ_ALWAYS_TRUE(sQueryValueTainted.init()); + MOZ_ALWAYS_TRUE(sContextValueTainted.init()); + MOZ_ALWAYS_TRUE(sStorageOriginValueTainted.init()); } void ScopedLogExtraInfo::AddInfo() { diff --git a/dom/quota/ScopedLogExtraInfo.h b/dom/quota/ScopedLogExtraInfo.h index 54b8bccb3f..5dc881a3b1 100644 --- a/dom/quota/ScopedLogExtraInfo.h +++ b/dom/quota/ScopedLogExtraInfo.h @@ -12,6 +12,7 @@ #include <map> #include "mozilla/Assertions.h" #include "mozilla/Attributes.h" +#include "mozilla/Tainting.h" #include "mozilla/ThreadLocal.h" #include "nsString.h" #include "nsXULAppAPI.h" @@ -19,8 +20,12 @@ namespace mozilla::dom::quota { struct MOZ_STACK_CLASS ScopedLogExtraInfo { - static constexpr const char kTagQuery[] = "query"; - static constexpr const char kTagContext[] = "context"; + static constexpr const char kTagQueryTainted[] = "query"; + static constexpr const char kTagContextTainted[] = "context"; + // Using the storage origin (instead of normal origin) on purpose to store + // the masked origin (uuid based) on the stack for origins partitioned for + // private browsing. + static constexpr const char kTagStorageOriginTainted[] = "storage-origin"; #ifdef QM_SCOPED_LOG_EXTRA_INFO_ENABLED private: @@ -41,18 +46,20 @@ struct MOZ_STACK_CLASS ScopedLogExtraInfo { ScopedLogExtraInfo(const ScopedLogExtraInfo&) = delete; ScopedLogExtraInfo& operator=(const ScopedLogExtraInfo&) = delete; - using ScopedLogExtraInfoMap = std::map<const char*, const nsACString*>; + using ScopedLogExtraInfoMap = + std::map<const char*, const Tainted<nsCString>*>; static ScopedLogExtraInfoMap GetExtraInfoMap(); static void Initialize(); private: const char* mTag; - const nsACString* mPreviousValue; - nsCString mCurrentValue; + const Tainted<nsCString>* mPreviousValue; + Tainted<nsCString> mCurrentValue; - static MOZ_THREAD_LOCAL(const nsACString*) sQueryValue; - static MOZ_THREAD_LOCAL(const nsACString*) sContextValue; + static MOZ_THREAD_LOCAL(const Tainted<nsCString>*) sQueryValueTainted; + static MOZ_THREAD_LOCAL(const Tainted<nsCString>*) sContextValueTainted; + static MOZ_THREAD_LOCAL(const Tainted<nsCString>*) sStorageOriginValueTainted; void AddInfo(); #else diff --git a/dom/quota/SerializationHelpers.h b/dom/quota/SerializationHelpers.h index 50c6c6a5e6..7eb4773964 100644 --- a/dom/quota/SerializationHelpers.h +++ b/dom/quota/SerializationHelpers.h @@ -64,7 +64,6 @@ struct ParamTraits<mozilla::OriginAttributesPattern> { static void Write(MessageWriter* aWriter, const paramType& aParam) { WriteParam(aWriter, aParam.mFirstPartyDomain); - WriteParam(aWriter, aParam.mInIsolatedMozBrowser); WriteParam(aWriter, aParam.mPrivateBrowsingId); WriteParam(aWriter, aParam.mUserContextId); WriteParam(aWriter, aParam.mGeckoViewSessionContextId); @@ -72,7 +71,6 @@ struct ParamTraits<mozilla::OriginAttributesPattern> { static bool Read(MessageReader* aReader, paramType* aResult) { return ReadParam(aReader, &aResult->mFirstPartyDomain) && - ReadParam(aReader, &aResult->mInIsolatedMozBrowser) && ReadParam(aReader, &aResult->mPrivateBrowsingId) && ReadParam(aReader, &aResult->mUserContextId) && ReadParam(aReader, &aResult->mGeckoViewSessionContextId); diff --git a/dom/quota/StorageOriginAttributes.cpp b/dom/quota/StorageOriginAttributes.cpp index 7d8fd9c802..bcdf47bce8 100644 --- a/dom/quota/StorageOriginAttributes.cpp +++ b/dom/quota/StorageOriginAttributes.cpp @@ -6,10 +6,58 @@ #include "StorageOriginAttributes.h" +#include "nsString.h" #include "nsURLHelper.h" +#include "mozilla/Assertions.h" +#include "mozilla/dom/quota/QuotaManager.h" namespace mozilla { +void StorageOriginAttributes::CreateSuffix(nsACString& aStr) const { + nsCString str1; + + URLParams params; + nsAutoString value; + + if (mInIsolatedMozBrowser) { + params.Set(u"inBrowser"_ns, u"1"_ns); + } + + str1.Truncate(); + + params.Serialize(value, true); + if (!value.IsEmpty()) { + str1.AppendLiteral("^"); + str1.Append(NS_ConvertUTF16toUTF8(value)); + } + + // Make sure that the string don't contain characters that would get replaced + // with the plus character by quota manager, potentially causing ambiguity. + MOZ_ASSERT(str1.FindCharInSet(dom::quota::QuotaManager::kReplaceChars) == + kNotFound); + + // Let OriginAttributes::CreateSuffix serialize other origin attributes. + nsCString str2; + mOriginAttributes.CreateSuffix(str2); + + aStr.Truncate(); + + if (str1.IsEmpty()) { + aStr.Append(str2); + return; + } + + if (str2.IsEmpty()) { + aStr.Append(str1); + return; + } + + // If both strings are not empty, we need to combine them. + aStr.Append(str1); + aStr.Append('&'); + aStr.Append(Substring(str2, 1, str2.Length() - 1)); +} + bool StorageOriginAttributes::PopulateFromSuffix(const nsACString& aStr) { if (aStr.IsEmpty()) { return true; @@ -20,7 +68,7 @@ bool StorageOriginAttributes::PopulateFromSuffix(const nsACString& aStr) { } bool ok = - URLParams::Parse(Substring(aStr, 1, aStr.Length() - 1), + URLParams::Parse(Substring(aStr, 1, aStr.Length() - 1), true, [this](const nsAString& aName, const nsAString& aValue) { if (aName.EqualsLiteral("inBrowser")) { if (!aValue.EqualsLiteral("1")) { diff --git a/dom/quota/StorageOriginAttributes.h b/dom/quota/StorageOriginAttributes.h index 5d8f416b38..b68fdc804b 100644 --- a/dom/quota/StorageOriginAttributes.h +++ b/dom/quota/StorageOriginAttributes.h @@ -15,12 +15,32 @@ namespace mozilla { // in OriginAttributes class anymore. class StorageOriginAttributes { public: + StorageOriginAttributes() : mInIsolatedMozBrowser(false) {} + + explicit StorageOriginAttributes(bool aInIsolatedMozBrowser) + : mInIsolatedMozBrowser(aInIsolatedMozBrowser) {} + bool InIsolatedMozBrowser() const { return mInIsolatedMozBrowser; } uint32_t UserContextId() const { return mOriginAttributes.mUserContextId; } // New getters can be added here incrementally. + void SetInIsolatedMozBrowser(bool aInIsolatedMozBrowser) { + mInIsolatedMozBrowser = aInIsolatedMozBrowser; + } + + void SetUserContextId(uint32_t aUserContextId) { + mOriginAttributes.mUserContextId = aUserContextId; + } + + // New setters can be added here incrementally. + + // Serializes/Deserializes non-default values into the suffix format, i.e. + // |^key1=value1&key2=value2|. If there are no non-default attributes, this + // returns an empty string + void CreateSuffix(nsACString& aStr) const; + [[nodiscard]] bool PopulateFromSuffix(const nsACString& aStr); // Populates the attributes from a string like @@ -31,7 +51,7 @@ class StorageOriginAttributes { private: OriginAttributes mOriginAttributes; - bool mInIsolatedMozBrowser = false; + bool mInIsolatedMozBrowser; }; } // namespace mozilla diff --git a/dom/quota/test/gtest/TestScopedLogExtraInfo.cpp b/dom/quota/test/gtest/TestScopedLogExtraInfo.cpp index 00a3393844..395246c1fb 100644 --- a/dom/quota/test/gtest/TestScopedLogExtraInfo.cpp +++ b/dom/quota/test/gtest/TestScopedLogExtraInfo.cpp @@ -16,19 +16,23 @@ TEST(DOM_Quota_ScopedLogExtraInfo, AddAndRemove) { const auto extraInfo = - ScopedLogExtraInfo{ScopedLogExtraInfo::kTagQuery, text}; + ScopedLogExtraInfo{ScopedLogExtraInfo::kTagQueryTainted, text}; #ifdef QM_SCOPED_LOG_EXTRA_INFO_ENABLED const auto& extraInfoMap = ScopedLogExtraInfo::GetExtraInfoMap(); - EXPECT_EQ(text, *extraInfoMap.at(ScopedLogExtraInfo::kTagQuery)); + const auto& queryValueTainted = + *extraInfoMap.at(ScopedLogExtraInfo::kTagQueryTainted); + + EXPECT_EQ(text, MOZ_NO_VALIDATE(queryValueTainted, + "It's ok to use query value in tests.")); #endif } #ifdef QM_SCOPED_LOG_EXTRA_INFO_ENABLED const auto& extraInfoMap = ScopedLogExtraInfo::GetExtraInfoMap(); - EXPECT_EQ(0u, extraInfoMap.count(ScopedLogExtraInfo::kTagQuery)); + EXPECT_EQ(0u, extraInfoMap.count(ScopedLogExtraInfo::kTagQueryTainted)); #endif } @@ -39,27 +43,38 @@ TEST(DOM_Quota_ScopedLogExtraInfo, Nested) { const auto extraInfo = - ScopedLogExtraInfo{ScopedLogExtraInfo::kTagQuery, text}; + ScopedLogExtraInfo{ScopedLogExtraInfo::kTagQueryTainted, text}; { const auto extraInfo = - ScopedLogExtraInfo{ScopedLogExtraInfo::kTagQuery, nestedText}; + ScopedLogExtraInfo{ScopedLogExtraInfo::kTagQueryTainted, nestedText}; #ifdef QM_SCOPED_LOG_EXTRA_INFO_ENABLED const auto& extraInfoMap = ScopedLogExtraInfo::GetExtraInfoMap(); - EXPECT_EQ(nestedText, *extraInfoMap.at(ScopedLogExtraInfo::kTagQuery)); + + const auto& queryValueTainted = + *extraInfoMap.at(ScopedLogExtraInfo::kTagQueryTainted); + + EXPECT_EQ(nestedText, + MOZ_NO_VALIDATE(queryValueTainted, + "It's ok to use query value in tests.")); #endif } #ifdef QM_SCOPED_LOG_EXTRA_INFO_ENABLED const auto& extraInfoMap = ScopedLogExtraInfo::GetExtraInfoMap(); - EXPECT_EQ(text, *extraInfoMap.at(ScopedLogExtraInfo::kTagQuery)); + + const auto& queryValueTainted = + *extraInfoMap.at(ScopedLogExtraInfo::kTagQueryTainted); + + EXPECT_EQ(text, MOZ_NO_VALIDATE(queryValueTainted, + "It's ok to use query value in tests.")); #endif } #ifdef QM_SCOPED_LOG_EXTRA_INFO_ENABLED const auto& extraInfoMap = ScopedLogExtraInfo::GetExtraInfoMap(); - EXPECT_EQ(0u, extraInfoMap.count(ScopedLogExtraInfo::kTagQuery)); + EXPECT_EQ(0u, extraInfoMap.count(ScopedLogExtraInfo::kTagQueryTainted)); #endif } diff --git a/dom/quota/test/gtest/TestStorageOriginAttributes.cpp b/dom/quota/test/gtest/TestStorageOriginAttributes.cpp index 4529e1d53b..f7a8387305 100644 --- a/dom/quota/test/gtest/TestStorageOriginAttributes.cpp +++ b/dom/quota/test/gtest/TestStorageOriginAttributes.cpp @@ -9,6 +9,33 @@ namespace mozilla::dom::quota::test { +TEST(DOM_Quota_StorageOriginAttributes, Constructor_Default) +{ + { + StorageOriginAttributes originAttributes; + + ASSERT_FALSE(originAttributes.InIsolatedMozBrowser()); + ASSERT_EQ(originAttributes.UserContextId(), 0u); + } +} + +TEST(DOM_Quota_StorageOriginAttributes, Constructor_InIsolatedMozbrowser) +{ + { + StorageOriginAttributes originAttributes(/* aInIsolatedMozBrowser */ false); + + ASSERT_FALSE(originAttributes.InIsolatedMozBrowser()); + ASSERT_EQ(originAttributes.UserContextId(), 0u); + } + + { + StorageOriginAttributes originAttributes(/* aInIsolatedMozBrowser */ true); + + ASSERT_TRUE(originAttributes.InIsolatedMozBrowser()); + ASSERT_EQ(originAttributes.UserContextId(), 0u); + } +} + TEST(DOM_Quota_StorageOriginAttributes, PopulateFromOrigin_NoOriginAttributes) { { @@ -162,4 +189,52 @@ TEST(DOM_Quota_StorageOriginAttributes, PopulateFromOrigin_Mixed_Invalid) } } +TEST(DOM_Quota_StorageOriginAttributes, CreateSuffix_NoOriginAttributes) +{ + { + StorageOriginAttributes originAttributes; + nsCString suffix; + originAttributes.CreateSuffix(suffix); + + ASSERT_TRUE(suffix.IsEmpty()); + } +} + +TEST(DOM_Quota_StorageOriginAttributes, CreateSuffix_InIsolatedMozbrowser) +{ + { + StorageOriginAttributes originAttributes; + originAttributes.SetInIsolatedMozBrowser(true); + nsCString suffix; + originAttributes.CreateSuffix(suffix); + + ASSERT_TRUE(suffix.Equals("^inBrowser=1"_ns)); + } +} + +TEST(DOM_Quota_StorageOriginAttributes, CreateSuffix_UserContextId) +{ + { + StorageOriginAttributes originAttributes; + originAttributes.SetUserContextId(42); + nsCString suffix; + originAttributes.CreateSuffix(suffix); + + ASSERT_TRUE(suffix.Equals("^userContextId=42"_ns)); + } +} + +TEST(DOM_Quota_StorageOriginAttributes, CreateSuffix_Mixed) +{ + { + StorageOriginAttributes originAttributes; + originAttributes.SetInIsolatedMozBrowser(true); + originAttributes.SetUserContextId(42); + nsCString suffix; + originAttributes.CreateSuffix(suffix); + + ASSERT_TRUE(suffix.Equals("^inBrowser=1&userContextId=42"_ns)); + } +} + } // namespace mozilla::dom::quota::test diff --git a/dom/quota/test/modules/system/worker/ModuleLoader.js b/dom/quota/test/modules/system/worker/ModuleLoader.js index b79f3ff5b9..5354c26e34 100644 --- a/dom/quota/test/modules/system/worker/ModuleLoader.js +++ b/dom/quota/test/modules/system/worker/ModuleLoader.js @@ -3,7 +3,7 @@ * http://creativecommons.org/publicdomain/zero/1.0/ */ -function ModuleLoader(base, depth, proto) { +function ModuleLoader(base, depth) { const modules = {}; const require = async function (id) { diff --git a/dom/quota/test/xpcshell/telemetry/test_dom_quota_try.js b/dom/quota/test/xpcshell/telemetry/test_dom_quota_try.js new file mode 100644 index 0000000000..28bb2d63b4 --- /dev/null +++ b/dom/quota/test/xpcshell/telemetry/test_dom_quota_try.js @@ -0,0 +1,180 @@ +/** + * Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/publicdomain/zero/1.0/ + */ + +const { AppConstants } = ChromeUtils.importESModule( + "resource://gre/modules/AppConstants.sys.mjs" +); + +const { TelemetryTestUtils } = ChromeUtils.importESModule( + "resource://testing-common/TelemetryTestUtils.sys.mjs" +); + +// This is a list of all extra keys that are exposed to telemetry. Please only +// add things to this list with great care and proper code review from relevant +// module owner/peers and proper data review from data stewards. +const allowedExtraKeys = [ + "context", + "frame_id", + "process_id", + "result", + "seq", + "severity", + "source_file", + "source_line", + "stack_id", +]; + +const originSchemes = [ + "http", + "https", + "ftp", + "ws", + "wss", + "gopher", + "blob", + "file", + "data", +]; + +const testcases = [ + // Test temporary storage initialization with and without a broken origin + // directory. + { + async setup(expectedInitResult) { + Services.prefs.setBoolPref("dom.quotaManager.loadQuotaFromCache", false); + + let request = init(); + await requestFinished(request); + + request = initTemporaryStorage(); + await requestFinished(request); + + request = initTemporaryOrigin( + "default", + getPrincipal("https://example.com") + ); + await requestFinished(request); + + const usageFile = getRelativeFile( + "storage/default/https+++example.com/ls/usage" + ); + + if (!expectedInitResult) { + usageFile.create(Ci.nsIFile.DIRECTORY_TYPE, 0o755); + } else { + usageFile.create(Ci.nsIFile.NORMAL_FILE_TYPE, 0o666); + } + + // XXX It would be nice to have a method which shuts down temporary + // storage only. Now we have to shut down entire storage (including + // temporary storage) and then initialize storage again. + request = reset(); + await requestFinished(request); + + request = init(); + await requestFinished(request); + }, + initFunction: initTemporaryStorage, + getExpectedNumberOfEvents() { + if (AppConstants.EARLY_BETA_OR_EARLIER || AppConstants.DEBUG) { + if (AppConstants.NIGHTLY_BUILD) { + return { + initFailure: 9, + initSuccess: 0, + }; + } + + return { + initFailure: 14, + initSuccess: 0, + }; + } + + return { + initFailure: 0, + initSuccess: 0, + }; + }, + async cleanup() { + const request = clear(); + await requestFinished(request); + + Services.prefs.setBoolPref("dom.quotaManager.loadQuotaFromCache", true); + }, + }, +]; + +function verifyEvents(expectedNumberOfEvents) { + const events = TelemetryTestUtils.getEvents({ + category: "dom.quota.try", + method: "error", + }); + + is( + events.length, + expectedNumberOfEvents, + "The number of events must match the expected number of events" + ); + + for (const event of events) { + for (const extraKey in event.extra) { + ok( + allowedExtraKeys.includes(extraKey), + `The extra key ${extraKey} must be in the allow list` + ); + + const extraValue = event.extra[extraKey]; + + // These are extra paranoia checks to ensure extra values don't contain + // origin like strings. + for (const suffix of ["://", "+++"]) { + ok( + originSchemes.every( + originScheme => !extraValue.includes(originScheme + suffix) + ), + `The extra value ${extraValue} must not contain origin like strings` + ); + } + } + } +} + +async function testSteps() { + for (const testcase of testcases) { + for (const expectedInitResult of [false, true]) { + // Clear all events. + Services.telemetry.clearEvents(); + + info( + `Verifying the events when the initialization ` + + `${expectedInitResult ? "succeeds" : "fails"}` + ); + + await testcase.setup(expectedInitResult); + + const msg = `Should ${expectedInitResult ? "not " : ""} have thrown`; + + let request = testcase.initFunction(); + try { + await requestFinished(request); + ok(expectedInitResult, msg); + } catch (ex) { + ok(!expectedInitResult, msg); + } + + const expectedNumberOfEventsObject = testcase.getExpectedNumberOfEvents + ? testcase.getExpectedNumberOfEvents() + : testcase.expectedNumberOfEvents; + + const expectedNumberOfEvents = expectedInitResult + ? expectedNumberOfEventsObject.initSuccess + : expectedNumberOfEventsObject.initFailure; + + verifyEvents(expectedNumberOfEvents); + + await testcase.cleanup(); + } + } +} diff --git a/dom/quota/test/xpcshell/telemetry/xpcshell.toml b/dom/quota/test/xpcshell/telemetry/xpcshell.toml index 949ef3cb06..36a176db2a 100644 --- a/dom/quota/test/xpcshell/telemetry/xpcshell.toml +++ b/dom/quota/test/xpcshell/telemetry/xpcshell.toml @@ -13,5 +13,8 @@ support-files = [ "version2_2_profile.zip", ] +["test_dom_quota_try.js"] +skip-if = ["os == 'android' || appname == 'thunderbird'"] + ["test_qm_first_initialization_attempt.js"] skip-if = ["appname == 'thunderbird'"] |