Bug 1120715 - Part 5: Treat a default cache mode Request with a revalidation header as no-store; r=bkelly
☠☠ backed out by 60633fe1415f ☠ ☠
authorEhsan Akhgari <ehsan@mozilla.com>
Wed, 02 Mar 2016 18:09:30 -0500
changeset 288105 79158f028ad3
parent 288104 c74866f52320
child 288106 c2871dbeb7cc
push id73306
push usereakhgari@mozilla.com
push dateThu, 10 Mar 2016 22:29:29 +0000
treeherdermozilla-inbound@043770204431 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbkelly
bugs1120715
milestone48.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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: