Bug 1849864 - Don't recalculate the userAgent header if it has been modified. r=tjr,jesup,necko-reviewers,devtools-reviewers

The userAgent header can be modified in several ways, such as using the
header field to set a custom userAgent header for a fetch request. We
want to preserve the custom header, so we shouldn't recalculate the
userAgent header if it's been overridden after the channel was created.
Otherwise, the custom header won't work.

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

devtools/client/netmonitor/test/browser_net_copy_as_fetch.js

@@ -18,12 +18,12 @@ add_task(async function () {
   await testClipboardContent(`await fetch("https://example.com/browser/devtools/client/netmonitor/test/sjs_simple-test-server.sjs", {
     "credentials": "omit",
     "headers": {
-        "User-Agent": "${navigator.userAgent}",
         "Accept": "*/*",
         "Accept-Language": "en-US",
         "X-Custom-Header-1": "Custom value",
         "X-Custom-Header-2": "8.8.8.8",
         "X-Custom-Header-3": "Mon, 3 Mar 2014 11:11:11 GMT",
+        "User-Agent": "${navigator.userAgent}",
         "Sec-Fetch-Dest": "empty",
         "Sec-Fetch-Mode": "cors",
         "Sec-Fetch-Site": "same-origin",

devtools/client/netmonitor/test/browser_net_copy_headers.js

@@ -41,10 +41,10 @@ add_task(async function () {
   const EXPECTED_REQUEST_HEADERS = [
     `${method} ${SIMPLE_URL.split("example.com")[1]} ${httpVersion}`,
     "Host: example.com",
-    "User-Agent: " + navigator.userAgent + "",
     "Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,*/*;q=0.8",
     "Accept-Language: " + navigator.languages.join(",") + ";q=0.5",
     "Accept-Encoding: gzip, deflate",
+    "User-Agent: " + navigator.userAgent + "",
     "Connection: keep-alive",
     "Upgrade-Insecure-Requests: 1",
     "Pragma: no-cache",

devtools/client/netmonitor/test/browser_net_headers_sorted.js

@@ -134,10 +134,10 @@ async function verifyRawHeaders(monitor) {
 
   const expectedRequestHeaders = [
     "Host",
-    "User-Agent",
     "Accept",
     "Accept-Language",
     "Accept-Encoding",
+    "User-Agent",
     "Connection",
     "Cookie",
     "Upgrade-Insecure-Requests",

devtools/client/netmonitor/test/browser_net_use_as_fetch.js

@@ -18,12 +18,12 @@ add_task(async function () {
   await testConsoleInput(`await fetch("http://example.com/browser/devtools/client/netmonitor/test/sjs_simple-test-server.sjs", {
     "credentials": "omit",
     "headers": {
-        "User-Agent": "${navigator.userAgent}",
         "Accept": "*/*",
         "Accept-Language": "en-US",
         "X-Custom-Header-1": "Custom value",
         "X-Custom-Header-2": "8.8.8.8",
         "X-Custom-Header-3": "Mon, 3 Mar 2014 11:11:11 GMT",
+        "User-Agent": "${navigator.userAgent}",
         "Pragma": "no-cache",
         "Cache-Control": "no-cache"
     },

netwerk/ipc/NeckoChannelParams.ipdlh

@@ -365,6 +365,7 @@ struct HttpChannelOpenArgs
   uint8_t                     redirectionLimit;
   nsString                    classicScriptHintCharset;
   nsString                    documentCharacterSet;
+  bool                        isUserAgentHeaderModified;
 };
 
 struct HttpChannelConnectArgs

netwerk/protocol/http/HttpBaseChannel.cpp

@@ -258,6 +258,7 @@ HttpBaseChannel::HttpBaseChannel()
   StoreAllRedirectsSameOrigin(true);
   StoreAllRedirectsPassTimingAllowCheck(true);
   StoreUpgradableToSecure(true);
+  StoreIsUserAgentHeaderModified(false);
 
   this->mSelfAddr.inet = {};
   this->mPeerAddr.inet = {};
@@ -1944,6 +1945,11 @@ HttpBaseChannel::SetRequestHeader(const nsACString& aHeader,
     return NS_ERROR_INVALID_ARG;
   }
 
+  // Mark that the User-Agent header has been modified.
+  if (nsHttp::ResolveAtom(aHeader) == nsHttp::User_Agent) {
+    StoreIsUserAgentHeaderModified(true);
+  }
+
   return mRequestHead.SetHeader(aHeader, flatValue, aMerge);
 }
 
@@ -1977,6 +1983,11 @@ HttpBaseChannel::SetEmptyRequestHeader(const nsACString& aHeader) {
     return NS_ERROR_INVALID_ARG;
   }
 
+  // Mark that the User-Agent header has been modified.
+  if (nsHttp::ResolveAtom(aHeader) == nsHttp::User_Agent) {
+    StoreIsUserAgentHeaderModified(true);
+  }
+
   return mRequestHead.SetEmptyHeader(aHeader);
 }
 
@@ -2092,6 +2103,19 @@ HttpBaseChannel::SetIsOCSP(bool value) {
   return NS_OK;
 }
 
+NS_IMETHODIMP
+HttpBaseChannel::GetIsUserAgentHeaderModified(bool* value) {
+  NS_ENSURE_ARG_POINTER(value);
+  *value = LoadIsUserAgentHeaderModified();
+  return NS_OK;
+}
+
+NS_IMETHODIMP
+HttpBaseChannel::SetIsUserAgentHeaderModified(bool value) {
+  StoreIsUserAgentHeaderModified(value);
+  return NS_OK;
+}
+
 NS_IMETHODIMP
 HttpBaseChannel::GetRedirectionLimit(uint32_t* value) {
   NS_ENSURE_ARG_POINTER(value);
@@ -4978,6 +5002,13 @@ nsresult HttpBaseChannel::SetupReplacementChannel(nsIURI* newURI,
     }
   }
 
+  // convery the IsUserAgentHeaderModified value.
+  if (httpInternal) {
+    rv = httpInternal->SetIsUserAgentHeaderModified(
+        LoadIsUserAgentHeaderModified());
+    MOZ_ASSERT(NS_SUCCEEDED(rv));
+  }
+
   // share the request context - see bug 1236650
   rv = httpChannel->SetRequestContextID(mRequestContextID);
   MOZ_ASSERT(NS_SUCCEEDED(rv));

netwerk/protocol/http/HttpBaseChannel.h

@@ -362,6 +362,9 @@ class HttpBaseChannel : public nsHashPropertyBag,
   NS_IMETHOD SetEarlyHintLinkType(uint32_t aEarlyHintLinkType) override;
   NS_IMETHOD GetEarlyHintLinkType(uint32_t* aEarlyHintLinkType) override;
 
+  NS_IMETHOD SetIsUserAgentHeaderModified(bool value) override;
+  NS_IMETHOD GetIsUserAgentHeaderModified(bool* value) override;
+
   NS_IMETHOD SetClassicScriptHintCharset(
       const nsAString& aClassicScriptHintCharset) override;
   NS_IMETHOD GetClassicScriptHintCharset(
@@ -956,7 +959,11 @@ class HttpBaseChannel : public nsHashPropertyBag,
 
     // Indicate whether the response of this channel is coming from
     // socket process.
-    (uint32_t, LoadedBySocketProcess, 1)
+    (uint32_t, LoadedBySocketProcess, 1),
+
+    // Indicates whether the user-agent header has been modifed since the channel
+    // was created.
+    (uint32_t, IsUserAgentHeaderModified, 1)
   ))
   // clang-format on
 

netwerk/protocol/http/HttpChannelChild.cpp

@@ -2483,6 +2483,8 @@ nsresult HttpChannelChild::ContinueAsyncOpen() {
 
   openArgs.classicScriptHintCharset() = mClassicScriptHintCharset;
 
+  openArgs.isUserAgentHeaderModified() = LoadIsUserAgentHeaderModified();
+
   RefPtr<Document> doc;
   mLoadInfo->GetLoadingDocument(getter_AddRefs(doc));
 
@@ -2565,6 +2567,11 @@ HttpChannelChild::SetRequestHeader(const nsACString& aHeader,
   RequestHeaderTuple* tuple = mClientSetRequestHeaders.AppendElement();
   if (!tuple) return NS_ERROR_OUT_OF_MEMORY;
 
+  // Mark that the User-Agent header has been modified.
+  if (nsHttp::ResolveAtom(aHeader) == nsHttp::User_Agent) {
+    StoreIsUserAgentHeaderModified(true);
+  }
+
   tuple->mHeader = aHeader;
   tuple->mValue = aValue;
   tuple->mMerge = aMerge;
@@ -2581,6 +2588,11 @@ HttpChannelChild::SetEmptyRequestHeader(const nsACString& aHeader) {
   RequestHeaderTuple* tuple = mClientSetRequestHeaders.AppendElement();
   if (!tuple) return NS_ERROR_OUT_OF_MEMORY;
 
+  // Mark that the User-Agent header has been modified.
+  if (nsHttp::ResolveAtom(aHeader) == nsHttp::User_Agent) {
+    StoreIsUserAgentHeaderModified(true);
+  }
+
   tuple->mHeader = aHeader;
   tuple->mMerge = false;
   tuple->mEmpty = true;

netwerk/protocol/http/HttpChannelParent.cpp

@@ -177,7 +177,7 @@ bool HttpChannelParent::Init(const HttpChannelCreationArgs& aArgs) {
           a.handleFetchEventStart(), a.handleFetchEventEnd(),
           a.forceMainDocumentChannel(), a.navigationStartTimeStamp(),
           a.earlyHintPreloaderId(), a.classicScriptHintCharset(),
-          a.documentCharacterSet());
+          a.documentCharacterSet(), a.isUserAgentHeaderModified());
     }
     case HttpChannelCreationArgs::THttpChannelConnectArgs: {
       const HttpChannelConnectArgs& cArgs = aArgs.get_HttpChannelConnectArgs();
@@ -444,7 +444,8 @@ bool HttpChannelParent::DoAsyncOpen(
     const TimeStamp& aNavigationStartTimeStamp,
     const uint64_t& aEarlyHintPreloaderId,
     const nsAString& aClassicScriptHintCharset,
-    const nsAString& aDocumentCharacterSet) {
+    const nsAString& aDocumentCharacterSet,
+    const bool& aIsUserAgentHeaderModified) {
   MOZ_ASSERT(aURI, "aURI should not be NULL");
 
   if (aEarlyHintPreloaderId) {
@@ -577,6 +578,8 @@ bool HttpChannelParent::DoAsyncOpen(
     }
   }
 
+  httpChannel->SetIsUserAgentHeaderModified(aIsUserAgentHeaderModified);
+
   RefPtr<ParentChannelListener> parentListener = new ParentChannelListener(
       this, mBrowserParent ? mBrowserParent->GetBrowsingContext() : nullptr);
 

netwerk/protocol/http/HttpChannelParent.h

@@ -171,7 +171,8 @@ class HttpChannelParent final : public nsIInterfaceRequestor,
       const TimeStamp& aNavigationStartTimeStamp,
       const uint64_t& aEarlyHintPreloaderId,
       const nsAString& aClassicScriptHintCharset,
-      const nsAString& aDocumentCharacterSet);
+      const nsAString& aDocumentCharacterSet,
+      const bool& aIsUserAgentHeaderModified);
 
   virtual mozilla::ipc::IPCResult RecvSetPriority(
       const int16_t& priority) override;

netwerk/protocol/http/nsHttpChannel.cpp

@@ -6133,14 +6133,23 @@ nsHttpChannel::AsyncOpen(nsIStreamListener* aListener) {
 
   AntiTrackingUtils::UpdateAntiTrackingInfoForChannel(this);
 
-  // Recalculate the userAgent header after the AntiTrackingInfo gets updated
-  // because we can only know whether the site is exempted from fingerprinting
-  // protection after we have the AntiTracking Info.
-  rv = mRequestHead.SetHeader(
-      nsHttp::User_Agent,
-      gHttpHandler->UserAgent(nsContentUtils::ShouldResistFingerprinting(
-          this, RFPTarget::HttpUserAgent)));
-  MOZ_ASSERT(NS_SUCCEEDED(rv));
+  // Recalculate the default userAgent header after the AntiTrackingInfo gets
+  // updated because we can only know whether the site is exempted from
+  // fingerprinting protection after we have the AntiTracking Info.
+  //
+  // Note that we don't recalculate the header if it has been modified since the
+  // channel was created because we want to preserve the modified header.
+  if (!LoadIsUserAgentHeaderModified()) {
+    rv = mRequestHead.ClearHeader(nsHttp::User_Agent);
+    MOZ_ASSERT(NS_SUCCEEDED(rv));
+
+    rv = mRequestHead.SetHeader(
+        nsHttp::User_Agent,
+        gHttpHandler->UserAgent(nsContentUtils::ShouldResistFingerprinting(
+            this, RFPTarget::HttpUserAgent)),
+        false, nsHttpHeaderArray::eVarietyRequestDefault);
+    MOZ_ASSERT(NS_SUCCEEDED(rv));
+  }
 
   if (WaitingForTailUnblock()) {
     // This channel is marked as Tail and is part of a request context

netwerk/protocol/http/nsIHttpChannelInternal.idl

@@ -510,4 +510,13 @@ interface nsIHttpChannelInternal : nsISupports
      * 103 response.
      */
     [must_use] attribute unsigned long earlyHintLinkType;
+
+    /**
+     * Indicates whether the User-Agent request header has been modified since
+     * the channel was created. This value will be used to decide if we need to
+     * recalculate the User-Agent header for fingerprinting protection. We won't
+     * recalculate the User-Agent header if it has been modified to preserve the
+     * overridden header value.
+     */
+    [must_use] attribute boolean isUserAgentHeaderModified;
 };