Bug 1921972 - Allow to propagate loadgroup flags to parent process. r=nika,necko-reviewers,valentin

As per discussion. Test is in the other patch, assuming this is green
I'll incorporate it, but gotta go for the day.

Differential Revision: https://phabricator.services.mozilla.com/D224824

netwerk/base/nsLoadGroup.cpp

@@ -1074,10 +1074,8 @@ nsresult nsLoadGroup::MergeLoadFlags(nsIRequest* aRequest,
 
   oldFlags = flags;
 
-  // Inherit the following bits...
-  flags |= (mLoadFlags &
-            (LOAD_BACKGROUND | LOAD_BYPASS_CACHE | LOAD_FROM_CACHE |
-             VALIDATE_ALWAYS | VALIDATE_ONCE_PER_SESSION | VALIDATE_NEVER));
+  // Inherit some bits...
+  flags |= mLoadFlags & kInheritedLoadFlags;
 
   // ... and force the default flags.
   flags |= mDefaultLoadFlags;

netwerk/base/nsLoadGroup.h

@@ -61,6 +61,18 @@ class nsLoadGroup : public nsILoadGroup,
   void SetGroupObserver(nsIRequestObserver* aObserver,
                         bool aIncludeBackgroundRequests);
 
+  /**
+   * Flags inherited from the default request in the load group onto other loads
+   * added to the load group.
+   *
+   * NOTE(emilio): If modifying these, be aware that we allow these flags to be
+   * effectively set from the content process on a document navigation, and
+   * thus nothing security-critical should be allowed here.
+   */
+  static constexpr nsLoadFlags kInheritedLoadFlags =
+      LOAD_BACKGROUND | LOAD_BYPASS_CACHE | LOAD_FROM_CACHE | VALIDATE_ALWAYS |
+      VALIDATE_ONCE_PER_SESSION | VALIDATE_NEVER;
+
  protected:
   virtual ~nsLoadGroup();
 

netwerk/ipc/DocumentChannel.cpp

@@ -32,6 +32,7 @@
 #include "nsILoadInfo.h"
 #include "nsIStreamListener.h"
 #include "nsIURI.h"
+#include "nsLoadGroup.h"
 #include "nsMimeTypes.h"
 #include "nsNetUtil.h"
 #include "nsPIDOMWindow.h"
@@ -294,25 +295,30 @@ DocumentChannel::SetTRRMode(nsIRequest::TRRMode aTRRMode) {
 }
 
 NS_IMETHODIMP DocumentChannel::SetLoadFlags(nsLoadFlags aLoadFlags) {
-  // Setting load flags for TYPE_OBJECT is OK, so long as the channel to parent
-  // isn't opened yet, or we're only setting the `LOAD_DOCUMENT_URI` flag.
-  auto contentPolicy = mLoadInfo->GetExternalContentPolicyType();
-  if (contentPolicy == ExtContentPolicy::TYPE_OBJECT) {
-    if (mWasOpened) {
-      MOZ_DIAGNOSTIC_ASSERT(
-          aLoadFlags == (mLoadFlags | nsIChannel::LOAD_DOCUMENT_URI),
-          "After the channel has been opened, can only set the "
-          "`LOAD_DOCUMENT_URI` flag.");
-    }
+  nsLoadFlags mayChange = 0;
+  if (mLoadInfo->GetExternalContentPolicyType() ==
+      ExtContentPolicy::TYPE_OBJECT) {
+    // Setting load flags for TYPE_OBJECT is OK, so long as the channel to
+    // parent isn't opened yet, or we're only setting the `LOAD_DOCUMENT_URI`
+    // flag.
+    mayChange = mWasOpened ? LOAD_DOCUMENT_URI : ~0u;
+  } else if (!mWasOpened) {
+    // If we haven't been opened yet, allow the LoadGroup to
+    // set cache control flags inherited from the default channel.
+    mayChange = nsLoadGroup::kInheritedLoadFlags;
+  }
+
+  // Check if we're allowed to adjust these flags.
+  if ((mLoadFlags & ~mayChange) == (aLoadFlags & ~mayChange)) {
     mLoadFlags = aLoadFlags;
     return NS_OK;
   }
-
   MOZ_CRASH_UNSAFE_PRINTF(
       "DocumentChannel::SetLoadFlags: Don't set flags after creation "
       "(differing flags %x != %x)",
       (mLoadFlags ^ aLoadFlags) & mLoadFlags,
       (mLoadFlags ^ aLoadFlags) & aLoadFlags);
+  return NS_OK;
 }
 
 NS_IMETHODIMP DocumentChannel::GetOriginalURI(nsIURI** aOriginalURI) {

netwerk/ipc/DocumentChannelChild.cpp

@@ -119,6 +119,7 @@ DocumentChannelChild::AsyncOpen(nsIStreamListener* aListener) {
     case ExtContentPolicy::TYPE_DOCUMENT:
     case ExtContentPolicy::TYPE_SUBDOCUMENT: {
       DocumentCreationArgs docArgs;
+      docArgs.loadFlags() = mLoadFlags;
       docArgs.uriModified() = mUriModified;
       docArgs.isEmbeddingBlockedError() = mIsEmbeddingBlockedError;
 

netwerk/ipc/DocumentChannelParent.cpp

@@ -67,10 +67,10 @@ bool DocumentChannelParent::Init(dom::CanonicalBrowsingContext* aContext,
       const DocumentCreationArgs& docArgs = aArgs.elementCreationArgs();
 
       promise = mDocumentLoadListener->OpenDocument(
-          loadState, aArgs.cacheKey(), Some(aArgs.channelId()),
-          aArgs.asyncOpenTime(), aArgs.timing(), std::move(clientInfo),
-          docArgs.uriModified(), Some(docArgs.isEmbeddingBlockedError()),
-          contentParent, &rv);
+          loadState, docArgs.loadFlags(), aArgs.cacheKey(),
+          Some(aArgs.channelId()), aArgs.asyncOpenTime(), aArgs.timing(),
+          std::move(clientInfo), docArgs.uriModified(),
+          Some(docArgs.isEmbeddingBlockedError()), contentParent, &rv);
     } else {
       const ObjectCreationArgs& objectArgs = aArgs.elementCreationArgs();
 

netwerk/ipc/DocumentLoadListener.cpp

@@ -8,6 +8,7 @@
 #include "DocumentLoadListener.h"
 
 #include "NeckoCommon.h"
+#include "nsLoadGroup.h"
 #include "mozilla/AntiTrackingUtils.h"
 #include "mozilla/DebugOnly.h"
 #include "mozilla/Components.h"
@@ -964,7 +965,7 @@ auto DocumentLoadListener::Open(nsDocShellLoadState* aLoadState,
 }
 
 auto DocumentLoadListener::OpenDocument(
-    nsDocShellLoadState* aLoadState, uint32_t aCacheKey,
+    nsDocShellLoadState* aLoadState, nsLoadFlags aLoadFlags, uint32_t aCacheKey,
     const Maybe<uint64_t>& aChannelId, const TimeStamp& aAsyncOpenTime,
     nsDOMNavigationTiming* aTiming, Maybe<dom::ClientInfo>&& aInfo,
     bool aUriModified, Maybe<bool> aIsEmbeddingBlockedError,
@@ -977,15 +978,32 @@ auto DocumentLoadListener::OpenDocument(
   RefPtr<CanonicalBrowsingContext> browsingContext =
       GetDocumentBrowsingContext();
 
+  // As a security check, check that aLoadFlags matches what we expect. We
+  // expect them to be the same we compute on the parent, except for the load
+  // group flags.
+  {
+    const nsLoadFlags parentLoadFlags = aLoadState->CalculateChannelLoadFlags(
+        browsingContext, aUriModified, std::move(aIsEmbeddingBlockedError));
+    const nsLoadFlags differing = parentLoadFlags ^ aLoadFlags;
+    if (differing & ~nsLoadGroup::kInheritedLoadFlags) {
+#ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED
+      MOZ_CRASH_UNSAFE_PRINTF(
+          "DocumentLoadListener::OpenDocument: Unexpected load flags: "
+          "%x vs. %x (differing %x vs. %x)",
+          parentLoadFlags, aLoadFlags, differing & parentLoadFlags,
+          differing & aLoadFlags);
+#endif
+      *aRv = NS_ERROR_UNEXPECTED;
+      return nullptr;
+    }
+  }
+
   // If this is a top-level load, then rebuild the LoadInfo from scratch,
   // since the goal is to be able to initiate loads in the parent, where the
   // content process won't have provided us with an existing one.
   RefPtr<LoadInfo> loadInfo =
       CreateDocumentLoadInfo(browsingContext, aLoadState);
 
-  nsLoadFlags loadFlags = aLoadState->CalculateChannelLoadFlags(
-      browsingContext, aUriModified, std::move(aIsEmbeddingBlockedError));
-
   // Keep track of navigation for the Bounce Tracking Protection.
   if (browsingContext->IsTopContent()) {
     RefPtr<BounceTrackingState> bounceTrackingState =
@@ -1007,7 +1025,7 @@ auto DocumentLoadListener::OpenDocument(
     }
   }
 
-  return Open(aLoadState, loadInfo, loadFlags, aCacheKey, aChannelId,
+  return Open(aLoadState, loadInfo, aLoadFlags, aCacheKey, aChannelId,
               aAsyncOpenTime, aTiming, std::move(aInfo), false, aContentParent,
               aRv);
 }

netwerk/ipc/DocumentLoadListener.h

@@ -160,11 +160,12 @@ class DocumentLoadListener : public nsIInterfaceRequestor,
 
  public:
   RefPtr<OpenPromise> OpenDocument(
-      nsDocShellLoadState* aLoadState, uint32_t aCacheKey,
-      const Maybe<uint64_t>& aChannelId, const TimeStamp& aAsyncOpenTime,
-      nsDOMNavigationTiming* aTiming, Maybe<dom::ClientInfo>&& aInfo,
-      bool aUriModified, Maybe<bool> aIsEmbeddingBlockedError,
-      dom::ContentParent* aContentParent, nsresult* aRv);
+      nsDocShellLoadState* aLoadState, nsLoadFlags aLoadFlags,
+      uint32_t aCacheKey, const Maybe<uint64_t>& aChannelId,
+      const TimeStamp& aAsyncOpenTime, nsDOMNavigationTiming* aTiming,
+      Maybe<dom::ClientInfo>&& aInfo, bool aUriModified,
+      Maybe<bool> aIsEmbeddingBlockedError, dom::ContentParent* aContentParent,
+      nsresult* aRv);
 
   RefPtr<OpenPromise> OpenObject(
       nsDocShellLoadState* aLoadState, uint32_t aCacheKey,

netwerk/ipc/NeckoChannelParams.ipdlh

@@ -481,13 +481,14 @@ struct CookieStructTable
 };
 
 struct DocumentCreationArgs {
+  uint32_t loadFlags;
   bool uriModified;
   bool isEmbeddingBlockedError;
 };
 
 struct ObjectCreationArgs {
-  uint64_t embedderInnerWindowId;
   uint32_t loadFlags;
+  uint64_t embedderInnerWindowId;
   nsContentPolicyType contentPolicyType;
   bool isUrgentStart;
 };

netwerk/ipc/ParentProcessDocumentChannel.cpp

@@ -175,8 +175,8 @@ NS_IMETHODIMP ParentProcessDocumentChannel::AsyncOpen(
   RefPtr<DocumentLoadListener::OpenPromise> promise;
   if (isDocumentLoad) {
     promise = mDocumentLoadListener->OpenDocument(
-        mLoadState, mCacheKey, Some(mChannelId), TimeStamp::Now(), mTiming,
-        std::move(initialClientInfo), mUriModified,
+        mLoadState, mLoadFlags, mCacheKey, Some(mChannelId), TimeStamp::Now(),
+        mTiming, std::move(initialClientInfo), mUriModified,
         Some(mIsEmbeddingBlockedError), nullptr /* ContentParent */, &rv);
   } else {
     promise = mDocumentLoadListener->OpenObject(

testing/web-platform/tests/html/browsers/browsing-the-web/navigating-across-documents/resources/unknown-protocol-reload-crash-frame.html

@@ -0,0 +1,15 @@
+<!doctype html>
+<script>
+  (async function() {
+    if (!location.hash) {
+      await new Promise(r => {
+        window.addEventListener("hashchange", r, { once: true });
+        location.hash = "foo";
+      });
+      location.reload(true);
+    } else {
+      location.href = "unlikely-protocol://foo"
+      parent.document.documentElement.classList = "";
+    }
+  })();
+</script>

testing/web-platform/tests/html/browsers/browsing-the-web/navigating-across-documents/unknown-protocol-reload-crash.html

@@ -0,0 +1,9 @@
+<!doctype html>
+<html class="test-wait">
+  <meta charset="utf-8">
+  <link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1921972">
+  <link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">
+  <link rel="author" title="Mozilla" href="https://mozilla.org">
+  <!-- frame removes test-wait -->
+  <iframe src="resources/unknown-protocol-reload-crash-frame.html"></iframe>
+</html>