Bug 1120715 - Part 5: Treat a default cache mode Request with a revalidation header as no-store; r=bkelly
authorEhsan Akhgari <ehsan@mozilla.com>
Wed, 02 Mar 2016 18:09:30 -0500
changeset 339607 1a97c346298fc7a658beec5c3b8e69c209b4fe92
parent 339606 f58317eb7f23592fbdb6586e889dba513c346c09
child 339608 5cce63231d48e35bd68ece3419996c530237e5d7
push id12762
push userbmo:rail@mozilla.com
push dateFri, 11 Mar 2016 19:47:45 +0000
reviewersbkelly
bugs1120715
milestone48.0a1
Bug 1120715 - Part 5: Treat a default cache mode Request with a revalidation header as no-store; r=bkelly
dom/fetch/FetchDriver.cpp
dom/fetch/InternalHeaders.cpp
dom/fetch/InternalHeaders.h
dom/fetch/InternalRequest.cpp
dom/fetch/InternalRequest.h
testing/web-platform/tests/fetch/api/request/request-cache.html
testing/web-platform/tests/fetch/api/request/resources/cache.py
--- a/dom/fetch/FetchDriver.cpp
+++ b/dom/fetch/FetchDriver.cpp
@@ -240,22 +240,16 @@ FetchDriver::HttpFetch()
   {
     nsCOMPtr<nsIInterfaceRequestor> notificationCallbacks;
     chan->GetNotificationCallbacks(getter_AddRefs(notificationCallbacks));
     MOZ_ASSERT(!notificationCallbacks);
   }
 #endif
   chan->SetNotificationCallbacks(this);
 
-  // FIXME(nsm): Bug 1120715.
-  // Step 3.4 "If request's cache mode is default and request's header list
-  // contains a header named `If-Modified-Since`, `If-None-Match`,
-  // `If-Unmodified-Since`, `If-Match`, or `If-Range`, set request's cache mode
-  // to no-store."
-
   // Step 3.5 begins "HTTP network or cache fetch".
   // HTTP network or cache fetch
   // ---------------------------
   // Step 1 "Let HTTPRequest..." The channel is the HTTPRequest.
   nsCOMPtr<nsIHttpChannel> httpChan = do_QueryInterface(chan);
   if (httpChan) {
     // Copy the method.
     nsAutoCString method;
@@ -327,22 +321,21 @@ FetchDriver::HttpFetch()
     // Credentials checks for CORS are handled by nsCORSListenerProxy,
 
     nsCOMPtr<nsIHttpChannelInternal> internalChan = do_QueryInterface(httpChan);
 
     // Conversion between enumerations is safe due to static asserts in
     // dom/workers/ServiceWorkerManager.cpp
     internalChan->SetCorsMode(static_cast<uint32_t>(mRequest->Mode()));
     internalChan->SetRedirectMode(static_cast<uint32_t>(mRequest->GetRedirectMode()));
+    mRequest->MaybeSkipCacheIfPerformingRevalidation();
     internalChan->SetFetchCacheMode(static_cast<uint32_t>(mRequest->GetCacheMode()));
   }
 
   // Step 5. Proxy authentication will be handled by Necko.
-  // FIXME(nsm): Bug 1120715.
-  // Step 7-10. "If request's cache mode is neither no-store nor reload..."
 
   // Continue setting up 'HTTPRequest'. Content-Type and body data.
   nsCOMPtr<nsIUploadChannel2> uploadChan = do_QueryInterface(chan);
   if (uploadChan) {
     nsAutoCString contentType;
     ErrorResult result;
     mRequest->Headers()->Get(NS_LITERAL_CSTRING("content-type"), contentType, result);
     // This is an error because the Request constructor explicitly extracts and
--- a/dom/fetch/InternalHeaders.cpp
+++ b/dom/fetch/InternalHeaders.cpp
@@ -169,16 +169,27 @@ InternalHeaders::IsSimpleHeader(const ns
   // from being set or appended.
   return aName.EqualsLiteral("accept") ||
          aName.EqualsLiteral("accept-language") ||
          aName.EqualsLiteral("content-language") ||
          (aName.EqualsLiteral("content-type") &&
           nsContentUtils::IsAllowedNonCorsContentType(aValue));
 }
 
+// static
+bool
+InternalHeaders::IsRevalidationHeader(const nsACString& aName)
+{
+  return aName.EqualsLiteral("if-modified-since") ||
+         aName.EqualsLiteral("if-none-match") ||
+         aName.EqualsLiteral("if-unmodified-since") ||
+         aName.EqualsLiteral("if-match") ||
+         aName.EqualsLiteral("if-range");
+}
+
 //static
 bool
 InternalHeaders::IsInvalidName(const nsACString& aName, ErrorResult& aRv)
 {
   if (!NS_IsValidHTTPToken(aName)) {
     NS_ConvertUTF8toUTF16 label(aName);
     aRv.ThrowTypeError<MSG_INVALID_HEADER_NAME>(label);
     return true;
@@ -278,16 +289,28 @@ InternalHeaders::HasOnlySimpleHeaders() 
     if (!IsSimpleHeader(mList[i].mName, mList[i].mValue)) {
       return false;
     }
   }
 
   return true;
 }
 
+bool
+InternalHeaders::HasRevalidationHeaders() const
+{
+  for (uint32_t i = 0; i < mList.Length(); ++i) {
+    if (IsRevalidationHeader(mList[i].mName)) {
+      return true;
+    }
+  }
+
+  return false;
+}
+
 // static
 already_AddRefed<InternalHeaders>
 InternalHeaders::BasicHeaders(InternalHeaders* aHeaders)
 {
   RefPtr<InternalHeaders> basic = new InternalHeaders(*aHeaders);
   ErrorResult result;
   // The Set-Cookie headers cannot be invalid mutable headers, so the Delete
   // must succeed.
--- a/dom/fetch/InternalHeaders.h
+++ b/dom/fetch/InternalHeaders.h
@@ -94,16 +94,18 @@ public:
   void SetGuard(HeadersGuardEnum aGuard, ErrorResult& aRv);
 
   void Fill(const InternalHeaders& aInit, ErrorResult& aRv);
   void Fill(const Sequence<Sequence<nsCString>>& aInit, ErrorResult& aRv);
   void Fill(const MozMap<nsCString>& aInit, ErrorResult& aRv);
 
   bool HasOnlySimpleHeaders() const;
 
+  bool HasRevalidationHeaders() const;
+
   static already_AddRefed<InternalHeaders>
   BasicHeaders(InternalHeaders* aHeaders);
 
   static already_AddRefed<InternalHeaders>
   CORSHeaders(InternalHeaders* aHeaders);
 
   void
   GetEntries(nsTArray<InternalHeaders::Entry>& aEntries) const;
@@ -137,14 +139,16 @@ private:
            IsImmutable(aRv) ||
            IsForbiddenRequestHeader(aName) ||
            IsForbiddenRequestNoCorsHeader(aName, aValue) ||
            IsForbiddenResponseHeader(aName);
   }
 
   static bool IsSimpleHeader(const nsACString& aName,
                              const nsACString& aValue);
+
+  static bool IsRevalidationHeader(const nsACString& aName);
 };
 
 } // namespace dom
 } // namespace mozilla
 
 #endif // mozilla_dom_InternalHeaders_h
--- a/dom/fetch/InternalRequest.cpp
+++ b/dom/fetch/InternalRequest.cpp
@@ -358,10 +358,19 @@ InternalRequest::MapChannelToRequestCred
   } else if (cookiePolicy == nsILoadInfo::SEC_COOKIES_SAME_ORIGIN) {
     return RequestCredentials::Same_origin;
   }
 
   MOZ_ASSERT_UNREACHABLE("Unexpected cookie policy!");
   return RequestCredentials::Same_origin;
 }
 
+void
+InternalRequest::MaybeSkipCacheIfPerformingRevalidation()
+{
+  if (mCacheMode == RequestCache::Default &&
+      mHeaders->HasRevalidationHeaders()) {
+    mCacheMode = RequestCache::No_store;
+  }
+}
+
 } // namespace dom
 } // namespace mozilla
--- a/dom/fetch/InternalRequest.h
+++ b/dom/fetch/InternalRequest.h
@@ -420,16 +420,19 @@ public:
   IsNavigationRequest() const;
 
   bool
   IsWorkerRequest() const;
 
   bool
   IsClientRequest() const;
 
+  void
+  MaybeSkipCacheIfPerformingRevalidation();
+
   static RequestMode
   MapChannelToRequestMode(nsIChannel* aChannel);
 
   static RequestCredentials
   MapChannelToRequestCredentials(nsIChannel* aChannel);
 
 private:
   // Does not copy mBodyStream.  Use fallible Clone() for complete copy.
--- a/testing/web-platform/tests/fetch/api/request/request-cache.html
+++ b/testing/web-platform/tests/fetch/api/request/request-cache.html
@@ -124,16 +124,176 @@
       {
         name: 'RequestCache "no-store" mode does not store the response in the cache',
         state: "fresh",
         request_cache: ["no-store", "default"],
         expected_validation_headers: [false, false],
         expected_no_cache_headers: [true, false],
       },
       {
+        name: 'RequestCache "default" mode with an If-Modified-Since header is treated similarly to "no-store"',
+        state: "stale",
+        request_cache: ["default", "default"],
+        request_headers: [{}, {"If-Modified-Since": new Date().toGMTString()}],
+        expected_validation_headers: [false, false],
+        expected_no_cache_headers: [false, true],
+      },
+      {
+        name: 'RequestCache "default" mode with an If-Modified-Since header is treated similarly to "no-store"',
+        state: "fresh",
+        request_cache: ["default", "default"],
+        request_headers: [{}, {"If-Modified-Since": new Date().toGMTString()}],
+        expected_validation_headers: [false, false],
+        expected_no_cache_headers: [false, true],
+      },
+      {
+        name: 'RequestCache "default" mode with an If-Modified-Since header is treated similarly to "no-store"',
+        state: "stale",
+        request_cache: ["default", "default"],
+        request_headers: [{"If-Modified-Since": new Date().toGMTString()}, {}],
+        expected_validation_headers: [false, false],
+        expected_no_cache_headers: [true, false],
+      },
+      {
+        name: 'RequestCache "default" mode with an If-Modified-Since header is treated similarly to "no-store"',
+        state: "fresh",
+        request_cache: ["default", "default"],
+        request_headers: [{"If-Modified-Since": new Date().toGMTString()}, {}],
+        expected_validation_headers: [false, false],
+        expected_no_cache_headers: [true, false],
+      },
+      {
+        name: 'RequestCache "default" mode with an If-None-Match header is treated similarly to "no-store"',
+        state: "stale",
+        request_cache: ["default", "default"],
+        request_headers: [{}, {"If-None-Match": '"foo"'}],
+        expected_validation_headers: [false, false],
+        expected_no_cache_headers: [false, true],
+      },
+      {
+        name: 'RequestCache "default" mode with an If-None-Match header is treated similarly to "no-store"',
+        state: "fresh",
+        request_cache: ["default", "default"],
+        request_headers: [{}, {"If-None-Match": '"foo"'}],
+        expected_validation_headers: [false, false],
+        expected_no_cache_headers: [false, true],
+      },
+      {
+        name: 'RequestCache "default" mode with an If-None-Match header is treated similarly to "no-store"',
+        state: "stale",
+        request_cache: ["default", "default"],
+        request_headers: [{"If-None-Match": '"foo"'}, {}],
+        expected_validation_headers: [false, false],
+        expected_no_cache_headers: [true, false],
+      },
+      {
+        name: 'RequestCache "default" mode with an If-None-Match header is treated similarly to "no-store"',
+        state: "fresh",
+        request_cache: ["default", "default"],
+        request_headers: [{"If-None-Match": '"foo"'}, {}],
+        expected_validation_headers: [false, false],
+        expected_no_cache_headers: [true, false],
+      },
+      {
+        name: 'RequestCache "default" mode with an If-Unmodified-Since header is treated similarly to "no-store"',
+        state: "stale",
+        request_cache: ["default", "default"],
+        request_headers: [{}, {"If-Unmodified-Since": new Date().toGMTString()}],
+        expected_validation_headers: [false, false],
+        expected_no_cache_headers: [false, true],
+      },
+      {
+        name: 'RequestCache "default" mode with an If-Unmodified-Since header is treated similarly to "no-store"',
+        state: "fresh",
+        request_cache: ["default", "default"],
+        request_headers: [{}, {"If-Unmodified-Since": new Date().toGMTString()}],
+        expected_validation_headers: [false, false],
+        expected_no_cache_headers: [false, true],
+      },
+      {
+        name: 'RequestCache "default" mode with an If-Unmodified-Since header is treated similarly to "no-store"',
+        state: "stale",
+        request_cache: ["default", "default"],
+        request_headers: [{"If-Unmodified-Since": new Date().toGMTString()}, {}],
+        expected_validation_headers: [false, false],
+        expected_no_cache_headers: [true, false],
+      },
+      {
+        name: 'RequestCache "default" mode with an If-Unmodified-Since header is treated similarly to "no-store"',
+        state: "fresh",
+        request_cache: ["default", "default"],
+        request_headers: [{"If-Unmodified-Since": new Date().toGMTString()}, {}],
+        expected_validation_headers: [false, false],
+        expected_no_cache_headers: [true, false],
+      },
+      {
+        name: 'RequestCache "default" mode with an If-Match header is treated similarly to "no-store"',
+        state: "stale",
+        request_cache: ["default", "default"],
+        request_headers: [{}, {"If-Match": '"foo"'}],
+        expected_validation_headers: [false, false],
+        expected_no_cache_headers: [false, true],
+      },
+      {
+        name: 'RequestCache "default" mode with an If-Match header is treated similarly to "no-store"',
+        state: "fresh",
+        request_cache: ["default", "default"],
+        request_headers: [{}, {"If-Match": '"foo"'}],
+        expected_validation_headers: [false, false],
+        expected_no_cache_headers: [false, true],
+      },
+      {
+        name: 'RequestCache "default" mode with an If-Match header is treated similarly to "no-store"',
+        state: "stale",
+        request_cache: ["default", "default"],
+        request_headers: [{"If-Match": '"foo"'}, {}],
+        expected_validation_headers: [false, false],
+        expected_no_cache_headers: [true, false],
+      },
+      {
+        name: 'RequestCache "default" mode with an If-Match header is treated similarly to "no-store"',
+        state: "fresh",
+        request_cache: ["default", "default"],
+        request_headers: [{"If-Match": '"foo"'}, {}],
+        expected_validation_headers: [false, false],
+        expected_no_cache_headers: [true, false],
+      },
+      {
+        name: 'RequestCache "default" mode with an If-Range header is treated similarly to "no-store"',
+        state: "stale",
+        request_cache: ["default", "default"],
+        request_headers: [{}, {"If-Range": '"foo"'}],
+        expected_validation_headers: [false, false],
+        expected_no_cache_headers: [false, true],
+      },
+      {
+        name: 'RequestCache "default" mode with an If-Range header is treated similarly to "no-store"',
+        state: "fresh",
+        request_cache: ["default", "default"],
+        request_headers: [{}, {"If-Range": '"foo"'}],
+        expected_validation_headers: [false, false],
+        expected_no_cache_headers: [false, true],
+      },
+      {
+        name: 'RequestCache "default" mode with an If-Range header is treated similarly to "no-store"',
+        state: "stale",
+        request_cache: ["default", "default"],
+        request_headers: [{"If-Range": '"foo"'}, {}],
+        expected_validation_headers: [false, false],
+        expected_no_cache_headers: [true, false],
+      },
+      {
+        name: 'RequestCache "default" mode with an If-Range header is treated similarly to "no-store"',
+        state: "fresh",
+        request_cache: ["default", "default"],
+        request_headers: [{"If-Range": '"foo"'}, {}],
+        expected_validation_headers: [false, false],
+        expected_no_cache_headers: [true, false],
+      },
+      {
         name: 'Responses with the "Cache-Control: no-store" header are not stored in the cache',
         state: "stale",
         cache_control: "no-store",
         request_cache: ["default", "default"],
         expected_validation_headers: [false, false],
         expected_no_cache_headers: [false, false],
       },
       {
@@ -196,51 +356,65 @@
       var vary = "";
       if ("vary" in info) {
         vary = "&vary=" + info.vary;
       }
       var cache_control = "";
       if ("cache_control" in info) {
         cache_control = "&cache_control=" + info.cache_control;
       }
+      var ignore_request_headers = "";
+      if ("request_headers" in info) {
+        // Ignore the request headers that we send since they may be synthesized by the test.
+        ignore_request_headers = "&ignore";
+      }
       return "resources/cache.py?token=" + uuid +
              "&content=" + content +
              "&" + id + "=" + value +
-             "&expires=" + dates[info.state] + vary + cache_control;
+             "&expires=" + dates[info.state] +
+             vary + cache_control + ignore_request_headers;
     }
     function server_state(uuid) {
       return fetch("resources/cache.py?querystate&token=" + uuid)
         .then(function(response) {
           return response.text();
         }).then(function(text) {
           return JSON.parse(text);
         });
     }
-    function populate_cache(url, content, cache) {
-      return fetch(url, {cache: cache})
+    function populate_cache(url, content, info) {
+      var init = {cache: info.request_cache[0]};
+      if ("request_headers" in info) {
+        init.headers = info.request_headers[0];
+      }
+      return fetch(url, init)
         .then(function(response) {
           assert_equals(response.status, 200);
           assert_equals(response.statusText, "OK");
           return response.text();
         }).then(function(text) {
           assert_equals(text, content);
         });
     }
     function make_test(type, info) {
       return function(test) {
         var uuid = token();
         var identifier = (type == "tag" ? Math.random() : new Date().toGMTString());
         var content = Math.random().toString();
         var url = make_url(uuid, type, identifier, content, info);
         var fetch_functions = [function() {
-          return populate_cache(url, content, info.request_cache[0]);
+          return populate_cache(url, content, info);
         }];
         for (var i = 1; i < info.request_cache.length; ++i) {
           fetch_functions.push(function(idx) {
-            return fetch(url, {cache: info.request_cache[idx]})
+            var init = {cache: info.request_cache[idx]};
+            if ("request_headers" in info) {
+              init.headers = info.request_headers[idx];
+            }
+            return fetch(url, init)
               .then(function(response) {
                 assert_equals(response.status, 200);
                 assert_equals(response.statusText, "OK");
                 return response.text();
               }).then(function(text) {
                 assert_equals(text, content);
               });
           });
--- a/testing/web-platform/tests/fetch/api/request/resources/cache.py
+++ b/testing/web-platform/tests/fetch/api/request/resources/cache.py
@@ -9,29 +9,31 @@ def main(request, response):
     date = request.GET.first("date", None)
     expires = request.GET.first("expires", None)
     vary = request.GET.first("vary", None)
     cc = request.GET.first("cache_control", None)
     inm = request.headers.get("If-None-Match", None)
     ims = request.headers.get("If-Modified-Since", None)
     pragma = request.headers.get("Pragma", None)
     cache_control = request.headers.get("Cache-Control", None)
+    ignore = "ignore" in request.GET
 
     server_state = request.server.stash.take(token)
     if not server_state:
         server_state = []
     state = dict()
-    if inm:
-        state["If-None-Match"] = inm
-    if ims:
-        state["If-Modified-Since"] = ims
-    if pragma:
-        state["Pragma"] = pragma
-    if cache_control:
-        state["Cache-Control"] = cache_control
+    if not ignore:
+        if inm:
+            state["If-None-Match"] = inm
+        if ims:
+            state["If-Modified-Since"] = ims
+        if pragma:
+            state["Pragma"] = pragma
+        if cache_control:
+            state["Cache-Control"] = cache_control
     server_state.append(state)
     request.server.stash.put(token, server_state)
 
     if tag:
         response.headers.set("ETag", '"%s"' % tag)
     elif date:
         response.headers.set("Last-Modified", date)
     if expires: