Bug 770243: Close cache input stream only when we're sure we don't need it in nsHttpChannel, r=bsmith for Honza's code, r=honzab for bsmith's changes

--HG--
extra : rebase_source : 6ea99a3a3cd3703cdd0b9a2e0c17dd71ffc3b3ac

netwerk/protocol/http/nsHttpChannel.cpp

@@ -1187,9 +1187,6 @@ nsHttpChannel::ProcessResponse()
     }
 
     MOZ_ASSERT(!mCachedContentIsValid);
-    if (httpStatus != 304 && httpStatus != 206) {
-        mCacheInputStream.CloseAndRelease();
-    }
 
     // notify "http-on-examine-response" observers
     gHttpHandler->OnExamineResponse(this);
@@ -3859,7 +3856,14 @@ nsHttpChannel::InstallCacheListener(uint32_t offset)
             LOG(("unable to mark cache entry for compression"));
         }
     } 
-      
+
+    LOG(("Trading cache input stream for output stream [channel=%p]", this));
+
+    // We must close the input stream first because cache entries do not
+    // correctly handle having an output stream and input streams open at
+    // the same time.
+    mCacheInputStream.CloseAndRelease();
+
     nsCOMPtr<nsIOutputStream> out;
     rv = mCacheEntry->OpenOutputStream(offset, getter_AddRefs(out));
     if (NS_FAILED(rv)) return rv;

netwerk/test/unit/head_channels.js

@@ -27,6 +27,8 @@ const CL_EXPECT_3S_DELAY = 0x4;
 const CL_SUSPEND = 0x8;
 const CL_ALLOW_UNKNOWN_CL = 0x10;
 const CL_EXPECT_LATE_FAILURE = 0x20;
+const CL_FROM_CACHE = 0x40; // Response must be from the cache
+const CL_NOT_FROM_CACHE = 0x80; // Response must NOT be from the cache
 
 const SUSPEND_DELAY = 3000;
 
@@ -85,6 +87,19 @@ ChannelListener.prototype = {
       if (this._contentLen == -1 && !(this._flags & (CL_EXPECT_FAILURE | CL_ALLOW_UNKNOWN_CL)))
         do_throw("Content length is unknown in onStartRequest!");
 
+      if ((this._flags & CL_FROM_CACHE)) {
+        request.QueryInterface(Ci.nsICachingChannel);
+        if (!request.isFromCache()) {
+          do_throw("Response is not from the cache (CL_FROM_CACHE)");
+        }
+      }
+      if ((this._flags & CL_NOT_FROM_CACHE)) {
+        request.QueryInterface(Ci.nsICachingChannel);
+        if (request.isFromCache()) {
+          do_throw("Response is from the cache (CL_NOT_FROM_CACHE)");
+        }
+      }
+
       if (this._flags & CL_SUSPEND) {
         request.suspend();
         do_timeout(SUSPEND_DELAY, function() { request.resume(); });

netwerk/test/unit/test_bug770243.js

@@ -0,0 +1,211 @@
+/* this test does the following:
+ Always requests the same resource, while for each request getting:
+ 1. 200 + ETag: "one"
+ 2. 401 followed by 200 + ETag: "two"
+ 3. 401 followed by 304
+ 4. 407 followed by 200 + ETag: "three"
+ 5. 407 followed by 304
+*/
+
+const Cc = Components.classes;
+const Ci = Components.interfaces;
+const Cu = Components.utils;
+const Cr = Components.results;
+
+Cu.import("resource://testing-common/httpd.js");
+
+var httpserv;
+ 
+function addCreds(scheme, host)
+{
+  var authMgr = Components.classes['@mozilla.org/network/http-auth-manager;1']
+                          .getService(Ci.nsIHttpAuthManager);
+  authMgr.setAuthIdentity(scheme, host, 4444, "basic", "secret", "/", "", "user", "pass");
+}
+
+function clearCreds()
+{
+  var authMgr = Components.classes['@mozilla.org/network/http-auth-manager;1']
+                          .getService(Ci.nsIHttpAuthManager);
+  authMgr.clearAll();
+}
+
+function makeChan() {
+  var ios = Cc["@mozilla.org/network/io-service;1"]
+                      .getService(Ci.nsIIOService);
+  var chan = ios.newChannel("http://localhost:4444/", null, null)
+                .QueryInterface(Ci.nsIHttpChannel);
+  return chan;
+}
+
+// Array of handlers that are called one by one in response to expected requests
+
+var handlers = [
+  // Test 1
+  function(metadata, response) {
+    do_check_eq(metadata.hasHeader("Authorization"), false);
+    response.setStatusLine(metadata.httpVersion, 200, "OK");
+    response.setHeader("ETag", '"one"', false);
+    response.setHeader("Cache-control", 'no-cache', false);
+    response.setHeader("Content-type", 'text/plain', false);
+    var body = "Response body 1";
+    response.bodyOutputStream.write(body, body.length);
+  },
+
+  // Test 2
+  function(metadata, response) {
+    do_check_eq(metadata.hasHeader("Authorization"), false);
+    do_check_eq(metadata.getHeader("If-None-Match"), '"one"');
+    response.setStatusLine(metadata.httpVersion, 401, "Authenticate");
+    response.setHeader("WWW-Authenticate", 'Basic realm="secret"', false);
+    addCreds("http", "localhost");
+  },
+  function(metadata, response) {
+    do_check_eq(metadata.hasHeader("Authorization"), true);
+    response.setStatusLine(metadata.httpVersion, 200, "OK");
+    response.setHeader("ETag", '"two"', false);
+    response.setHeader("Cache-control", 'no-cache', false);
+    response.setHeader("Content-type", 'text/plain', false);
+    var body = "Response body 2";
+    response.bodyOutputStream.write(body, body.length);
+    clearCreds();
+  },
+  
+  // Test 3
+  function(metadata, response) {
+    do_check_eq(metadata.hasHeader("Authorization"), false);
+    do_check_eq(metadata.getHeader("If-None-Match"), '"two"');
+    response.setStatusLine(metadata.httpVersion, 401, "Authenticate");
+    response.setHeader("WWW-Authenticate", 'Basic realm="secret"', false);
+    addCreds("http", "localhost");
+  },
+  function(metadata, response) {
+    do_check_eq(metadata.hasHeader("Authorization"), true);
+    do_check_eq(metadata.getHeader("If-None-Match"), '"two"');
+    response.setStatusLine(metadata.httpVersion, 304, "OK");
+    response.setHeader("ETag", '"two"', false);
+    clearCreds();
+  },
+  
+  // Test 4
+  function(metadata, response) {
+    do_check_eq(metadata.hasHeader("Authorization"), false);
+    do_check_eq(metadata.getHeader("If-None-Match"), '"two"');
+    response.setStatusLine(metadata.httpVersion, 407, "Proxy Authenticate");
+    response.setHeader("Proxy-Authenticate", 'Basic realm="secret"', false);
+    addCreds("http", "localhost");
+  },
+  function(metadata, response) {
+    do_check_eq(metadata.hasHeader("Proxy-Authorization"), true);
+    do_check_eq(metadata.getHeader("If-None-Match"), '"two"');
+    response.setStatusLine(metadata.httpVersion, 200, "OK");
+    response.setHeader("ETag", '"three"', false);
+    response.setHeader("Cache-control", 'no-cache', false);
+    response.setHeader("Content-type", 'text/plain', false);
+    var body = "Response body 3";
+    response.bodyOutputStream.write(body, body.length);
+    clearCreds();
+  }, 
+  
+  // Test 5
+  function(metadata, response) {
+    do_check_eq(metadata.hasHeader("Proxy-Authorization"), false);
+    do_check_eq(metadata.getHeader("If-None-Match"), '"three"');
+    response.setStatusLine(metadata.httpVersion, 407, "Proxy Authenticate");
+    response.setHeader("Proxy-Authenticate", 'Basic realm="secret"', false);
+    addCreds("http", "localhost");
+  },
+  function(metadata, response) {
+    do_check_eq(metadata.hasHeader("Proxy-Authorization"), true);
+    do_check_eq(metadata.getHeader("If-None-Match"), '"three"');
+    response.setStatusLine(metadata.httpVersion, 304, "OK");
+    response.setHeader("ETag", '"three"', false);
+    response.setHeader("Cache-control", 'no-cache', false);
+    clearCreds();
+  }
+];
+
+function handler(metadata, response) 
+{
+  handlers.shift()(metadata, response); 
+}
+
+// Array of tests to run, self-driven
+
+function sync_and_run_next_test()
+{
+  syncWithCacheIOThread(function() {
+    tests.shift()();
+  });
+}
+
+var tests = [
+  // Test 1: 200 (cacheable)
+  function() {
+    var ch = makeChan();
+    ch.asyncOpen(new ChannelListener(function(req, body) {
+      do_check_eq(body, "Response body 1");
+      sync_and_run_next_test();
+    }, null, CL_NOT_FROM_CACHE), null);
+  },
+
+  // Test 2: 401 and 200 + new content
+  function() {
+    var ch = makeChan();
+    ch.asyncOpen(new ChannelListener(function(req, body) {
+      do_check_eq(body, "Response body 2");
+      sync_and_run_next_test();
+    }, null, CL_NOT_FROM_CACHE), null);
+  },
+  
+  // Test 3: 401 and 304
+  function() {
+    var ch = makeChan();
+    ch.asyncOpen(new ChannelListener(function(req, body) {
+      do_check_eq(body, "Response body 2");
+      sync_and_run_next_test();
+    }, null, CL_FROM_CACHE), null);
+  },
+  
+  // Test 4: 407 and 200 + new content
+  function() {
+    var ch = makeChan();
+    ch.asyncOpen(new ChannelListener(function(req, body) {
+      do_check_eq(body, "Response body 3");
+      sync_and_run_next_test();
+    }, null, CL_NOT_FROM_CACHE), null);
+  },
+  
+  // Test 5: 407 and 304
+  function() {
+    var ch = makeChan();
+    ch.asyncOpen(new ChannelListener(function(req, body) {
+      do_check_eq(body, "Response body 3");
+      sync_and_run_next_test();
+    }, null, CL_FROM_CACHE), null);
+  },
+
+  // End of test run
+  function() {
+    httpserv.stop(do_test_finished);
+  }
+];
+
+function run_test()
+{
+  do_get_profile();
+  
+  httpserv = new HttpServer();
+  httpserv.registerPathHandler("/", handler);
+  httpserv.start(4444);
+
+  const prefs = Cc["@mozilla.org/preferences-service;1"]
+                         .getService(Ci.nsIPrefBranch);
+  prefs.setCharPref("network.proxy.http", "localhost");
+  prefs.setIntPref("network.proxy.http_port", 4444);
+  prefs.setCharPref("network.proxy.no_proxies_on", "");
+  prefs.setIntPref("network.proxy.type", 1);
+
+  tests.shift()();
+  do_test_pending();
+}

netwerk/test/unit/xpcshell.ini

@@ -87,6 +87,7 @@ fail-if = os == "android"
 [test_bug667818.js]
 [test_bug669001.js]
 [test_bug712914_secinfo_validation.js]
+[test_bug770243.js]
 [test_doomentry.js]
 [test_cacheflags.js]
 [test_channel_close.js]