Bug 460425. Do better security checks during redirection. r=sicking,biesi, sr=sicking
authorBoris Zbarsky <bzbarsky@mit.edu>
Tue, 25 Nov 2008 20:50:04 -0500
changeset 21937 5d1fbada858917573a87e817f732e7d4cc8559b3
parent 21936 328c67561b8bac4e1177ece95431dd26a1da28a2
child 21938 006236d9a61020c255ef45383760b021f49dfb4d
push id3748
push userbzbarsky@mozilla.com
push dateWed, 26 Nov 2008 01:51:07 +0000
treeherdermozilla-central@5d1fbada8589 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssicking, biesi, sicking
bugs460425
milestone1.9.1b3pre
Bug 460425. Do better security checks during redirection. r=sicking,biesi, sr=sicking
caps/src/nsScriptSecurityManager.cpp
content/base/src/nsContentUtils.cpp
content/base/src/nsCrossSiteListenerProxy.cpp
content/base/src/nsDocument.cpp
content/base/src/nsXMLHttpRequest.cpp
content/xml/document/src/nsXMLDocument.cpp
content/xml/document/src/nsXMLDocument.h
content/xml/document/test/test_bug392338.html
docshell/base/nsWebShell.cpp
dom/src/base/nsDOMClassInfo.cpp
netwerk/base/src/nsBaseChannel.cpp
netwerk/protocol/http/src/nsHttpChannel.cpp
uriloader/base/nsDocLoader.cpp
--- a/caps/src/nsScriptSecurityManager.cpp
+++ b/caps/src/nsScriptSecurityManager.cpp
@@ -3138,23 +3138,29 @@ nsScriptSecurityManager::OnChannelRedire
                                            nsIChannel* newChannel,
                                            PRUint32 redirFlags)
 {
     nsCOMPtr<nsIPrincipal> oldPrincipal;
     GetChannelPrincipal(oldChannel, getter_AddRefs(oldPrincipal));
 
     nsCOMPtr<nsIURI> newURI;
     newChannel->GetURI(getter_AddRefs(newURI));
-
-    NS_ENSURE_STATE(oldPrincipal && newURI);
+    nsCOMPtr<nsIURI> newOriginalURI;
+    newChannel->GetOriginalURI(getter_AddRefs(newOriginalURI));
+
+    NS_ENSURE_STATE(oldPrincipal && newURI && newOriginalURI);
 
     const PRUint32 flags =
         nsIScriptSecurityManager::LOAD_IS_AUTOMATIC_DOCUMENT_REPLACEMENT |
         nsIScriptSecurityManager::DISALLOW_SCRIPT;
-    return CheckLoadURIWithPrincipal(oldPrincipal, newURI, flags);
+    nsresult rv = CheckLoadURIWithPrincipal(oldPrincipal, newURI, flags);
+    if (NS_SUCCEEDED(rv) && newOriginalURI != newURI) {
+        rv = CheckLoadURIWithPrincipal(oldPrincipal, newOriginalURI, flags);
+    }
+    return rv;
 }
 
 
 /////////////////////////////////////
 // Method implementing nsIObserver //
 /////////////////////////////////////
 static const char sPrincipalPrefix[] = "capability.principal";
 static const char sPolicyPrefix[] = "capability.policy.";
--- a/content/base/src/nsContentUtils.cpp
+++ b/content/base/src/nsContentUtils.cpp
@@ -4482,27 +4482,36 @@ NS_IMPL_ISUPPORTS2(nsSameOriginChecker,
                    nsIInterfaceRequestor)
 
 NS_IMETHODIMP
 nsSameOriginChecker::OnChannelRedirect(nsIChannel *aOldChannel,
                                        nsIChannel *aNewChannel,
                                        PRUint32    aFlags)
 {
   NS_PRECONDITION(aNewChannel, "Redirecting to null channel?");
-
-  nsCOMPtr<nsIURI> oldURI;
-  nsresult rv = aOldChannel->GetURI(getter_AddRefs(oldURI)); // The original URI
-  NS_ENSURE_SUCCESS(rv, rv);
+  if (!nsContentUtils::GetSecurityManager()) {
+    return NS_ERROR_NOT_AVAILABLE;
+  }
+
+  nsCOMPtr<nsIPrincipal> oldPrincipal;
+  nsContentUtils::GetSecurityManager()->
+    GetChannelPrincipal(aOldChannel, getter_AddRefs(oldPrincipal));
 
   nsCOMPtr<nsIURI> newURI;
-  rv = aNewChannel->GetURI(getter_AddRefs(newURI)); // The new URI
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  return nsContentUtils::GetSecurityManager()->
-    CheckSameOriginURI(oldURI, newURI, PR_TRUE);
+  aNewChannel->GetURI(getter_AddRefs(newURI));
+  nsCOMPtr<nsIURI> newOriginalURI;
+  aNewChannel->GetOriginalURI(getter_AddRefs(newOriginalURI));
+
+  NS_ENSURE_STATE(oldPrincipal && newURI && newOriginalURI);
+
+  nsresult rv = oldPrincipal->CheckMayLoad(newURI, PR_FALSE);
+  if (NS_SUCCEEDED(rv) && newOriginalURI != newURI) {
+    rv = oldPrincipal->CheckMayLoad(newOriginalURI, PR_FALSE);
+  }
+  return rv;
 }
 
 NS_IMETHODIMP
 nsSameOriginChecker::GetInterface(const nsIID & aIID, void **aResult)
 {
   return QueryInterface(aIID, aResult);
 }
 
--- a/content/base/src/nsCrossSiteListenerProxy.cpp
+++ b/content/base/src/nsCrossSiteListenerProxy.cpp
@@ -346,28 +346,40 @@ nsCrossSiteListenerProxy::OnChannelRedir
   }
 
   return UpdateChannel(aNewChannel);
 }
 
 nsresult
 nsCrossSiteListenerProxy::UpdateChannel(nsIChannel* aChannel)
 {
-  nsCOMPtr<nsIURI> uri;
+  nsCOMPtr<nsIURI> uri, originalURI;
   nsresult rv = aChannel->GetURI(getter_AddRefs(uri));
   NS_ENSURE_SUCCESS(rv, rv);
+  rv = aChannel->GetOriginalURI(getter_AddRefs(originalURI));
+  NS_ENSURE_SUCCESS(rv, rv);  
 
   // Check that the uri is ok to load
   rv = nsContentUtils::GetSecurityManager()->
     CheckLoadURIWithPrincipal(mRequestingPrincipal, uri,
                               nsIScriptSecurityManager::STANDARD);
   NS_ENSURE_SUCCESS(rv, rv);
 
+  if (originalURI != uri) {
+    rv = nsContentUtils::GetSecurityManager()->
+      CheckLoadURIWithPrincipal(mRequestingPrincipal, originalURI,
+                                nsIScriptSecurityManager::STANDARD);
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
+
   if (!mHasBeenCrossSite &&
-      NS_SUCCEEDED(mRequestingPrincipal->CheckMayLoad(uri, PR_FALSE))) {
+      NS_SUCCEEDED(mRequestingPrincipal->CheckMayLoad(uri, PR_FALSE)) &&
+      (originalURI == uri ||
+       NS_SUCCEEDED(mRequestingPrincipal->CheckMayLoad(originalURI,
+                                                       PR_FALSE)))) {
     return NS_OK;
   }
 
   // It's a cross site load
   mHasBeenCrossSite = PR_TRUE;
 
   nsCString userpass;
   uri->GetUserPass(userpass);
--- a/content/base/src/nsDocument.cpp
+++ b/content/base/src/nsDocument.cpp
@@ -1112,27 +1112,26 @@ nsExternalResourceMap::PendingLoad::Star
     return NS_ERROR_CONTENT_BLOCKED;
   }
 
   nsIDocument* doc = aRequestingNode->GetOwnerDoc();
   if (!doc) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
+  nsCOMPtr<nsIInterfaceRequestor> req = nsContentUtils::GetSameOriginChecker();
+  NS_ENSURE_TRUE(req, NS_ERROR_OUT_OF_MEMORY);
+
   nsCOMPtr<nsILoadGroup> loadGroup = doc->GetDocumentLoadGroup();
   nsCOMPtr<nsIChannel> channel;
-  rv = NS_NewChannel(getter_AddRefs(channel), aURI, nsnull, loadGroup);
+  rv = NS_NewChannel(getter_AddRefs(channel), aURI, nsnull, loadGroup, req);
   NS_ENSURE_SUCCESS(rv, rv);
 
   mURI = aURI;
 
-  nsCOMPtr<nsIInterfaceRequestor> req = nsContentUtils::GetSameOriginChecker();
-  NS_ENSURE_TRUE(req, NS_ERROR_OUT_OF_MEMORY);
-
-  channel->SetNotificationCallbacks(req);
   return channel->AsyncOpen(this, nsnull);
 }
 
 NS_IMPL_ISUPPORTS1(nsExternalResourceMap::LoadgroupCallbacks,
                    nsIInterfaceRequestor)
 
 #define IMPL_SHIM(_i) \
   NS_IMPL_ISUPPORTS1(nsExternalResourceMap::LoadgroupCallbacks::_i##Shim, _i)
--- a/content/base/src/nsXMLHttpRequest.cpp
+++ b/content/base/src/nsXMLHttpRequest.cpp
@@ -1733,21 +1733,26 @@ IsSystemPrincipal(nsIPrincipal* aPrincip
   return isSystem;
 }
 
 static PRBool
 CheckMayLoad(nsIPrincipal* aPrincipal, nsIChannel* aChannel)
 {
   NS_ASSERTION(!IsSystemPrincipal(aPrincipal), "Shouldn't get here!");
 
-  nsCOMPtr<nsIURI> channelURI;
+  nsCOMPtr<nsIURI> channelURI, originalURI;
   nsresult rv = aChannel->GetURI(getter_AddRefs(channelURI));
   NS_ENSURE_SUCCESS(rv, PR_FALSE);
+  rv = aChannel->GetOriginalURI(getter_AddRefs(originalURI));
+  NS_ENSURE_SUCCESS(rv, PR_FALSE);
 
   rv = aPrincipal->CheckMayLoad(channelURI, PR_FALSE);
+  if (NS_SUCCEEDED(rv) && originalURI != channelURI) {
+    rv = aPrincipal->CheckMayLoad(originalURI, PR_FALSE);
+  }
   return NS_SUCCEEDED(rv);
 }
 
 nsresult
 nsXMLHttpRequest::CheckChannelForCrossSiteRequest(nsIChannel* aChannel)
 {
   // First check if this is a same-origin request, or if cross-site requests
   // are enabled.
--- a/content/xml/document/src/nsXMLDocument.cpp
+++ b/content/xml/document/src/nsXMLDocument.cpp
@@ -231,18 +231,16 @@ nsXMLDocument::~nsXMLDocument()
 {
   // XXX We rather crash than hang
   mLoopingForSyncLoad = PR_FALSE;
 }
 
 // QueryInterface implementation for nsXMLDocument
 NS_INTERFACE_TABLE_HEAD(nsXMLDocument)
   NS_DOCUMENT_INTERFACE_TABLE_BEGIN(nsXMLDocument)
-    NS_INTERFACE_TABLE_ENTRY(nsXMLDocument, nsIInterfaceRequestor)
-    NS_INTERFACE_TABLE_ENTRY(nsXMLDocument, nsIChannelEventSink)
     NS_INTERFACE_TABLE_ENTRY(nsXMLDocument, nsIDOMXMLDocument)
   NS_OFFSET_AND_INTERFACE_TABLE_END
   NS_OFFSET_AND_INTERFACE_TABLE_TO_MAP_SEGUE
   NS_INTERFACE_MAP_ENTRY_CONTENT_CLASSINFO(XMLDocument)
 NS_INTERFACE_MAP_END_INHERITING(nsDocument)
 
 
 NS_IMPL_ADDREF_INHERITED(nsXMLDocument, nsDocument)
@@ -272,45 +270,16 @@ nsXMLDocument::ResetToURI(nsIURI *aURI, 
     StopDocumentLoad();
     mChannel->Cancel(NS_BINDING_ABORTED);
     mChannelIsPending = nsnull;
   }
 
   nsDocument::ResetToURI(aURI, aLoadGroup, aPrincipal);
 }
 
-/////////////////////////////////////////////////////
-// nsIInterfaceRequestor methods:
-//
-NS_IMETHODIMP
-nsXMLDocument::GetInterface(const nsIID& aIID, void** aSink)
-{
-  return QueryInterface(aIID, aSink);
-}
-
-// nsIChannelEventSink
-NS_IMETHODIMP
-nsXMLDocument::OnChannelRedirect(nsIChannel *aOldChannel,
-                                 nsIChannel *aNewChannel,
-                                 PRUint32 aFlags)
-{
-  NS_PRECONDITION(aNewChannel, "Redirecting to null channel?");
-
-  nsCOMPtr<nsIURI> oldURI;
-  nsresult rv = aOldChannel->GetURI(getter_AddRefs(oldURI));
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  nsCOMPtr<nsIURI> newURI;
-  rv = aNewChannel->GetURI(getter_AddRefs(newURI));
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  return nsContentUtils::GetSecurityManager()->
-    CheckSameOriginURI(oldURI, newURI, PR_TRUE);
-}
-
 NS_IMETHODIMP
 nsXMLDocument::EvaluateFIXptr(const nsAString& aExpression, nsIDOMRange **aRange)
 {
   nsresult rv;
   nsCOMPtr<nsIFIXptrEvaluator> e =
     do_CreateInstance("@mozilla.org/xmlextras/fixptrevaluator;1", &rv);
   NS_ENSURE_SUCCESS(rv, rv);
   
@@ -433,21 +402,23 @@ nsXMLDocument::Load(const nsAString& aUr
     loadGroup = callingDoc->GetDocumentLoadGroup();
   }
 
   ResetToURI(uri, loadGroup, principal);
 
   mListenerManager = elm;
 
   // Create a channel
+  nsCOMPtr<nsIInterfaceRequestor> req = nsContentUtils::GetSameOriginChecker();
+  NS_ENSURE_TRUE(req, NS_ERROR_OUT_OF_MEMORY);  
 
   nsCOMPtr<nsIChannel> channel;
   // nsIRequest::LOAD_BACKGROUND prevents throbber from becoming active,
   // which in turn keeps STOP button from becoming active  
-  rv = NS_NewChannel(getter_AddRefs(channel), uri, nsnull, loadGroup, this, 
+  rv = NS_NewChannel(getter_AddRefs(channel), uri, nsnull, loadGroup, req, 
                      nsIRequest::LOAD_BACKGROUND);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
   // Prepare for loading the XML document "into oneself"
   nsCOMPtr<nsIStreamListener> listener;
   if (NS_FAILED(rv = StartDocumentLoad(kLoadAsData, channel, 
--- a/content/xml/document/src/nsXMLDocument.h
+++ b/content/xml/document/src/nsXMLDocument.h
@@ -34,32 +34,27 @@
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
 
 #ifndef nsXMLDocument_h___
 #define nsXMLDocument_h___
 
 #include "nsDocument.h"
-#include "nsIInterfaceRequestor.h"
-#include "nsIInterfaceRequestorUtils.h"
-#include "nsIChannelEventSink.h"
 #include "nsIDOMXMLDocument.h"
 #include "nsIScriptContext.h"
 #include "nsHTMLStyleSheet.h"
 #include "nsIHTMLCSSStyleSheet.h"
 
 class nsIParser;
 class nsIDOMNode;
 class nsIURI;
 class nsIChannel;
 
-class nsXMLDocument : public nsDocument,
-                      public nsIInterfaceRequestor,
-                      public nsIChannelEventSink
+class nsXMLDocument : public nsDocument
 {
 public:
   nsXMLDocument(const char* aContentType = "application/xml");
   virtual ~nsXMLDocument();
 
   NS_DECL_ISUPPORTS_INHERITED
 
   virtual void Reset(nsIChannel* aChannel, nsILoadGroup* aLoadGroup);
@@ -70,22 +65,16 @@ public:
                                      nsILoadGroup* aLoadGroup,
                                      nsISupports* aContainer,
                                      nsIStreamListener **aDocListener,
                                      PRBool aReset = PR_TRUE,
                                      nsIContentSink* aSink = nsnull);
 
   virtual void EndLoad();
 
-  // nsIInterfaceRequestor
-  NS_DECL_NSIINTERFACEREQUESTOR
-
-  // nsIHTTPEventSink
-  NS_DECL_NSICHANNELEVENTSINK
-
   // nsIDOMXMLDocument
   NS_DECL_NSIDOMXMLDOCUMENT
 
   virtual nsresult Init();
 
   virtual nsresult Clone(nsINodeInfo *aNodeInfo, nsINode **aResult) const;
 
 protected:
--- a/content/xml/document/test/test_bug392338.html
+++ b/content/xml/document/test/test_bug392338.html
@@ -25,46 +25,53 @@ function obs () {
                                      .getService(Components.interfaces.nsIObserverService);
   this.observerService.addObserver(this,"http-on-modify-request", false);
   this.window = window;
 }
 
 obs.prototype = {
   observe: function obs_observe (theSubject, theTopic, theData)
   {
+    this.timeToFinish = true;
     try{
       this.window.netscape.security
           .PrivilegeManager.enablePrivilege("UniversalXPConnect");
       var ir = theSubject.QueryInterface(this.window.Components.interfaces
                                              .nsIChannel).notificationCallbacks;
-      if (!(ir instanceof this.window.Components.interfaces.nsIDOMDocument)) {
+      if (!ir) {
+        this.timeToFinish = false;
         return;
       }
+      this.window.ok(true, "No exception thrown");
 
       this.window.is(ir instanceof
                        this.window.Components.interfaces.nsIInterfaceRequestor,
                      true, "Must be an interface requestor");
 
-      var count = {};
-      var interfaces = ir.
-                   QueryInterface(this.window.Components
-                                      .interfaces.nsIClassInfo).
-                   getInterfaces(count).
-                   map(function(id) {
-                              return this.window.Components
-                                                .interfacesByID[id].toString();
-                       });
-      this.window.isnot(interfaces.indexOf("nsIInterfaceRequestor"), -1,
-                        "Must have interface requestor classinfo");
+      if (ir instanceof this.window.Components.interfaces.nsIClassInfo) {
+        var count = {};
+        var interfaces = ir.
+                     QueryInterface(this.window.Components
+                                        .interfaces.nsIClassInfo).
+                     getInterfaces(count).
+                     map(function(id) {
+                                return this.window.Components
+                                                  .interfacesByID[id].toString();
+                         });
+        this.window.isnot(interfaces.indexOf("nsIInterfaceRequestor"), -1,
+                          "Must have interface requestor classinfo");
+      }
     } catch(ex) {
       this.window.is(true, false, "Exception thrown " + ex);
     } finally {
-      this.remove();
-      this.window.SimpleTest.finish();
-      this.window = null;
+      if (this.timeToFinish) {
+        this.remove();
+        this.window.SimpleTest.finish();
+        this.window = null;
+      }
     }
   },
 
   remove: function obs_remove()
   {
     netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect");
     this.observerService.removeObserver(this,"http-on-modify-request");
     this.observerService = null;
--- a/docshell/base/nsWebShell.cpp
+++ b/docshell/base/nsWebShell.cpp
@@ -385,16 +385,18 @@ nsPingListener::OnChannelRedirect(nsICha
   newChan->GetURI(getter_AddRefs(newURI));
 
   if (!CheckPingURI(newURI, mContent))
     return NS_ERROR_ABORT;
 
   if (!mRequireSameHost)
     return NS_OK;
 
+  // XXXbz should this be using something more like the nsContentUtils
+  // same-origin checker?
   nsCOMPtr<nsIURI> oldURI;
   oldChan->GetURI(getter_AddRefs(oldURI));
   NS_ENSURE_STATE(oldURI && newURI);
 
   if (!IsSameHost(oldURI, newURI))
     return NS_ERROR_ABORT;
 
   return NS_OK;
--- a/dom/src/base/nsDOMClassInfo.cpp
+++ b/dom/src/base/nsDOMClassInfo.cpp
@@ -2033,17 +2033,16 @@ nsDOMClassInfo::Init()
 
   DOM_CLASSINFO_MAP_BEGIN(DOMConstructor, nsIDOMDOMConstructor)
     DOM_CLASSINFO_MAP_ENTRY(nsIDOMDOMConstructor)
   DOM_CLASSINFO_MAP_END
 
   DOM_CLASSINFO_MAP_BEGIN(XMLDocument, nsIDOMXMLDocument)
     DOM_CLASSINFO_MAP_ENTRY(nsIDOMDocument)
     DOM_CLASSINFO_MAP_ENTRY(nsIDOMXMLDocument)
-    DOM_CLASSINFO_MAP_ENTRY(nsIInterfaceRequestor)
     DOM_CLASSINFO_DOCUMENT_MAP_ENTRIES
   DOM_CLASSINFO_MAP_END
 
   DOM_CLASSINFO_MAP_BEGIN(DocumentType, nsIDOMDocumentType)
     DOM_CLASSINFO_MAP_ENTRY(nsIDOMDocumentType)
     DOM_CLASSINFO_MAP_ENTRY(nsIDOMEventTarget)
     DOM_CLASSINFO_MAP_ENTRY(nsIDOM3Node)
   DOM_CLASSINFO_MAP_END
--- a/netwerk/base/src/nsBaseChannel.cpp
+++ b/netwerk/base/src/nsBaseChannel.cpp
@@ -97,17 +97,16 @@ nsBaseChannel::nsBaseChannel()
 nsresult
 nsBaseChannel::Redirect(nsIChannel *newChannel, PRUint32 redirectFlags,
                         PRBool openNewChannel)
 {
   SUSPEND_PUMP_FOR_SCOPE();
 
   // Transfer properties
 
-  newChannel->SetOriginalURI(OriginalURI());
   newChannel->SetLoadGroup(mLoadGroup);
   newChannel->SetNotificationCallbacks(mCallbacks);
   newChannel->SetLoadFlags(mLoadFlags | LOAD_REPLACE);
 
   nsCOMPtr<nsIWritablePropertyBag> bag = ::do_QueryInterface(newChannel);
   if (bag)
     mPropertyHash.EnumerateRead(CopyProperties, bag.get());
 
@@ -140,16 +139,19 @@ nsBaseChannel::Redirect(nsIChannel *newC
   // Give our consumer a chance to observe/block this redirect.
   GetCallback(channelEventSink);
   if (channelEventSink) {
     rv = channelEventSink->OnChannelRedirect(this, newChannel, redirectFlags);
     if (NS_FAILED(rv))
       return rv;
   }
 
+  // Make sure to do this _after_ making all the  OnChannelRedirect calls
+  newChannel->SetOriginalURI(OriginalURI());
+
   // If we fail to open the new channel, then we want to leave this channel
   // unaffected, so we defer tearing down our channel until we have succeeded
   // with the redirect.
 
   if (openNewChannel) {
     rv = newChannel->AsyncOpen(mListener, mListenerContext);
     if (NS_FAILED(rv))
       return rv;
--- a/netwerk/protocol/http/src/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/src/nsHttpChannel.cpp
@@ -1128,16 +1128,19 @@ nsHttpChannel::DoReplaceWithProxy(nsIPro
         return rv;
 
     // Inform consumers about this fake redirect
     PRUint32 flags = nsIChannelEventSink::REDIRECT_INTERNAL;
     rv = gHttpHandler->OnChannelRedirect(this, newChannel, flags);
     if (NS_FAILED(rv))
         return rv;
 
+    // Make sure to do this _after_ calling OnChannelRedirect
+    newChannel->SetOriginalURI(mOriginalURI);
+
     // open new channel
     rv = newChannel->AsyncOpen(mListener, mListenerContext);
     if (NS_FAILED(rv))
         return rv;
 
     mStatus = NS_BINDING_REDIRECTED;
     mListener = nsnull;
     mListenerContext = nsnull;
@@ -1458,16 +1461,19 @@ nsHttpChannel::ProcessFallback(PRBool *f
     rv = newChannel->SetLoadFlags(newLoadFlags);
 
     // Inform consumers about this fake redirect
     PRUint32 redirectFlags = nsIChannelEventSink::REDIRECT_INTERNAL;
     rv = gHttpHandler->OnChannelRedirect(this, newChannel, redirectFlags);
     if (NS_FAILED(rv))
         return rv;
 
+    // Make sure to do this _after_ calling OnChannelRedirect
+    newChannel->SetOriginalURI(mOriginalURI);
+    
     rv = newChannel->AsyncOpen(mListener, mListenerContext);
     NS_ENSURE_SUCCESS(rv, rv);
 
     // close down this channel
     Cancel(NS_BINDING_REDIRECTED);
 
     // disconnect from our listener
     mListener = 0;
@@ -2577,17 +2583,16 @@ nsHttpChannel::SetupReplacementChannel(n
     // since we force set INHIBIT_PERSISTENT_CACHING on all HTTPS channels,
     // we only need to check if the original channel was using SSL.
     if (mConnectionInfo->UsingSSL())
         newLoadFlags &= ~INHIBIT_PERSISTENT_CACHING;
 
     // Do not pass along LOAD_CHECK_OFFLINE_CACHE
     newLoadFlags &= ~LOAD_CHECK_OFFLINE_CACHE;
 
-    newChannel->SetOriginalURI(mOriginalURI);
     newChannel->SetLoadGroup(mLoadGroup); 
     newChannel->SetNotificationCallbacks(mCallbacks);
     newChannel->SetLoadFlags(newLoadFlags);
 
     nsCOMPtr<nsIHttpChannel> httpChannel = do_QueryInterface(newChannel);
     if (!httpChannel)
         return NS_OK; // no other options to set
 
@@ -2758,16 +2763,19 @@ nsHttpChannel::ProcessRedirection(PRUint
     else
         redirectFlags = nsIChannelEventSink::REDIRECT_TEMPORARY;
 
     // verify that this is a legal redirect
     rv = gHttpHandler->OnChannelRedirect(this, newChannel, redirectFlags);
     if (NS_FAILED(rv))
         return rv;
 
+    // Make sure to do this _after_ calling OnChannelRedirect
+    newChannel->SetOriginalURI(mOriginalURI);    
+
     // And now, the deprecated way
     nsCOMPtr<nsIHttpEventSink> httpEventSink;
     GetCallback(httpEventSink);
     if (httpEventSink) {
         // NOTE: nsIHttpEventSink is only used for compatibility with pre-1.8
         // versions.
         rv = httpEventSink->OnRedirect(this, newChannel);
         if (NS_FAILED(rv)) return rv;
--- a/uriloader/base/nsDocLoader.cpp
+++ b/uriloader/base/nsDocLoader.cpp
@@ -1482,25 +1482,16 @@ PRInt64 nsDocLoader::CalculateMaxProgres
 }
 
 NS_IMETHODIMP nsDocLoader::OnChannelRedirect(nsIChannel *aOldChannel,
                                              nsIChannel *aNewChannel,
                                              PRUint32    aFlags)
 {
   if (aOldChannel)
   {
-    nsresult rv;
-    nsCOMPtr<nsIURI> oldURI, newURI;
-
-    rv = aOldChannel->GetOriginalURI(getter_AddRefs(oldURI));
-    if (NS_FAILED(rv)) return rv;
-
-    rv = aNewChannel->GetURI(getter_AddRefs(newURI));
-    if (NS_FAILED(rv)) return rv;
-
     nsLoadFlags loadFlags = 0;
     PRInt32 stateFlags = nsIWebProgressListener::STATE_REDIRECTING |
                          nsIWebProgressListener::STATE_IS_REQUEST;
 
     aOldChannel->GetLoadFlags(&loadFlags);
     // If the document channel is being redirected, then indicate that the
     // document is being redirected in the notification...
     if (loadFlags & nsIChannel::LOAD_DOCUMENT_URI)