From f40ce49b0242f9bd23447dc96e6a6e0cf204e06a Mon Sep 17 00:00:00 2001 From: Jan Varga Date: Tue, 15 Nov 2022 17:13:02 +0000 Subject: [PATCH] Bug 1790207 - Forward declaration of mozilla::Result in nsIGlobalObject.h and other cleanup; r=dom-storage-reviewers,jesup Depends on D162087 Differential Revision: https://phabricator.services.mozilla.com/D162088 --- caps/nsJSPrincipals.h | 5 +-- dom/base/nsIGlobalObject.cpp | 11 +++--- dom/base/nsIGlobalObject.h | 8 +++-- dom/fs/api/FileSystemHandle.cpp | 42 +++++++++++------------ dom/fs/api/FileSystemManager.cpp | 9 ++--- dom/fs/child/FileSystemRequestHandler.cpp | 2 +- dom/quota/ResultExtensions.h | 7 ++-- dom/security/test/gtest/moz.build | 2 -- dom/workers/WorkerScope.cpp | 39 ++++++++++----------- dom/workers/WorkerScope.h | 5 ++- ipc/glue/BackgroundUtils.cpp | 16 ++++----- js/xpconnect/src/Sandbox.cpp | 1 + js/xpconnect/src/XPCRuntimeService.cpp | 1 + 13 files changed, 72 insertions(+), 76 deletions(-) diff --git a/caps/nsJSPrincipals.h b/caps/nsJSPrincipals.h index 6c34e7bff517..a928c0de200a 100644 --- a/caps/nsJSPrincipals.h +++ b/caps/nsJSPrincipals.h @@ -35,11 +35,12 @@ class nsJSPrincipals : public nsIPrincipal, public JSPrincipals { uint32_t aTag, JSPrincipals** aOutPrincipals); + static bool ReadPrincipalInfo(JSStructuredCloneReader* aReader, + mozilla::ipc::PrincipalInfo& aInfo); + /* For write() implementations of off-main-thread JSPrincipals. */ static bool WritePrincipalInfo(JSStructuredCloneWriter* aWriter, const mozilla::ipc::PrincipalInfo& aInfo); - static bool ReadPrincipalInfo(JSStructuredCloneReader* aReader, - mozilla::ipc::PrincipalInfo& aInfo); // This class is used on the main thread to specify which principal to use // when reading principals data that was set on a DOM worker thread. diff --git a/dom/base/nsIGlobalObject.cpp b/dom/base/nsIGlobalObject.cpp index 15bf5729c229..42d2cd841c18 100644 --- a/dom/base/nsIGlobalObject.cpp +++ b/dom/base/nsIGlobalObject.cpp @@ -6,17 +6,16 @@ #include "nsIGlobalObject.h" #include "mozilla/CycleCollectedJSContext.h" +#include "mozilla/Result.h" #include "mozilla/StorageAccess.h" #include "mozilla/dom/BlobURLProtocolHandler.h" #include "mozilla/dom/FunctionBinding.h" -#include "mozilla/ipc/PBackgroundSharedTypes.h" #include "mozilla/dom/Report.h" #include "mozilla/dom/ReportingObserver.h" #include "mozilla/dom/ServiceWorker.h" #include "mozilla/dom/ServiceWorkerRegistration.h" -#include "mozilla/dom/WorkerPrivate.h" +#include "mozilla/ipc/PBackgroundSharedTypes.h" #include "nsContentUtils.h" -#include "nsJSPrincipals.h" #include "nsThreadUtils.h" #include "nsGlobalWindowInner.h" @@ -387,13 +386,13 @@ nsIGlobalObject::GetStorageKey() { } bool nsIGlobalObject::IsEqualStorageKey( - mozilla::ipc::PrincipalInfo& aPrincipalInfo) { + const mozilla::ipc::PrincipalInfo& aPrincipalInfo) { auto result = GetStorageKey(); - mozilla::ipc::PrincipalInfo storagePrincipalInfo; if (result.isErr()) { return false; } - storagePrincipalInfo = result.unwrap(); + + const auto& storagePrincipalInfo = result.inspect(); return mozilla::ipc::NonExpandedPrincipalInfoEquals(aPrincipalInfo, storagePrincipalInfo); diff --git a/dom/base/nsIGlobalObject.h b/dom/base/nsIGlobalObject.h index 4834f4650e53..00705dc59092 100644 --- a/dom/base/nsIGlobalObject.h +++ b/dom/base/nsIGlobalObject.h @@ -9,7 +9,6 @@ #include "mozilla/LinkedList.h" #include "mozilla/Maybe.h" -#include "mozilla/Result.h" #include "mozilla/dom/ClientInfo.h" #include "mozilla/dom/DispatcherTrait.h" #include "mozilla/dom/ServiceWorkerDescriptor.h" @@ -35,6 +34,8 @@ class nsPIDOMWindowInner; namespace mozilla { class DOMEventTargetHelper; +template +class Result; enum class StorageAccess; namespace dom { class VoidFunction; @@ -48,6 +49,9 @@ class ServiceWorker; class ServiceWorkerRegistration; class ServiceWorkerRegistrationDescriptor; } // namespace dom +namespace ipc { +class PrincipalInfo; +} // namespace ipc } // namespace mozilla namespace JS::loader { @@ -258,7 +262,7 @@ class nsIGlobalObject : public nsISupports, virtual mozilla::Result GetStorageKey(); - virtual bool IsEqualStorageKey(mozilla::ipc::PrincipalInfo& aPrincipalInfo); + bool IsEqualStorageKey(const mozilla::ipc::PrincipalInfo& aPrincipalInfo); protected: virtual ~nsIGlobalObject(); diff --git a/dom/fs/api/FileSystemHandle.cpp b/dom/fs/api/FileSystemHandle.cpp index f0dc445e188e..f93ccde6b3cd 100644 --- a/dom/fs/api/FileSystemHandle.cpp +++ b/dom/fs/api/FileSystemHandle.cpp @@ -29,10 +29,10 @@ namespace mozilla::dom { namespace { -bool ConstructHandleMetadata(JSContext* aCx, JSStructuredCloneReader* aReader, - const bool aDirectory, nsIGlobalObject* aGlobal, - fs::FileSystemEntryMetadata& aMetadata, - mozilla::ipc::PrincipalInfo& info) { +bool ConstructHandleMetadata(JSContext* aCx, nsIGlobalObject* aGlobal, + JSStructuredCloneReader* aReader, + const bool aDirectory, + fs::FileSystemEntryMetadata& aMetadata) { using namespace mozilla::dom::fs; EntryId entryId; @@ -54,19 +54,20 @@ bool ConstructHandleMetadata(JSContext* aCx, JSStructuredCloneReader* aReader, return false; } - aMetadata = fs::FileSystemEntryMetadata(entryId, name, aDirectory); - - if (!nsJSPrincipals::ReadPrincipalInfo(aReader, info)) { + mozilla::ipc::PrincipalInfo principalInfo; + if (!nsJSPrincipals::ReadPrincipalInfo(aReader, principalInfo)) { return false; } - if (!aGlobal->IsEqualStorageKey(info)) { + + if (!aGlobal->IsEqualStorageKey(principalInfo)) { LOG(("Blocking deserialization of %s due to cross-origin", - NS_ConvertUTF16toUTF8(aMetadata.entryName()).get())); + NS_ConvertUTF16toUTF8(name).get())); return false; } - LOG_VERBOSE( - ("Deserializing %s", NS_ConvertUTF16toUTF8(aMetadata.entryName()).get())); + LOG_VERBOSE(("Deserializing %s", NS_ConvertUTF16toUTF8(name).get())); + + aMetadata = fs::FileSystemEntryMetadata(entryId, name, aDirectory); return true; } @@ -257,8 +258,7 @@ bool FileSystemHandle::WriteStructuredClone( // Needed to make sure the destination nsIGlobalObject is from the same // origin/principal - QM_TRY_UNWRAP(mozilla::ipc::PrincipalInfo principalInfo, - mGlobal->GetStorageKey(), false); + QM_TRY_INSPECT(const auto& principalInfo, mGlobal->GetStorageKey(), false); return nsJSPrincipals::WritePrincipalInfo(aWriter, principalInfo); } @@ -269,14 +269,14 @@ already_AddRefed FileSystemHandle::ConstructFileHandle( JSStructuredCloneReader* aReader) { using namespace mozilla::dom::fs; - mozilla::ipc::PrincipalInfo info; FileSystemEntryMetadata metadata; - if (!ConstructHandleMetadata(aCx, aReader, /* aDirectory */ false, aGlobal, - metadata, info)) { + if (!ConstructHandleMetadata(aCx, aGlobal, aReader, /* aDirectory */ false, + metadata)) { return nullptr; } - // Note that the actor may not be connected yet + // XXX Get the manager from Navigator! + // Note that the actor may not exist or may not be connected yet. auto fileSystemManager = MakeRefPtr(aGlobal, nullptr); RefPtr fsHandle = @@ -292,14 +292,14 @@ FileSystemHandle::ConstructDirectoryHandle(JSContext* aCx, JSStructuredCloneReader* aReader) { using namespace mozilla::dom::fs; - mozilla::ipc::PrincipalInfo info; FileSystemEntryMetadata metadata; - if (!ConstructHandleMetadata(aCx, aReader, /* aDirectory */ true, aGlobal, - metadata, info)) { + if (!ConstructHandleMetadata(aCx, aGlobal, aReader, /* aDirectory */ true, + metadata)) { return nullptr; } - // Note that the actor may not be connected yet + // XXX Get the manager from Navigator! + // Note that the actor may not exist or may not be connected yet. auto fileSystemManager = MakeRefPtr(aGlobal, nullptr); RefPtr fsHandle = diff --git a/dom/fs/api/FileSystemManager.cpp b/dom/fs/api/FileSystemManager.cpp index 540212fafcef..7b81daa16fba 100644 --- a/dom/fs/api/FileSystemManager.cpp +++ b/dom/fs/api/FileSystemManager.cpp @@ -12,12 +12,8 @@ #include "mozilla/dom/FileSystemManagerChild.h" #include "mozilla/dom/Promise.h" #include "mozilla/dom/StorageManager.h" -#include "mozilla/dom/WorkerPrivate.h" #include "mozilla/dom/quota/QuotaCommon.h" #include "mozilla/dom/quota/ResultExtensions.h" -#include "mozilla/ipc/BackgroundUtils.h" -#include "mozilla/ipc/PBackgroundSharedTypes.h" -#include "nsIScriptObjectPrincipal.h" namespace mozilla::dom { @@ -67,9 +63,8 @@ void FileSystemManager::BeginRequest( MOZ_ASSERT(mGlobal); - QM_TRY_UNWRAP(mozilla::ipc::PrincipalInfo principalInfo, - mGlobal->GetStorageKey(), QM_VOID, - [aFailure](nsresult rv) { aFailure(rv); }); + QM_TRY_INSPECT(const auto& principalInfo, mGlobal->GetStorageKey(), QM_VOID, + [&aFailure](nsresult rv) { aFailure(rv); }); mBackgroundRequestHandler->CreateFileSystemManagerChild(principalInfo) ->Then( diff --git a/dom/fs/child/FileSystemRequestHandler.cpp b/dom/fs/child/FileSystemRequestHandler.cpp index 68e6ec71e184..d0e3153eabc5 100644 --- a/dom/fs/child/FileSystemRequestHandler.cpp +++ b/dom/fs/child/FileSystemRequestHandler.cpp @@ -335,7 +335,7 @@ mozilla::ipc::RejectCallback GetRejectCallback( } struct BeginRequestFailureCallback { - BeginRequestFailureCallback(RefPtr aPromise) + explicit BeginRequestFailureCallback(RefPtr aPromise) : mPromise(std::move(aPromise)) {} void operator()(nsresult aRv) const { diff --git a/dom/quota/ResultExtensions.h b/dom/quota/ResultExtensions.h index e0620cab9623..c19bf926612e 100644 --- a/dom/quota/ResultExtensions.h +++ b/dom/quota/ResultExtensions.h @@ -134,12 +134,11 @@ Result ToResultGet(const Func& aFunc, Args&&... aArgs) { } // namespace mozilla // TODO: Maybe move this to mfbt/ResultExtensions.h -#define MOZ_TO_RESULT(expr) ::mozilla::ToResult(expr) +#define MOZ_TO_RESULT(expr) ToResult(expr) -#define QM_TO_RESULT(expr) ::mozilla::ToResult(expr) +#define QM_TO_RESULT(expr) ToResult(expr) -#define QM_TO_RESULT_TRANSFORM(value) \ - ::mozilla::ToResultTransform(value) +#define QM_TO_RESULT_TRANSFORM(value) ToResultTransform(value) #define MOZ_TO_RESULT_GET_TYPED(resultType, ...) \ ::mozilla::ToResultGet(__VA_ARGS__) diff --git a/dom/security/test/gtest/moz.build b/dom/security/test/gtest/moz.build index 816b1e93c987..c9ab4dcece85 100644 --- a/dom/security/test/gtest/moz.build +++ b/dom/security/test/gtest/moz.build @@ -16,8 +16,6 @@ if CONFIG["OS_TARGET"] != "Android": "TestUnexpectedPrivilegedLoads.cpp", ] -include("/ipc/chromium/chromium-config.mozbuild") - FINAL_LIBRARY = "xul-gtest" LOCAL_INCLUDES += [ diff --git a/dom/workers/WorkerScope.cpp b/dom/workers/WorkerScope.cpp index b3210a3bc947..613ead32312e 100644 --- a/dom/workers/WorkerScope.cpp +++ b/dom/workers/WorkerScope.cpp @@ -63,6 +63,7 @@ #include "mozilla/dom/ImageBitmapSource.h" #include "mozilla/dom/MessagePortBinding.h" #include "mozilla/ipc/PBackgroundChild.h" +#include "mozilla/ipc/PBackgroundSharedTypes.h" #include "mozilla/dom/Performance.h" #include "mozilla/dom/Promise.h" #include "mozilla/dom/PromiseWorkerProxy.h" @@ -296,6 +297,24 @@ Maybe WorkerGlobalScopeBase::GetController() const { return mClientSource->GetController(); } +mozilla::Result +WorkerGlobalScopeBase::GetStorageKey() { + AssertIsOnWorkerThread(); + + const mozilla::ipc::PrincipalInfo& principalInfo = + mWorkerPrivate->GetEffectiveStoragePrincipalInfo(); + + // Block expanded and null principals, let content and system through. + if (principalInfo.type() != + mozilla::ipc::PrincipalInfo::TContentPrincipalInfo && + principalInfo.type() != + mozilla::ipc::PrincipalInfo::TSystemPrincipalInfo) { + return Err(NS_ERROR_DOM_SECURITY_ERR); + } + + return principalInfo; +} + void WorkerGlobalScopeBase::Control( const ServiceWorkerDescriptor& aServiceWorker) { AssertIsOnWorkerThread(); @@ -313,26 +332,6 @@ void WorkerGlobalScopeBase::Control( } } -mozilla::Result -WorkerGlobalScopeBase::GetStorageKey() { - using mozilla::ipc::PrincipalInfo; - - MOZ_ASSERT(!NS_IsMainThread()); - - const PrincipalInfo& principalInfo = - mWorkerPrivate->GetEffectiveStoragePrincipalInfo(); - - // Block expanded and null principals, let content and system through. - if (principalInfo.type() != - mozilla::ipc::PrincipalInfo::TContentPrincipalInfo && - principalInfo.type() != - mozilla::ipc::PrincipalInfo::TSystemPrincipalInfo) { - return Err(NS_ERROR_DOM_SECURITY_ERR); - } - - return principalInfo; -} - nsresult WorkerGlobalScopeBase::Dispatch( TaskCategory aCategory, already_AddRefed&& aRunnable) { return EventTargetFor(aCategory)->Dispatch(std::move(aRunnable), diff --git a/dom/workers/WorkerScope.h b/dom/workers/WorkerScope.h index 2ce7961cc923..33163157fa80 100644 --- a/dom/workers/WorkerScope.h +++ b/dom/workers/WorkerScope.h @@ -128,10 +128,9 @@ class WorkerGlobalScopeBase : public DOMEventTargetHelper, Maybe GetController() const final; - virtual void Control(const ServiceWorkerDescriptor& aServiceWorker); + mozilla::Result GetStorageKey() final; - virtual mozilla::Result GetStorageKey() - override; + virtual void Control(const ServiceWorkerDescriptor& aServiceWorker); // DispatcherTrait implementation nsresult Dispatch(TaskCategory aCategory, diff --git a/ipc/glue/BackgroundUtils.cpp b/ipc/glue/BackgroundUtils.cpp index 38ebe4b51514..a841c687d435 100644 --- a/ipc/glue/BackgroundUtils.cpp +++ b/ipc/glue/BackgroundUtils.cpp @@ -174,22 +174,22 @@ bool NonExpandedPrincipalInfoEquals(const PrincipalInfo& aLeft, const ContentPrincipalInfo& leftContent = aLeft.get_ContentPrincipalInfo(); const ContentPrincipalInfo& rightContent = aRight.get_ContentPrincipalInfo(); + switch (aLeft.type()) { - case PrincipalInfo::TContentPrincipalInfo: { + case PrincipalInfo::TContentPrincipalInfo: return leftContent.attrs() == rightContent.attrs() && leftContent.originNoSuffix() == rightContent.originNoSuffix(); - } - case PrincipalInfo::TSystemPrincipalInfo: { + + case PrincipalInfo::TSystemPrincipalInfo: // system principal always matches return true; - } - case PrincipalInfo::TNullPrincipalInfo: { + + case PrincipalInfo::TNullPrincipalInfo: return leftContent.attrs() == rightContent.attrs() && leftContent.spec() == rightContent.spec(); - } - default: { + + default: break; - } } // Clients (windows/workers) should never have an expanded principal type. diff --git a/js/xpconnect/src/Sandbox.cpp b/js/xpconnect/src/Sandbox.cpp index 402a17b73580..0cd1b008778b 100644 --- a/js/xpconnect/src/Sandbox.cpp +++ b/js/xpconnect/src/Sandbox.cpp @@ -35,6 +35,7 @@ #include "xpc_make_class.h" #include "XPCWrapper.h" #include "Crypto.h" +#include "mozilla/Result.h" #include "mozilla/dom/AbortControllerBinding.h" #include "mozilla/dom/AutoEntryScript.h" #include "mozilla/dom/BindingCallContext.h" diff --git a/js/xpconnect/src/XPCRuntimeService.cpp b/js/xpconnect/src/XPCRuntimeService.cpp index fc7701fc4dd7..3b791d52a2ae 100644 --- a/js/xpconnect/src/XPCRuntimeService.cpp +++ b/js/xpconnect/src/XPCRuntimeService.cpp @@ -9,6 +9,7 @@ #include "nsContentUtils.h" #include "BackstagePass.h" +#include "mozilla/Result.h" #include "mozilla/dom/BindingUtils.h" #include "mozilla/dom/WebIDLGlobalNameHash.h" #include "mozilla/dom/IndexedDatabaseManager.h"