From 39045fbf7d159a184e9b1394758a0d1ed7b0d947 Mon Sep 17 00:00:00 2001 From: "bjarne@runitsoft.com" Date: Mon, 23 May 2011 16:47:02 -0400 Subject: [PATCH] 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 | 16 +- netwerk/test/unit/test_bug633743.js | 193 ++++++++++++++++++++++++ netwerk/test/unit/xpcshell.ini | 1 + 3 files changed, 202 insertions(+), 8 deletions(-) create mode 100644 netwerk/test/unit/test_bug633743.js diff --git a/netwerk/protocol/http/nsHttpChannel.cpp b/netwerk/protocol/http/nsHttpChannel.cpp index bdd7dc568d70..378b7b682c1c 100644 --- a/netwerk/protocol/http/nsHttpChannel.cpp +++ b/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")); diff --git a/netwerk/test/unit/test_bug633743.js b/netwerk/test/unit/test_bug633743.js new file mode 100644 index 000000000000..c98e4bf28bea --- /dev/null +++ b/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(); +} diff --git a/netwerk/test/unit/xpcshell.ini b/netwerk/test/unit/xpcshell.ini index 8d79dde67191..dbd6835a7c22 100644 --- a/netwerk/test/unit/xpcshell.ini +++ b/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]