Bug 1121447 - trust cache less for error codes r=mayhemer

netwerk/base/nsIRequest.idl

@@ -143,8 +143,7 @@ interface nsIRequest : nsISupports
 
     /**
      * This flag prevents caching on disk (or other persistent media), which
-     * may be needed to preserve privacy.  For HTTPS, this flag is set auto-
-     * matically.
+     * may be needed to preserve privacy.
      */
     const unsigned long INHIBIT_PERSISTENT_CACHING = 1 << 8;
 

netwerk/protocol/http/nsHttpChannel.cpp

@@ -3330,12 +3330,14 @@ nsHttpChannel::OnCacheEntryCheck(nsICacheEntry* entry, nsIApplicationCache* appC
         // expect it to be cached.  (we only keep it in our cache for the
         // purposes of back/forward, etc.)
         //
-        // the request method MUST be either GET or HEAD (see bug 175641).
+        // the request method MUST be either GET or HEAD (see bug 175641) and
+        // the cached response code must be < 400
         //
         // do not override conditional headers when consumer has defined its own
         if (!mCachedResponseHead->NoStore() &&
             (mRequestHead.IsGet() || mRequestHead.IsHead()) &&
-             !mCustomConditionalRequest) {
+            !mCustomConditionalRequest &&
+            (mCachedResponseHead->Status() < 400)) {
 
             if (mConcurentCacheAccess) {
                 // In case of concurrent read and also validation request we

netwerk/protocol/http/nsHttpResponseHead.cpp

@@ -450,6 +450,12 @@ nsHttpResponseHead::ComputeFreshnessLifetime(uint32_t *result) const
         return NS_OK;
     }
 
+    if (mStatus >= 400) {
+        LOG(("nsHttpResponseHead::ComputeFreshnessLifetime [this = %p] "
+             "Do not calculate heuristic max-age for most responses >= 400\n", this));
+        return NS_OK;
+    }
+
     // Fallback on heuristic using last modified header...
     if (NS_SUCCEEDED(GetLastModifiedValue(&date2))) {
         LOG(("using last-modified to determine freshness-lifetime\n"));

netwerk/test/unit/test_cacheflags.js

@@ -14,6 +14,7 @@ var longexp2Path = "/longexp/2/" + suffix;
 var nocachePath = "/nocache" + suffix;
 var nostorePath = "/nostore" + suffix;
 var test410Path = "/test410" + suffix;
+var test404Path = "/test404" + suffix;
 
 // We attach this to channel when we want to test Private Browsing mode
 function LoadContext(usePrivateBrowsing) {
@@ -267,7 +268,16 @@ var gTests = [
   new Test(httpBase + test410Path, 0,
            true,   // expect success
            true,   // read from cache
-           false)  // hit server
+           false), // hit server
+
+  new Test(httpBase + test404Path, 0,
+           true,   // expect success
+           false,  // read from cache
+           true),  // hit server
+  new Test(httpBase + test404Path, 0,
+           true,   // expect success
+           false,  // read from cache
+           true)   // hit server
 ];
 
 function run_next_test()
@@ -314,6 +324,10 @@ function test410_handler(metadata, response) {
   handler(410, metadata, response);
 }
 
+function test404_handler(metadata, response) {
+  handler(404, metadata, response);
+}
+
 function shortexp_handler(metadata, response) {
   response.setHeader("Cache-Control", "max-age=0", false);
   handler(200, metadata, response);
@@ -337,6 +351,7 @@ function run_test() {
   httpserver.registerPathHandler(nocachePath, nocache_handler);
   httpserver.registerPathHandler(nostorePath, nostore_handler);
   httpserver.registerPathHandler(test410Path, test410_handler);
+  httpserver.registerPathHandler(test404Path, test404_handler);
 
   run_next_test();
   do_test_pending();