From 2f1da72bddb9c256750e70dd711f1fe15b47c11a Mon Sep 17 00:00:00 2001
From: Thomas Wisniewski <twisniewski@mozilla.com>
Date: Fri, 26 Apr 2024 16:21:41 +0000
Subject: [PATCH] Bug 1893257 - don't overwrite pragma or cache-control request
 headers per spec, and otherwise always set cache-control:max-age=0 for
 no-cache cache mode; r=valentin,necko-reviewers

Differential Revision: https://phabricator.services.mozilla.com/D208634
---
 netwerk/protocol/http/nsHttpChannel.cpp       | 27 ++++++++++++----
 .../fetch/http-cache/cache-mode.any.js.ini    | 32 -------------------
 2 files changed, 20 insertions(+), 39 deletions(-)

diff --git a/netwerk/protocol/http/nsHttpChannel.cpp b/netwerk/protocol/http/nsHttpChannel.cpp
index efb23061c8c1..9068ef6967f2 100644
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -1352,24 +1352,37 @@ nsresult nsHttpChannel::SetupChannelForTransaction() {
     // We need to send 'Pragma:no-cache' to inhibit proxy caching even if
     // no proxy is configured since we might be talking with a transparent
     // proxy, i.e. one that operates at the network level.  See bug #14772.
-    rv = mRequestHead.SetHeaderOnce(nsHttp::Pragma, "no-cache", true);
-    MOZ_ASSERT(NS_SUCCEEDED(rv));
+    // But we should not touch Pragma if Cache-Control is already set
+    // (https://fetch.spec.whatwg.org/#ref-for-concept-request-cache-mode%E2%91%A3)
+    if (!mRequestHead.HasHeader(nsHttp::Pragma)) {
+      rv = mRequestHead.SetHeaderOnce(nsHttp::Pragma, "no-cache", true);
+      MOZ_ASSERT(NS_SUCCEEDED(rv));
+    }
     // If we're configured to speak HTTP/1.1 then also send 'Cache-control:
-    // no-cache'
-    if (mRequestHead.Version() >= HttpVersion::v1_1) {
+    // no-cache'. But likewise don't touch Cache-Control if it's already set.
+    if (mRequestHead.Version() >= HttpVersion::v1_1 &&
+        !mRequestHead.HasHeader(nsHttp::Cache_Control)) {
       rv = mRequestHead.SetHeaderOnce(nsHttp::Cache_Control, "no-cache", true);
       MOZ_ASSERT(NS_SUCCEEDED(rv));
     }
-  } else if ((mLoadFlags & VALIDATE_ALWAYS) && !LoadCacheEntryIsWriteOnly()) {
+  } else if (mLoadFlags & VALIDATE_ALWAYS) {
     // We need to send 'Cache-Control: max-age=0' to force each cache along
     // the path to the origin server to revalidate its own entry, if any,
     // with the next cache or server.  See bug #84847.
     //
     // If we're configured to speak HTTP/1.0 then just send 'Pragma: no-cache'
+    //
+    // But don't send the headers if they're already set:
+    // https://fetch.spec.whatwg.org/#ref-for-concept-request-cache-mode%E2%91%A2
     if (mRequestHead.Version() >= HttpVersion::v1_1) {
-      rv = mRequestHead.SetHeaderOnce(nsHttp::Cache_Control, "max-age=0", true);
+      if (!mRequestHead.HasHeader(nsHttp::Cache_Control)) {
+        rv = mRequestHead.SetHeaderOnce(nsHttp::Cache_Control, "max-age=0",
+                                        true);
+      }
     } else {
-      rv = mRequestHead.SetHeaderOnce(nsHttp::Pragma, "no-cache", true);
+      if (!mRequestHead.HasHeader(nsHttp::Pragma)) {
+        rv = mRequestHead.SetHeaderOnce(nsHttp::Pragma, "no-cache", true);
+      }
     }
     MOZ_ASSERT(NS_SUCCEEDED(rv));
   }
diff --git a/testing/web-platform/meta/fetch/http-cache/cache-mode.any.js.ini b/testing/web-platform/meta/fetch/http-cache/cache-mode.any.js.ini
index 996dbab4b0f2..fe0b49f0608f 100644
--- a/testing/web-platform/meta/fetch/http-cache/cache-mode.any.js.ini
+++ b/testing/web-platform/meta/fetch/http-cache/cache-mode.any.js.ini
@@ -1,50 +1,18 @@
 [cache-mode.any.sharedworker.html]
   expected:
     if (os == "android") and fission: [OK, TIMEOUT]
-  [Fetch doesn't touch Cache-Control when cache mode is no-store and Cache-Control is already present]
-    expected: FAIL
-
-  [Fetch sends Cache-Control: max-age=0 when cache mode is no-cache]
-    expected: FAIL
-
-  [Fetch doesn't touch Pragma when cache mode is no-store and Pragma is already present]
-    expected: FAIL
 
 
 [cache-mode.any.worker.html]
   expected:
     if (os == "android") and fission: [OK, TIMEOUT]
-  [Fetch doesn't touch Cache-Control when cache mode is no-store and Cache-Control is already present]
-    expected: FAIL
-
-  [Fetch sends Cache-Control: max-age=0 when cache mode is no-cache]
-    expected: FAIL
-
-  [Fetch doesn't touch Pragma when cache mode is no-store and Pragma is already present]
-    expected: FAIL
 
 
 [cache-mode.any.html]
   expected:
     if (os == "android") and fission: [OK, TIMEOUT]
-  [Fetch doesn't touch Cache-Control when cache mode is no-store and Cache-Control is already present]
-    expected: FAIL
-
-  [Fetch sends Cache-Control: max-age=0 when cache mode is no-cache]
-    expected: FAIL
-
-  [Fetch doesn't touch Pragma when cache mode is no-store and Pragma is already present]
-    expected: FAIL
 
 
 [cache-mode.any.serviceworker.html]
   expected:
     if (os == "android") and fission: [OK, TIMEOUT]
-  [Fetch doesn't touch Cache-Control when cache mode is no-store and Cache-Control is already present]
-    expected: FAIL
-
-  [Fetch sends Cache-Control: max-age=0 when cache mode is no-cache]
-    expected: FAIL
-
-  [Fetch doesn't touch Pragma when cache mode is no-store and Pragma is already present]
-    expected: FAIL