bug 717350 doom cache entry on last-modified mismatch r=honzab

--HG--
extra : rebase_source : 5c8692c94bf7e5dcb2d381a85e06bddbabffe22c

netwerk/protocol/http/nsHttpChannel.cpp

@@ -1973,6 +1973,33 @@ nsHttpChannel::ProcessNotModified()
     NS_ENSURE_TRUE(mCachedResponseHead, NS_ERROR_NOT_INITIALIZED);
     NS_ENSURE_TRUE(mCacheEntry, NS_ERROR_NOT_INITIALIZED);
 
+    // If the 304 response contains a Last-Modified different than the
+    // one in our cache that is pretty suspicious and is, in at least the
+    // case of bug 716840, a sign of the server having previously corrupted
+    // our cache with a bad response. Take the minor step here of just dooming
+    // that cache entry so there is a fighting chance of getting things on the
+    // right track as well as disabling pipelining for that host.
+
+    nsCAutoString lastModified;
+    nsCAutoString lastModified304;
+
+    rv = mCachedResponseHead->GetHeader(nsHttp::Last_Modified,
+                                        lastModified);
+    if (NS_SUCCEEDED(rv))
+        rv = mResponseHead->GetHeader(nsHttp::Last_Modified, 
+                                      lastModified304);
+    if (NS_SUCCEEDED(rv) && !lastModified304.Equals(lastModified)) {
+        LOG(("Cache Entry and 304 Last-Modified Headers Do Not Match "
+             "%s and %s\n", lastModified.get(), lastModified304.get()));
+
+        mCacheEntry->Doom();
+        if (mConnectionInfo)
+            gHttpHandler->ConnMgr()->
+                PipelineFeedbackInfo(mConnectionInfo,
+                                     nsHttpConnectionMgr::RedCorruptedContent,
+                                     nsnull, 0);
+    }
+
     // merge any new headers with the cached response headers
     rv = mCachedResponseHead->UpdateHeaders(mResponseHead->Headers());
     if (NS_FAILED(rv)) return rv;

netwerk/protocol/http/nsHttpConnectionMgr.h

@@ -160,8 +160,9 @@ public:
         // pipelining list is received
         RedBannedServer = kPipelineInfoTypeRed | kPipelineInfoTypeBad | 0x0002,
     
-        // Used when a response is terminated early, or when it fails an
+        // Used when a response is terminated early, when it fails an
-        // integrity check such as assoc-req
+        // integrity check such as assoc-req or when a 304 contained a Last-Modified
+        // differnet than the entry being validated.
         RedCorruptedContent = kPipelineInfoTypeRed | kPipelineInfoTypeBad | 0x0004,
 
         // Used when a pipeline is only partly satisfied - for instance if the

netwerk/test/unit/test_mismatch_lm.js

@@ -0,0 +1,126 @@
+do_load_httpd_js();
+var httpserver = new nsHttpServer();
+var cacheService;
+var ios;
+
+// Test the handling of a cache revalidation with mismatching last-modified
+// headers. If we get such a revalidation the cache entry should be purged.
+// see bug 717350
+
+// In this test the wrong data is from 11-16-1994 with a value of 'A',
+// and the right data is from 11-15-1994 with a value of 'B'.
+
+// the same URL is requested 3 times. the first time the wrong data comes
+// back, the second time that wrong data is revalidated with a 304 but
+// a L-M header of the right data (this triggers a cache purge), and
+// the third time the right data is returned.
+
+var listener_3 = {
+    // this listener is used to process the the request made after
+    // the cache invalidation. it expects to see the 'right data'
+
+    onStartRequest: function test_onStartR(request, ctx) {},
+    
+    onDataAvailable: function test_ODA(request, cx, inputStream,
+                                       offset, count) {
+	var data = new BinaryInputStream(inputStream).readByteArray(count);
+      
+	// This is 'B'
+	do_check_eq(data, 66);
+    },
+
+    onStopRequest: function test_onStopR(request, ctx, status) {
+	httpserver.stop(do_test_finished);
+    }
+};
+
+var listener_2 = {
+    // this listener is used to process the revalidation of the
+    // corrupted cache entry. its revalidation prompts it to be cleaned
+
+    onStartRequest: function test_onStartR(request, ctx) {},
+    
+    onDataAvailable: function test_ODA(request, cx, inputStream,
+                                       offset, count) {
+	var data = new BinaryInputStream(inputStream).readByteArray(count);
+      
+	// This is 'A' from a cache revalidation, but that reval will clean the cache
+	// because of mismatched last-modified response headers
+	
+	do_check_eq(data, 65);
+    },
+
+    onStopRequest: function test_onStopR(request, ctx, status) {
+	var channel = request.QueryInterface(Ci.nsIHttpChannel);
+
+	var chan = ios.newChannel("http://localhost:4444/test1", "", null);
+	var httpChan = chan.QueryInterface(Ci.nsIHttpChannel);
+	httpChan.requestMethod = "GET";
+	httpChan.asyncOpen(listener_3, null);
+    }
+};
+
+var listener_1 = {
+    // this listener processes the initial request from a empty cache.
+    // the server responds with the wrong data ('A')
+
+    onStartRequest: function test_onStartR(request, ctx) {},
+    
+    onDataAvailable: function test_ODA(request, cx, inputStream,
+                                       offset, count) {
+	var data = new BinaryInputStream(inputStream).readByteArray(count);
+	do_check_eq(data, 65);
+    },
+
+    onStopRequest: function test_onStopR(request, ctx, status) {
+	var channel = request.QueryInterface(Ci.nsIHttpChannel);
+
+	var chan = ios.newChannel("http://localhost:4444/test1", "", null);
+	var httpChan = chan.QueryInterface(Ci.nsIHttpChannel);
+	httpChan.requestMethod = "GET";
+	httpChan.asyncOpen(listener_2, null);
+    }
+};
+
+function run_test() {
+    do_get_profile();
+    cacheService = Cc["@mozilla.org/network/cache-service;1"].
+        getService(Ci.nsICacheService);
+    ios = Cc["@mozilla.org/network/io-service;1"]
+            .getService(Ci.nsIIOService);
+
+    cacheService.evictEntries(Ci.nsICache.STORE_ANYWHERE);
+
+    httpserver.registerPathHandler("/test1", handler);
+    httpserver.start(4444);
+
+    var chan = ios.newChannel("http://localhost:4444/test1", "", null);
+    var httpChan = chan.QueryInterface(Ci.nsIHttpChannel);
+    httpChan.requestMethod = "GET";
+    httpChan.asyncOpen(listener_1, null);
+
+    do_test_pending();
+}
+
+var iter=0;
+function handler(metadata, response) {
+    iter++;
+    if (metadata.hasHeader("If-Modified-Since")) {
+	response.setStatusLine(metadata.httpVersion, 304, "Not Modified");
+	response.setHeader("Last-Modified", "Tue, 15 Nov 1994 12:45:26 GMT", false);
+    }    
+    else {
+	response.setStatusLine(metadata.httpVersion, 200, "OK");
+	response.setHeader("Cache-Control", "max-age=0", false)
+	if (iter == 1) {
+	    // simulated wrong response
+	    response.setHeader("Last-Modified", "Wed, 16 Nov 1994 00:00:00 GMT", false);
+	    response.bodyOutputStream.write("A", 1);
+	}
+	if (iter == 3) {
+	    // 'correct' response
+	    response.setHeader("Last-Modified", "Tue, 15 Nov 1994 12:45:26 GMT", false);
+	    response.bodyOutputStream.write("B", 1);
+	}
+    }
+}

netwerk/test/unit/xpcshell.ini

@@ -132,6 +132,7 @@ skip-if = os == "android"
 [test_httpsuspend.js]
 [test_idnservice.js]
 [test_localstreams.js]
+[test_mismatch_lm.js]
 [test_MIME_params.js]
 [test_multipart_streamconv.js]
 [test_multipart_streamconv_missing_lead_boundary.js]