From d2cd0b67a2c43fe2d0f90361f71d24b1ca43eae3 Mon Sep 17 00:00:00 2001 From: Jason Duell Date: Thu, 3 May 2012 00:28:57 -0700 Subject: [PATCH] Bug 738484: part 2: fix missing onStartRequest calls for some failed redirects. r=mcmanus --- netwerk/protocol/http/nsHttpChannel.cpp | 15 +++++--- netwerk/protocol/http/nsHttpChannel.h | 4 +++ netwerk/test/unit/test_redirect_baduri.js | 42 +++++++++++++++++++++++ netwerk/test/unit/xpcshell.ini | 1 + 4 files changed, 58 insertions(+), 4 deletions(-) create mode 100644 netwerk/test/unit/test_redirect_baduri.js diff --git a/netwerk/protocol/http/nsHttpChannel.cpp b/netwerk/protocol/http/nsHttpChannel.cpp index c2939d66c5b..06466a773e6 100644 --- a/netwerk/protocol/http/nsHttpChannel.cpp +++ b/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; } diff --git a/netwerk/protocol/http/nsHttpChannel.h b/netwerk/protocol/http/nsHttpChannel.h index dbebc043c0e..749ebdfa209 100644 --- a/netwerk/protocol/http/nsHttpChannel.h +++ b/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 mSecurityInfo; nsCOMPtr mProxyRequest; diff --git a/netwerk/test/unit/test_redirect_baduri.js b/netwerk/test/unit/test_redirect_baduri.js new file mode 100644 index 00000000000..f6ac01a8780 --- /dev/null +++ b/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(); +} diff --git a/netwerk/test/unit/xpcshell.ini b/netwerk/test/unit/xpcshell.ini index 0c912f4c470..4394c2502c8 100644 --- a/netwerk/test/unit/xpcshell.ini +++ b/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]