Bug 738484: part 2: fix missing onStartRequest calls for some failed redirects. r=mcmanus

netwerk/protocol/http/nsHttpChannel.cpp

@@ -419,6 +419,8 @@ nsHttpChannel::HandleAsyncRedirect()
         rv = AsyncProcessRedirection(mResponseHead->Status());
         if (NS_FAILED(rv)) {
             PopRedirectAsyncFunc(&nsHttpChannel::ContinueHandleAsyncRedirect);
+            // TODO: if !DoNotRender3xxBody(), render redirect body instead.
+            // But first we need to cache 3xx bodies (bug 748510)
             ContinueHandleAsyncRedirect(rv);
         }
     }
@@ -1099,7 +1101,15 @@ nsHttpChannel::ProcessResponse()
         if (NS_FAILED(rv)) {
             PopRedirectAsyncFunc(&nsHttpChannel::ContinueProcessResponse);
             LOG(("AsyncProcessRedirection failed [rv=%x]\n", rv));
-            rv = ContinueProcessResponse(rv);
+            // don't cache failed redirect responses.
+            if (mCacheEntry)
+                mCacheEntry->Doom();
+            if (DoNotRender3xxBody(rv)) {
+                mStatus = rv;
+                DoNotifyListener();
+            } else {
+                rv = ContinueProcessResponse(rv);
+            }
         }
         break;
     case 304:
@@ -3553,8 +3563,6 @@ nsHttpChannel::AsyncProcessRedirection(PRUint32 redirectType)
 
     if (mRedirectionLimit == 0) {
         LOG(("redirection limit reached!\n"));
-        // this error code is fatal, and should be conveyed to our listener.
-        Cancel(NS_ERROR_REDIRECT_LOOP);
         return NS_ERROR_REDIRECT_LOOP;
     }
 
@@ -3567,7 +3575,6 @@ nsHttpChannel::AsyncProcessRedirection(PRUint32 redirectType)
 
     if (NS_FAILED(rv)) {
         LOG(("Invalid URI for redirect: Location: %s\n", location));
-        Cancel(NS_ERROR_CORRUPTED_CONTENT);
         return NS_ERROR_CORRUPTED_CONTENT;
     }
 

netwerk/protocol/http/nsHttpChannel.h

@@ -288,6 +288,10 @@ private:
                 (tmpHost1 == tmpHost2));
     }
 
+    inline static bool DoNotRender3xxBody(nsresult rv) {
+        return rv == NS_ERROR_REDIRECT_LOOP || rv == NS_ERROR_CORRUPTED_CONTENT;
+    }
+
 private:
     nsCOMPtr<nsISupports>             mSecurityInfo;
     nsCOMPtr<nsICancelable>           mProxyRequest;

netwerk/test/unit/test_redirect_baduri.js

@@ -0,0 +1,42 @@
+do_load_httpd_js();
+
+/*
+ * Test whether we fail bad URIs in HTTP redirect as CORRUPTED_CONTENT.
+ */
+
+var httpServer = null;
+
+var BadRedirectPath = "/BadRedirect"; 
+var BadRedirectURI = "http://localhost:4444" + BadRedirectPath;
+
+function make_channel(url, callback, ctx) {
+  var ios = Cc["@mozilla.org/network/io-service;1"].
+            getService(Ci.nsIIOService);
+  return ios.newChannel(url, "", null);
+}
+
+function BadRedirectHandler(metadata, response)
+{
+  response.setStatusLine(metadata.httpVersion, 301, "Moved");
+  // '>' in URI will fail to parse: we should not render response
+  response.setHeader("Location", 'http://localhost:4444>BadRedirect', false);
+}
+
+function checkFailed(request, buffer)
+{
+  do_check_eq(request.status, Components.results.NS_ERROR_CORRUPTED_CONTENT);
+
+  httpServer.stop(do_test_finished);
+}
+
+function run_test()
+{
+  httpServer = new nsHttpServer();
+  httpServer.registerPathHandler(BadRedirectPath, BadRedirectHandler);
+  httpServer.start(4444);
+
+  var chan = make_channel(BadRedirectURI);
+  chan.asyncOpen(new ChannelListener(checkFailed, null, CL_EXPECT_FAILURE), 
+                 null);
+  do_test_pending();
+}

netwerk/test/unit/xpcshell.ini

@@ -164,6 +164,7 @@ skip-if = os == "win"
 [test_redirect_failure.js]
 [test_redirect_passing.js]
 [test_redirect_loop.js]
+[test_redirect_baduri.js]
 [test_reentrancy.js]
 [test_reopen.js]
 [test_resumable_channel.js]