Bug 633743 - reverting closed tab with pushState changes sends request with HTTP_X_REQUESTED_WITH:XMLHttpRequest r=bzbarsky

With this change, we check for ResponseWouldVary() to make sure that we're talking about the same entity as the caller before we worry about whether the caller passed LOAD_FROM_CACHE and such.  If what we have cached is just fundamentally different from what the caller wants, we don't want to return it.

netwerk/protocol/http/nsHttpChannel.cpp

@@ -2557,8 +2557,14 @@ nsHttpChannel::CheckCache()
     PRBool doValidation = PR_FALSE;
     PRBool canAddImsHeader = PR_TRUE;
 
-    // If the LOAD_FROM_CACHE flag is set, any cached data can simply be used.
-    if (mLoadFlags & LOAD_FROM_CACHE) {
+    // Cached entry is not the entity we request (see bug #633743)
+    if (ResponseWouldVary()) {
+        LOG(("Validating based on Vary headers returning TRUE\n"));
+        canAddImsHeader = PR_FALSE;
+        doValidation = PR_TRUE;
+    }
+    // If the LOAD_FROM_CACHE flag is set, any cached data can simply be used
+    else if (mLoadFlags & LOAD_FROM_CACHE) {
         LOG(("NOT validating based on LOAD_FROM_CACHE load flag\n"));
         doValidation = PR_FALSE;
     }
@@ -2590,12 +2596,6 @@ nsHttpChannel::CheckCache()
         doValidation = PR_TRUE;
     }
 
-    else if (ResponseWouldVary()) {
-        LOG(("Validating based on Vary headers returning TRUE\n"));
-        canAddImsHeader = PR_FALSE;
-        doValidation = PR_TRUE;
-    }
-    
     else if (MustValidateBasedOnQueryUrl()) {
         LOG(("Validating based on RFC 2616 section 13.9 "
              "(query-url w/o explicit expiration-time)\n"));

netwerk/test/unit/test_bug633743.js

@@ -0,0 +1,193 @@
+do_load_httpd_js();
+
+const VALUE_HDR_NAME = "X-HTTP-VALUE-HEADER";
+const VARY_HDR_NAME = "X-HTTP-VARY-HEADER";
+const CACHECTRL_HDR_NAME = "X-CACHE-CONTROL-HEADER";
+
+var httpserver = null;
+
+var _CSvc;
+function get_cache_service() {
+  if (_CSvc)
+    return _CSvc;
+
+  return _CSvc = Cc["@mozilla.org/network/cache-service;1"].
+    getService(Ci.nsICacheService);
+}
+
+function make_channel(flags, vary, value) {
+  var ios = Cc["@mozilla.org/network/io-service;1"].
+    getService(Ci.nsIIOService);
+  var chan = ios.newChannel("http://localhost:4444/bug633743", null, null);
+  return chan.QueryInterface(Ci.nsIHttpChannel);
+}
+
+function Test(flags, varyHdr, sendValue, expectValue, cacheHdr) {
+    this._flags = flags;
+    this._varyHdr = varyHdr;
+    this._sendVal = sendValue;
+    this._expectVal = expectValue;
+    this._cacheHdr = cacheHdr;
+}
+
+Test.prototype = {
+  _buffer: "",
+  _flags: null,
+  _varyHdr: null,
+  _sendVal: null,
+  _expectVal: null,
+  _cacheHdr: null,
+
+  QueryInterface: function(iid) {
+    if (iid.equals(Ci.nsIStreamListener) ||
+        iid.equals(Ci.nsIRequestObserver) ||
+        iid.equals(Ci.nsISupports))
+      return this;
+    throw Cr.NS_ERROR_NO_INTERFACE;
+  },
+
+  onStartRequest: function(request, context) { },
+
+  onDataAvailable: function(request, context, stream, offset, count) {
+    this._buffer = this._buffer.concat(read_stream(stream, count));
+  },
+
+  onStopRequest: function(request, context, status) {
+    do_check_eq(this._buffer, this._expectVal);
+    do_timeout(0, run_next_test);
+  },
+
+  run: function() {
+    var channel = make_channel();
+    channel.loadFlags = this._flags;
+    channel.setRequestHeader(VALUE_HDR_NAME, this._sendVal, false);
+    channel.setRequestHeader(VARY_HDR_NAME, this._varyHdr, false);
+    if (this._cacheHdr)
+        channel.setRequestHeader(CACHECTRL_HDR_NAME, this._cacheHdr, false);
+
+    channel.asyncOpen(this, null);
+  }
+};
+
+var gTests = [
+// Test LOAD_FROM_CACHE: Load cache-entry
+  new Test(Ci.nsIRequest.LOAD_NORMAL,
+          "entity-initial", // hdr-value used to vary
+          "request1", // echoed by handler
+          "request1"  // value expected to receive in channel
+          ),
+// Verify that it was cached
+  new Test(Ci.nsIRequest.LOAD_NORMAL,
+          "entity-initial", // hdr-value used to vary
+          "fresh value with LOAD_NORMAL",  // echoed by handler
+          "request1"  // value expected to receive in channel
+          ),
+// Load same entity with LOAD_FROM_CACHE-flag
+  new Test(Ci.nsIRequest.LOAD_FROM_CACHE,
+          "entity-initial", // hdr-value used to vary
+          "fresh value with LOAD_FROM_CACHE",  // echoed by handler
+          "request1"  // value expected to receive in channel
+          ),
+// Load different entity with LOAD_FROM_CACHE-flag
+  new Test(Ci.nsIRequest.LOAD_FROM_CACHE,
+          "entity-l-f-c", // hdr-value used to vary
+          "request2", // echoed by handler
+          "request2"  // value expected to receive in channel
+          ),
+// Verify that new value was cached
+  new Test(Ci.nsIRequest.LOAD_NORMAL,
+          "entity-l-f-c", // hdr-value used to vary
+          "fresh value with LOAD_NORMAL",  // echoed by handler
+          "request2"  // value expected to receive in channel
+          ),
+
+// Test VALIDATE_NEVER: Note previous cache-entry
+  new Test(Ci.nsIRequest.VALIDATE_NEVER,
+          "entity-v-n", // hdr-value used to vary
+          "request3",  // echoed by handler
+          "request3"  // value expected to receive in channel
+          ),
+// Verify that cache-entry was replaced
+  new Test(Ci.nsIRequest.LOAD_NORMAL,
+          "entity-v-n", // hdr-value used to vary
+          "fresh value with LOAD_NORMAL",  // echoed by handler
+          "request3"  // value expected to receive in channel
+          ),
+
+// Test combination VALIDATE_NEVER && no-store: Load new cache-entry
+  new Test(Ci.nsIRequest.LOAD_NORMAL,
+          "entity-2",// hdr-value used to vary
+          "request4",  // echoed by handler
+          "request4",  // value expected to receive in channel
+          "no-store"   // set no-store on response
+          ),
+// Ensure we validate without IMS header in this case (verified in handler)
+  new Test(Ci.nsIRequest.VALIDATE_NEVER,
+          "entity-2-v-n",// hdr-value used to vary
+          "request5",  // echoed by handler
+          "request5"  // value expected to receive in channel
+          ),
+
+// Test VALIDATE-ALWAYS: Load new entity
+  new Test(Ci.nsIRequest.LOAD_NORMAL,
+          "entity-3",// hdr-value used to vary
+          "request6",  // echoed by handler
+          "request6",  // value expected to receive in channel
+          "no-cache"   // set no-cache on response
+          ),
+// Ensure we don't send IMS header also in this case (verified in handler)
+  new Test(Ci.nsIRequest.VALIDATE_ALWAYS,
+          "entity-3-v-a",// hdr-value used to vary
+          "request7",  // echoed by handler
+          "request7"  // value expected to receive in channel
+          ),
+  ];
+
+function run_next_test()
+{
+  if (gTests.length == 0) {
+    httpserver.stop(do_test_finished);
+    return;
+  }
+
+  var test = gTests.shift();
+  test.run();
+}
+
+function handler(metadata, response) {
+
+  // None of the tests above should send an IMS
+  do_check_false(metadata.hasHeader("If-Modified-Since"));
+
+  // Pick up requested value to echo
+  var hdr = "default value";
+  try {
+    hdr = metadata.getHeader(VALUE_HDR_NAME);
+  } catch(ex) { }
+
+  // Pick up requested cache-control header-value
+  var cctrlVal = "max-age=10000";
+  try {
+      cctrlVal = metadata.getHeader(CACHECTRL_HDR_NAME);
+  } catch(ex) { }
+
+  response.setStatusLine(metadata.httpVersion, 200, "OK");
+  response.setHeader("Content-Type", "text/plain", false);
+  response.setHeader("Cache-Control", cctrlVal, false);
+  response.setHeader("Vary", VARY_HDR_NAME, false);
+  response.setHeader("Last-Modified", "Tue, 15 Nov 1994 12:45:26 GMT", false);
+  response.bodyOutputStream.write(hdr, hdr.length);
+}
+
+function run_test() {
+
+  // clear the cache
+  get_cache_service().evictEntries(Ci.nsICache.STORE_ANYWHERE);
+
+  httpserver = new nsHttpServer();
+  httpserver.registerPathHandler("/bug633743", handler);
+  httpserver.start(4444);
+
+  run_next_test();
+  do_test_pending();
+}

netwerk/test/unit/xpcshell.ini

@@ -56,6 +56,7 @@ tail =
 [test_bug586908.js]
 [test_bug588389.js]
 [test_bug596443.js]
+[test_bug633743.js]
 [test_bug652761.js]
 [test_cacheflags.js]
 [test_channel_close.js]