Bug 1733558 - stop duplicating append redirect history entry logic everywhere, r=ckerschb,necko-reviewers,valentin

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

dom/security/test/gtest/TestUnexpectedPrivilegedLoads.cpp

@@ -26,7 +26,6 @@
 #include "nsILoadInfo.h"
 #include "nsNetUtil.h"
 #include "nsStringFwd.h"
-#include "mozilla/nsRedirectHistoryEntry.h"
 
 using namespace mozilla;
 using namespace TelemetryTestHelpers;
@@ -188,10 +187,12 @@ TEST_F(TelemetryTestFixture, UnexpectedPrivilegedLoadsTelemetryTest) {
                 "https://www.analytics.example/analytics.js"_ns);
       nsCOMPtr<nsIPrincipal> redirPrincipal =
           BasePrincipal::CreateContentPrincipal(redirUri, OriginAttributes());
-      nsCOMPtr<nsIRedirectHistoryEntry> entry =
-          new net::nsRedirectHistoryEntry(redirPrincipal, nullptr, ""_ns);
+      nsCOMPtr<nsIChannel> redirectChannel;
+      Unused << service->NewChannelFromURI(redirUri, nullptr, redirPrincipal,
+                                           nullptr, 0, currentTest.contentType,
+                                           getter_AddRefs(redirectChannel));
 
-      mockLoadInfo->AppendRedirectHistoryEntry(entry, false);
+      mockLoadInfo->AppendRedirectHistoryEntry(redirectChannel, false);
     }
 
     // this will record the event

netwerk/base/LoadInfo.cpp

@@ -30,6 +30,8 @@
 #include "nsIContentSecurityPolicy.h"
 #include "nsIDocShell.h"
 #include "mozilla/dom/Document.h"
+#include "nsIHttpChannel.h"
+#include "nsIHttpChannelInternal.h"
 #include "nsIInterfaceRequestorUtils.h"
 #include "nsIScriptElement.h"
 #include "nsISupportsImpl.h"
@@ -1326,14 +1328,41 @@ LoadInfo::GetInitialSecurityCheckDone(bool* aResult) {
 }
 
 NS_IMETHODIMP
-LoadInfo::AppendRedirectHistoryEntry(nsIRedirectHistoryEntry* aEntry,
+LoadInfo::AppendRedirectHistoryEntry(nsIChannel* aChannel,
                                      bool aIsInternalRedirect) {
-  NS_ENSURE_ARG(aEntry);
+  NS_ENSURE_ARG(aChannel);
   MOZ_ASSERT(NS_IsMainThread());
 
-  mRedirectChainIncludingInternalRedirects.AppendElement(aEntry);
+  nsCOMPtr<nsIPrincipal> uriPrincipal;
+  nsIScriptSecurityManager* sm = nsContentUtils::GetSecurityManager();
+  sm->GetChannelURIPrincipal(aChannel, getter_AddRefs(uriPrincipal));
+
+  nsCOMPtr<nsIURI> referrer;
+  nsCString remoteAddress;
+
+  nsCOMPtr<nsIHttpChannel> httpChannel(do_QueryInterface(aChannel));
+  if (httpChannel) {
+    nsCOMPtr<nsIReferrerInfo> referrerInfo;
+    Unused << httpChannel->GetReferrerInfo(getter_AddRefs(referrerInfo));
+    if (referrerInfo) {
+      referrer = referrerInfo->GetComputedReferrer();
+    }
+  }
+
+  // ClassifierDummyChannel implements this, but not nsIHttpChannel,
+  // whereas NullHttpChannel implements nsIHttpChannel but not this, so
+  // we can't make assumptions by nesting these QIs.
+  nsCOMPtr<nsIHttpChannelInternal> intChannel(do_QueryInterface(aChannel));
+  if (intChannel) {
+    Unused << intChannel->GetRemoteAddress(remoteAddress);
+  }
+
+  nsCOMPtr<nsIRedirectHistoryEntry> entry =
+      new nsRedirectHistoryEntry(uriPrincipal, referrer, remoteAddress);
+
+  mRedirectChainIncludingInternalRedirects.AppendElement(entry);
   if (!aIsInternalRedirect) {
-    mRedirectChain.AppendElement(aEntry);
+    mRedirectChain.AppendElement(entry);
   }
   return NS_OK;
 }

netwerk/base/TRRLoadInfo.cpp

@@ -391,7 +391,7 @@ TRRLoadInfo::GetInitialSecurityCheckDone(bool* aResult) {
 }
 
 NS_IMETHODIMP
-TRRLoadInfo::AppendRedirectHistoryEntry(nsIRedirectHistoryEntry* aEntry,
+TRRLoadInfo::AppendRedirectHistoryEntry(nsIChannel* aChannelToDeriveFrom,
                                         bool aIsInternalRedirect) {
   return NS_ERROR_NOT_IMPLEMENTED;
 }

netwerk/base/nsBaseChannel.cpp

@@ -78,19 +78,11 @@ nsresult nsBaseChannel::Redirect(nsIChannel* newChannel, uint32_t redirectFlags,
       static_cast<net::LoadInfo*>(mLoadInfo.get())
           ->CloneWithNewSecFlags(secFlags);
 
-  nsCOMPtr<nsIPrincipal> uriPrincipal;
-  nsIScriptSecurityManager* sm = nsContentUtils::GetSecurityManager();
-  sm->GetChannelURIPrincipal(this, getter_AddRefs(uriPrincipal));
   bool isInternalRedirect =
       (redirectFlags & (nsIChannelEventSink::REDIRECT_INTERNAL |
                         nsIChannelEventSink::REDIRECT_STS_UPGRADE));
 
-  // nsBaseChannel hst no thing to do with HttpBaseChannel, we would not care
-  // about referrer and remote address in this case
-  nsCOMPtr<nsIRedirectHistoryEntry> entry =
-      new net::nsRedirectHistoryEntry(uriPrincipal, nullptr, ""_ns);
-
-  newLoadInfo->AppendRedirectHistoryEntry(entry, isInternalRedirect);
+  newLoadInfo->AppendRedirectHistoryEntry(this, isInternalRedirect);
 
   // Ensure the channel's loadInfo's result principal URI so that it's
   // either non-null or updated to the redirect target URI.

netwerk/base/nsILoadInfo.idl

@@ -857,12 +857,11 @@ interface nsILoadInfo : nsISupports
    * the channels got redirected] to the loadinfo, so that at every point this
    * array provides us information about all the redirects this channel went
    * through.
-   * @param entry, the nsIRedirectHistoryEntry before the channel
-   *         got redirected.
+   * @param channelToDeriveFrom the channel being redirected
    * @param aIsInternalRedirect should be true if the channel is going
    *        through an internal redirect, otherwise false.
    */
-  void appendRedirectHistoryEntry(in nsIRedirectHistoryEntry entry,
+  void appendRedirectHistoryEntry(in nsIChannel channelToDeriveFrom,
                                   in boolean isInternalRedirect);
 
   /**

netwerk/ipc/DocumentLoadListener.cpp

@@ -1399,14 +1399,7 @@ void DocumentLoadListener::SerializeRedirectData(
     redirectLoadInfo =
         static_cast<mozilla::net::LoadInfo*>(channelLoadInfo.get())->Clone();
 
-    nsCOMPtr<nsIPrincipal> uriPrincipal;
-    nsIScriptSecurityManager* sm = nsContentUtils::GetSecurityManager();
-    sm->GetChannelURIPrincipal(mChannel, getter_AddRefs(uriPrincipal));
-
-    nsCOMPtr<nsIRedirectHistoryEntry> entry =
-        new nsRedirectHistoryEntry(uriPrincipal, nullptr, ""_ns);
-
-    redirectLoadInfo->AppendRedirectHistoryEntry(entry, true);
+    redirectLoadInfo->AppendRedirectHistoryEntry(mChannel, true);
   }
 
   const Maybe<ClientInfo>& reservedClientInfo =

netwerk/protocol/http/HttpBaseChannel.cpp

@@ -3719,30 +3719,6 @@ void HttpBaseChannel::ClearConsoleReports() {
   mReportCollector->ClearConsoleReports();
 }
 
-nsIPrincipal* HttpBaseChannel::GetURIPrincipal() {
-  if (mPrincipal) {
-    return mPrincipal;
-  }
-
-  nsIScriptSecurityManager* securityManager =
-      nsContentUtils::GetSecurityManager();
-
-  if (!securityManager) {
-    LOG(("HttpBaseChannel::GetURIPrincipal: No security manager [this=%p]",
-         this));
-    return nullptr;
-  }
-
-  securityManager->GetChannelURIPrincipal(this, getter_AddRefs(mPrincipal));
-  if (!mPrincipal) {
-    LOG(("HttpBaseChannel::GetURIPrincipal: No channel principal [this=%p]",
-         this));
-    return nullptr;
-  }
-
-  return mPrincipal;
-}
-
 bool HttpBaseChannel::IsNavigation() {
   return LoadForceMainDocumentChannel() || (mLoadFlags & LOAD_DOCUMENT_URI);
 }
@@ -3939,17 +3915,7 @@ already_AddRefed<nsILoadInfo> HttpBaseChannel::CloneLoadInfoForRedirect(
     newLoadInfo->ResetSandboxedNullPrincipalID();
   }
 
-  nsCString remoteAddress;
-  Unused << GetRemoteAddress(remoteAddress);
-  nsCOMPtr<nsIURI> referrer;
-  if (mReferrerInfo) {
-    referrer = mReferrerInfo->GetComputedReferrer();
-  }
-
-  nsCOMPtr<nsIRedirectHistoryEntry> entry =
-      new nsRedirectHistoryEntry(GetURIPrincipal(), referrer, remoteAddress);
-
-  newLoadInfo->AppendRedirectHistoryEntry(entry, isInternalRedirect);
+  newLoadInfo->AppendRedirectHistoryEntry(this, isInternalRedirect);
 
   return newLoadInfo.forget();
 }

netwerk/protocol/http/HttpBaseChannel.h

@@ -575,9 +575,6 @@ class HttpBaseChannel : public nsHashPropertyBag,
   // Checks whether or not aURI and mOriginalURI share the same domain.
   virtual bool SameOriginWithOriginalUri(nsIURI* aURI);
 
-  // GetPrincipal Returns the channel's URI principal.
-  nsIPrincipal* GetURIPrincipal();
-
   [[nodiscard]] bool BypassServiceWorker() const;
 
   // Returns true if this channel should intercept the network request and

netwerk/protocol/http/HttpChannelChild.cpp

@@ -1537,17 +1537,7 @@ void HttpChannelChild::CleanupRedirectingChannel(nsresult rv) {
   if (mLoadGroup) mLoadGroup->RemoveRequest(this, nullptr, NS_BINDING_ABORTED);
 
   if (NS_SUCCEEDED(rv)) {
-    nsCString remoteAddress;
-    Unused << GetRemoteAddress(remoteAddress);
-    nsCOMPtr<nsIURI> referrer;
-    if (mReferrerInfo) {
-      referrer = mReferrerInfo->GetComputedReferrer();
-    }
-
-    nsCOMPtr<nsIRedirectHistoryEntry> entry =
-        new nsRedirectHistoryEntry(GetURIPrincipal(), referrer, remoteAddress);
-
-    mLoadInfo->AppendRedirectHistoryEntry(entry, false);
+    mLoadInfo->AppendRedirectHistoryEntry(this, false);
   } else {
     NS_WARNING("CompleteRedirectSetup failed, HttpChannelChild already open?");
   }