Bug 1182571: Make nsXMLHttpRequest use AsyncOpen2. r=ehsan
authorJonas Sicking <jonas@sicking.cc>
Mon, 19 Oct 2015 11:14:54 -0700
changeset 303568 ae826f6849c430ad1c32f5e1c89c4fb5aaa44831
parent 303567 a31b2d7d07b80484ae2b66689559e39108a7ee71
child 303569 c159526f914587b787d17d352b3d82c5bbae0e05
push id1001
push userraliiev@mozilla.com
push dateMon, 18 Jan 2016 19:06:03 +0000
treeherdermozilla-release@8b89261f3ac4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersehsan
bugs1182571
milestone44.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1182571: Make nsXMLHttpRequest use AsyncOpen2. r=ehsan
dom/base/nsXMLHttpRequest.cpp
dom/base/nsXMLHttpRequest.h
dom/base/test/file_XHRResponseURL.js
dom/security/nsContentSecurityManager.cpp
dom/security/test/mixedcontentblocker/file_main.html
dom/workers/test/test_xhr_responseURL.html
netwerk/base/LoadInfo.cpp
netwerk/base/LoadInfo.h
netwerk/protocol/http/nsCORSListenerProxy.cpp
netwerk/protocol/http/nsCORSListenerProxy.h
testing/web-platform/meta/content-security-policy/blink-contrib/shared-worker-connect-src-allowed.sub.html.ini
testing/web-platform/meta/content-security-policy/blink-contrib/shared-worker-connect-src-blocked.sub.html.ini
testing/web-platform/meta/content-security-policy/blink-contrib/worker-connect-src-blocked.sub.html.ini
testing/web-platform/meta/cors/redirect-userinfo.htm.ini
testing/web-platform/tests/content-security-policy/blink-contrib/connect-src-xmlhttprequest-blocked.sub.html
--- a/dom/base/nsXMLHttpRequest.cpp
+++ b/dom/base/nsXMLHttpRequest.cpp
@@ -46,20 +46,17 @@
 #include "nsIDOMWindow.h"
 #include "nsIVariant.h"
 #include "nsVariant.h"
 #include "nsIScriptError.h"
 #include "nsIStreamConverterService.h"
 #include "nsICachingChannel.h"
 #include "nsContentUtils.h"
 #include "nsCycleCollectionParticipant.h"
-#include "nsIContentPolicy.h"
-#include "nsContentPolicyUtils.h"
 #include "nsError.h"
-#include "nsCORSListenerProxy.h"
 #include "nsIHTMLDocument.h"
 #include "nsIStorageStream.h"
 #include "nsIPromptFactory.h"
 #include "nsIWindowWatcher.h"
 #include "nsIConsoleService.h"
 #include "nsIContentSecurityPolicy.h"
 #include "nsAsyncRedirectVerifyHelper.h"
 #include "nsStringBuffer.h"
@@ -121,17 +118,17 @@ using namespace mozilla::dom;
 // The above states are mutually exclusive, change with ChangeState() only.
 // The states below can be combined.
 #define XML_HTTP_REQUEST_ABORTED        (1 << 7)  // Internal
 #define XML_HTTP_REQUEST_ASYNC          (1 << 8)  // Internal
 #define XML_HTTP_REQUEST_PARSEBODY      (1 << 9)  // Internal
 #define XML_HTTP_REQUEST_SYNCLOOPING    (1 << 10) // Internal
 #define XML_HTTP_REQUEST_BACKGROUND     (1 << 13) // Internal
 #define XML_HTTP_REQUEST_USE_XSITE_AC   (1 << 14) // Internal
-#define XML_HTTP_REQUEST_NEED_AC_PREFLIGHT (1 << 15) // Internal
+#define XML_HTTP_REQUEST_NEED_AC_PREFLIGHT_IF_XSITE (1 << 15) // Internal
 #define XML_HTTP_REQUEST_AC_WITH_CREDENTIALS (1 << 16) // Internal
 #define XML_HTTP_REQUEST_TIMED_OUT (1 << 17) // Internal
 #define XML_HTTP_REQUEST_DELETED (1 << 18) // Internal
 
 #define XML_HTTP_REQUEST_LOADSTATES         \
   (XML_HTTP_REQUEST_UNSENT |                \
    XML_HTTP_REQUEST_OPENED |                \
    XML_HTTP_REQUEST_HEADERS_RECEIVED |      \
@@ -1528,57 +1525,25 @@ nsXMLHttpRequest::GetCurrentJARChannel()
 }
 
 bool
 nsXMLHttpRequest::IsSystemXHR()
 {
   return mIsSystem || nsContentUtils::IsSystemPrincipal(mPrincipal);
 }
 
-nsresult
+void
 nsXMLHttpRequest::CheckChannelForCrossSiteRequest(nsIChannel* aChannel)
 {
   // A system XHR (chrome code or a web app with the right permission) can
-  // always perform cross-site requests. In the web app case, however, we
-  // must still check for protected URIs like file:///.
-  if (IsSystemXHR()) {
-    if (!nsContentUtils::IsSystemPrincipal(mPrincipal)) {
-      nsIScriptSecurityManager *secMan = nsContentUtils::GetSecurityManager();
-      nsCOMPtr<nsIURI> uri;
-      aChannel->GetOriginalURI(getter_AddRefs(uri));
-      return secMan->CheckLoadURIWithPrincipal(
-        mPrincipal, uri, nsIScriptSecurityManager::STANDARD);
-    }
-    return NS_OK;
-  }
-
-  // If this is a same-origin request or the channel's URI inherits
-  // its principal, it's allowed.
-  if (nsContentUtils::CheckMayLoad(mPrincipal, aChannel, true)) {
-    return NS_OK;
+  // load anything, and same-origin loads are always allowed.
+  if (!IsSystemXHR() &&
+      !nsContentUtils::CheckMayLoad(mPrincipal, aChannel, true)) {
+    mState |= XML_HTTP_REQUEST_USE_XSITE_AC;
   }
-
-  // This is a cross-site request
-  mState |= XML_HTTP_REQUEST_USE_XSITE_AC;
-
-  // Check if we need to do a preflight request.
-  nsCOMPtr<nsIHttpChannel> httpChannel = do_QueryInterface(aChannel);
-  NS_ENSURE_TRUE(httpChannel, NS_ERROR_DOM_BAD_URI);
-
-  nsAutoCString method;
-  httpChannel->GetRequestMethod(method);
-  if (!mCORSUnsafeHeaders.IsEmpty() ||
-      (mUpload && mUpload->HasListeners()) ||
-      (!method.LowerCaseEqualsLiteral("get") &&
-       !method.LowerCaseEqualsLiteral("post") &&
-       !method.LowerCaseEqualsLiteral("head"))) {
-    mState |= XML_HTTP_REQUEST_NEED_AC_PREFLIGHT;
-  }
-
-  return NS_OK;
 }
 
 NS_IMETHODIMP
 nsXMLHttpRequest::Open(const nsACString& method, const nsACString& url,
                        bool async, const nsAString& user,
                        const nsAString& password, uint8_t optional_argc)
 {
   if (!optional_argc) {
@@ -1675,31 +1640,16 @@ nsXMLHttpRequest::Open(const nsACString&
     baseURI = doc->GetBaseURI();
   }
 
   rv = NS_NewURI(getter_AddRefs(uri), url, nullptr, baseURI);
   if (NS_FAILED(rv)) return rv;
 
   rv = CheckInnerWindowCorrectness();
   NS_ENSURE_SUCCESS(rv, rv);
-  int16_t shouldLoad = nsIContentPolicy::ACCEPT;
-  rv = NS_CheckContentLoadPolicy(nsIContentPolicy::TYPE_INTERNAL_XMLHTTPREQUEST,
-                                 uri,
-                                 mPrincipal,
-                                 doc,
-                                 EmptyCString(), //mime guess
-                                 nullptr,         //extra
-                                 &shouldLoad,
-                                 nsContentUtils::GetContentPolicy(),
-                                 nsContentUtils::GetSecurityManager());
-  if (NS_FAILED(rv)) return rv;
-  if (NS_CP_REJECTED(shouldLoad)) {
-    // Disallowed by content policy
-    return NS_ERROR_CONTENT_BLOCKED;
-  }
 
   // XXXbz this is wrong: we should only be looking at whether
   // user/password were passed, not at the values!  See bug 759624.
   if (user.WasPassed() && !user.Value().IsEmpty()) {
     nsAutoCString userpass;
     CopyUTF16toUTF8(user.Value(), userpass);
     if (password.WasPassed() && !password.Value().IsEmpty()) {
       userpass.Append(':');
@@ -1712,30 +1662,37 @@ nsXMLHttpRequest::Open(const nsACString&
   // operations will merge/override correctly.
   mAlreadySetHeaders.Clear();
 
   // When we are called from JS we can find the load group for the page,
   // and add ourselves to it. This way any pending requests
   // will be automatically aborted if the user leaves the page.
   nsCOMPtr<nsILoadGroup> loadGroup = GetLoadGroup();
 
-  nsSecurityFlags secFlags = nsILoadInfo::SEC_NORMAL;
+  nsSecurityFlags secFlags;
   nsLoadFlags loadFlags = nsIRequest::LOAD_BACKGROUND;
-  if (IsSystemXHR()) {
-    // Don't give this document the system principal.  We need to keep track of
-    // mPrincipal being system because we use it for various security checks
-    // that should be passing, but the document data shouldn't get a system
-    // principal.  Hence we set the sandbox flag in loadinfo, so that 
-    // GetChannelResultPrincipal will give us the nullprincipal.
-    secFlags |= nsILoadInfo::SEC_SANDBOXED;
-
-    //For a XHR, disable interception
+  if (nsContentUtils::IsSystemPrincipal(mPrincipal)) {
+    // When chrome is loading we want to make sure to sandbox any potential
+    // result document. We also want to allow cross-origin loads.
+    secFlags = nsILoadInfo::SEC_ALLOW_CROSS_ORIGIN_DATA_IS_NULL |
+               nsILoadInfo::SEC_SANDBOXED;
+  }
+  else if (IsSystemXHR()) {
+    // For pages that have appropriate permissions, we want to still allow
+    // cross-origin loads, but make sure that the any potential result
+    // documents get the same principal as the loader.
+    secFlags = nsILoadInfo::SEC_ALLOW_CROSS_ORIGIN_DATA_INHERITS |
+               nsILoadInfo::SEC_FORCE_INHERIT_PRINCIPAL;
     loadFlags |= nsIChannel::LOAD_BYPASS_SERVICE_WORKER;
-  } else {
-    secFlags |= nsILoadInfo::SEC_FORCE_INHERIT_PRINCIPAL;
+  }
+  else {
+    // Otherwise use CORS. Again, make sure that potential result documents
+    // use the same principal as the loader.
+    secFlags = nsILoadInfo::SEC_REQUIRE_CORS_DATA_INHERITS |
+               nsILoadInfo::SEC_FORCE_INHERIT_PRINCIPAL;
   }
 
   // If we have the document, use it
   if (doc) {
     rv = NS_NewChannel(getter_AddRefs(mChannel),
                        uri,
                        doc,
                        secFlags,
@@ -1750,36 +1707,36 @@ nsXMLHttpRequest::Open(const nsACString&
                        mPrincipal,
                        secFlags,
                        nsIContentPolicy::TYPE_INTERNAL_XMLHTTPREQUEST,
                        loadGroup,
                        nullptr,   // aCallbacks
                        loadFlags);
   }
 
-  if (NS_FAILED(rv)) return rv;
+  NS_ENSURE_SUCCESS(rv, rv);
 
   mState &= ~(XML_HTTP_REQUEST_USE_XSITE_AC |
-              XML_HTTP_REQUEST_NEED_AC_PREFLIGHT);
+              XML_HTTP_REQUEST_NEED_AC_PREFLIGHT_IF_XSITE);
 
   nsCOMPtr<nsIHttpChannel> httpChannel(do_QueryInterface(mChannel));
   if (httpChannel) {
     rv = httpChannel->SetRequestMethod(method);
     NS_ENSURE_SUCCESS(rv, rv);
 
     // Set the initiator type
     nsCOMPtr<nsITimedChannel> timedChannel(do_QueryInterface(httpChannel));
     if (timedChannel) {
       timedChannel->SetInitiatorType(NS_LITERAL_STRING("xmlhttprequest"));
     }
   }
 
   ChangeState(XML_HTTP_REQUEST_OPENED);
 
-  return rv;
+  return NS_OK;
 }
 
 void
 nsXMLHttpRequest::PopulateNetworkInterfaceId()
 {
   if (mNetworkInterfaceId.IsEmpty()) {
     return;
   }
@@ -2813,24 +2770,31 @@ nsXMLHttpRequest::Send(nsIVariant* aVari
       if (!nsContentUtils::IsAllowedNonCorsContentType(contentTypeHeader)) {
         mCORSUnsafeHeaders.AppendElement(NS_LITERAL_CSTRING("Content-Type"));
       }
     }
   }
 
   ResetResponse();
 
-  rv = CheckChannelForCrossSiteRequest(mChannel);
-  NS_ENSURE_SUCCESS(rv, rv);
+  CheckChannelForCrossSiteRequest(mChannel);
 
   bool withCredentials = !!(mState & XML_HTTP_REQUEST_AC_WITH_CREDENTIALS);
 
-  // Hook us up to listen to redirects and the like
-  mChannel->GetNotificationCallbacks(getter_AddRefs(mNotificationCallbacks));
-  mChannel->SetNotificationCallbacks(this);
+  if (!IsSystemXHR() && withCredentials) {
+    // This is quite sad. We have to create the channel in .open(), since the
+    // chrome-only xhr.channel API depends on that. However .withCredentials
+    // can be modified after, so we don't know what to set the
+    // SEC_REQUIRE_CORS_WITH_CREDENTIALS flag to when the channel is
+    // created. So set it here using a hacky internal API.
+
+    // Not doing this for system XHR uses since those don't use CORS.
+    nsCOMPtr<nsILoadInfo> loadInfo = mChannel->GetLoadInfo();
+    static_cast<LoadInfo*>(loadInfo.get())->SetWithCredentialsSecFlag();
+  }
 
   // Blocking gets are common enough out of XHR that we should mark
   // the channel slow by default for pipeline purposes
   AddLoadFlags(mChannel, nsIRequest::INHIBIT_PIPELINE);
 
   nsCOMPtr<nsIClassOfService> cos(do_QueryInterface(mChannel));
   if (cos) {
     // we never let XHR be blocked by head CSS/JS loads to avoid
@@ -2841,34 +2805,16 @@ nsXMLHttpRequest::Send(nsIVariant* aVari
 
   nsCOMPtr<nsIHttpChannelInternal>
     internalHttpChannel(do_QueryInterface(mChannel));
   if (internalHttpChannel) {
     // Disable Necko-internal response timeouts.
     internalHttpChannel->SetResponseTimeoutEnabled(false);
   }
 
-  nsCOMPtr<nsIStreamListener> listener = this;
-  if (!IsSystemXHR()) {
-    // Always create a nsCORSListenerProxy here even if it's
-    // a same-origin request right now, since it could be redirected.
-    RefPtr<nsCORSListenerProxy> corsListener =
-      new nsCORSListenerProxy(listener, mPrincipal, withCredentials);
-    rv = corsListener->Init(mChannel, DataURIHandling::Allow);
-    NS_ENSURE_SUCCESS(rv, rv);
-    listener = corsListener;
-  }
-  else {
-    // Because of bug 682305, we can't let listener be the XHR object itself
-    // because JS wouldn't be able to use it. So if we haven't otherwise
-    // created a listener around 'this', do so now.
-
-    listener = new nsStreamListenerWrapper(listener);
-  }
-
   if (mIsAnon) {
     AddLoadFlags(mChannel, nsIRequest::LOAD_ANONYMOUS);
   }
   else {
     AddLoadFlags(mChannel, nsIChannel::LOAD_EXPLICIT_CREDENTIALS);
   }
 
   NS_ASSERTION(listener != this,
@@ -2901,20 +2847,29 @@ nsXMLHttpRequest::Send(nsIVariant* aVari
       contentType.Equals(UNKNOWN_CONTENT_TYPE)) {
     mChannel->SetContentType(NS_LITERAL_CSTRING("application/xml"));
   }
 
   // We're about to send the request.  Start our timeout.
   mRequestSentTime = PR_Now();
   StartTimeoutTimer();
 
+  // Check if we need to do a preflight request.
+  if (!mCORSUnsafeHeaders.IsEmpty() ||
+      (mUpload && mUpload->HasListeners()) ||
+      (!method.LowerCaseEqualsLiteral("get") &&
+       !method.LowerCaseEqualsLiteral("post") &&
+       !method.LowerCaseEqualsLiteral("head"))) {
+    mState |= XML_HTTP_REQUEST_NEED_AC_PREFLIGHT_IF_XSITE;
+  }
+
   // Set up the preflight if needed
-  if (mState & XML_HTTP_REQUEST_NEED_AC_PREFLIGHT) {
-    // Check to see if this initial OPTIONS request has already been cached
-    // in our special Access Control Cache.
+  if ((mState & XML_HTTP_REQUEST_USE_XSITE_AC) &&
+      (mState & XML_HTTP_REQUEST_NEED_AC_PREFLIGHT_IF_XSITE)) {
+    NS_ENSURE_TRUE(internalHttpChannel, NS_ERROR_DOM_BAD_URI);
 
     rv = internalHttpChannel->SetCorsPreflightParameters(mCORSUnsafeHeaders,
                                                          withCredentials, mPrincipal);
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
   mIsMappedArrayBuffer = false;
   if (mResponseType == XML_HTTP_RESPONSE_TYPE_ARRAYBUFFER &&
@@ -2927,26 +2882,40 @@ nsXMLHttpRequest::Send(nsIVariant* aVari
       uri->GetScheme(scheme);
       if (scheme.LowerCaseEqualsLiteral("app") ||
           scheme.LowerCaseEqualsLiteral("jar")) {
         mIsMappedArrayBuffer = true;
       }
     }
   }
 
+  // Hook us up to listen to redirects and the like
+  // Only do this very late since this creates a cycle between
+  // the channel and us. This cycle has to be manually broken if anything
+  // below fails.
+  mChannel->GetNotificationCallbacks(getter_AddRefs(mNotificationCallbacks));
+  mChannel->SetNotificationCallbacks(this);
+
   // Start reading from the channel
-  rv = mChannel->AsyncOpen(listener, nullptr);
+  // Because of bug 682305, we can't let listener be the XHR object itself
+  // because JS wouldn't be able to use it. So create a listener around 'this'.
+  // Make sure to hold a strong reference so that we don't leak the wrapper.
+  nsCOMPtr<nsIStreamListener> listener = new nsStreamListenerWrapper(this);
+  rv = mChannel->AsyncOpen2(listener);
+  listener = nullptr;
 
   if (NS_WARN_IF(NS_FAILED(rv))) {
-    // Drop our ref to the channel to avoid cycles
+    // Drop our ref to the channel to avoid cycles. Also drop channel's
+    // ref to us to be extra safe.
+    mChannel->SetNotificationCallbacks(mNotificationCallbacks);
     mChannel = nullptr;
+
     return rv;
   }
 
-  // Either AsyncOpen was called, or CORS will open the channel later.
   mWaitingForOnStopRequest = true;
 
   // If we're synchronous, spin an event loop here and wait
   if (!(mState & XML_HTTP_REQUEST_ASYNC)) {
     mState |= XML_HTTP_REQUEST_SYNCLOOPING;
 
     nsCOMPtr<nsIDocument> suspendedDoc;
     nsCOMPtr<nsIRunnable> resumeTimeoutRunnable;
@@ -3379,27 +3348,22 @@ nsXMLHttpRequest::AsyncOnChannelRedirect
                                          uint32_t    aFlags,
                                          nsIAsyncVerifyRedirectCallback *callback)
 {
   NS_PRECONDITION(aNewChannel, "Redirect without a channel?");
 
   nsresult rv;
 
   if (!NS_IsInternalSameURIRedirect(aOldChannel, aNewChannel, aFlags)) {
-    rv = CheckChannelForCrossSiteRequest(aNewChannel);
-    if (NS_FAILED(rv)) {
-      NS_WARNING("nsXMLHttpRequest::OnChannelRedirect: "
-                 "CheckChannelForCrossSiteRequest returned failure");
-      return rv;
-    }
+    CheckChannelForCrossSiteRequest(aNewChannel);
 
     // Disable redirects for preflighted cross-site requests entirely for now
-    // Note, do this after the call to CheckChannelForCrossSiteRequest
-    // to make sure that XML_HTTP_REQUEST_USE_XSITE_AC is up-to-date
-    if ((mState & XML_HTTP_REQUEST_NEED_AC_PREFLIGHT)) {
+    if ((mState & XML_HTTP_REQUEST_USE_XSITE_AC) &&
+        (mState & XML_HTTP_REQUEST_NEED_AC_PREFLIGHT_IF_XSITE)) {
+       aOldChannel->Cancel(NS_ERROR_DOM_BAD_URI);
        return NS_ERROR_DOM_BAD_URI;
     }
   }
 
   // Prepare to receive callback
   mRedirectCallback = callback;
   mNewRedirectChannel = aNewChannel;
 
@@ -3550,17 +3514,17 @@ nsXMLHttpRequest::OnStatus(nsIRequest *a
 
   return NS_OK;
 }
 
 bool
 nsXMLHttpRequest::AllowUploadProgress()
 {
   return !(mState & XML_HTTP_REQUEST_USE_XSITE_AC) ||
-    (mState & XML_HTTP_REQUEST_NEED_AC_PREFLIGHT);
+    (mState & XML_HTTP_REQUEST_NEED_AC_PREFLIGHT_IF_XSITE);
 }
 
 /////////////////////////////////////////////////////
 // nsIInterfaceRequestor methods:
 //
 NS_IMETHODIMP
 nsXMLHttpRequest::GetInterface(const nsIID & aIID, void **aResult)
 {
--- a/dom/base/nsXMLHttpRequest.h
+++ b/dom/base/nsXMLHttpRequest.h
@@ -621,17 +621,17 @@ protected:
   void ChangeStateToDone();
 
   /**
    * Check if aChannel is ok for a cross-site request by making sure no
    * inappropriate headers are set, and no username/password is set.
    *
    * Also updates the XML_HTTP_REQUEST_USE_XSITE_AC bit.
    */
-  nsresult CheckChannelForCrossSiteRequest(nsIChannel* aChannel);
+  void CheckChannelForCrossSiteRequest(nsIChannel* aChannel);
 
   void StartProgressEventTimer();
 
   friend class AsyncVerifyRedirectCallbackForwarder;
   void OnRedirectVerifyCallback(nsresult result);
 
   nsresult Open(const nsACString& method, const nsACString& url, bool async,
                 const mozilla::dom::Optional<nsAString>& user,
--- a/dom/base/test/file_XHRResponseURL.js
+++ b/dom/base/test/file_XHRResponseURL.js
@@ -45,38 +45,16 @@ function info(aMessage) {
   var obj = {
     type: "info",
     message: aMessage
   };
   ++message.ping;
   message(obj);
 }
 
-function todo(aBool, aMessage) {
-  var obj = {
-    type: "todo",
-    bool: aBool,
-    message: aMessage
-  };
-  ++message.ping;
-  message(obj);
-}
-
-function todo_is(aActual, aExpected, aMessage) {
-  var obj = {
-    type: "todo_is",
-    actual: aActual,
-    expected: aExpected,
-    message: aMessage
-  };
-  ++message.ping;
-  message(obj);
-}
-
-
 function request(aURL) {
   return new Promise(function (aResolve, aReject) {
     var xhr = new XMLHttpRequest();
     xhr.open("GET", aURL);
     xhr.addEventListener("load", function () {
       xhr.succeeded = true;
       aResolve(xhr);
     });
@@ -120,25 +98,21 @@ function testSuccessResponse() {
       message: "request to same-origin redirects several times and finally go to same-origin URL",
       requestURL: "http://mochi.test:8888/tests/dom/base/test/file_XHRResponseURL.sjs?url=" + encodeURIComponent("http://mochi.test:8888/tests/dom/base/test/file_XHRResponseURL.sjs?url=http://mochi.test:8888/tests/dom/base/test/file_XHRResponseURL.text"),
       responseURL: "http://mochi.test:8888/tests/dom/base/test/file_XHRResponseURL.text"
     },
     {
       message: "request to same-origin redirect to cross-origin URL",
       requestURL: "http://mochi.test:8888/tests/dom/base/test/file_XHRResponseURL.sjs?url=http://example.com/tests/dom/base/test/file_XHRResponseURL.text",
       responseURL: "http://example.com/tests/dom/base/test/file_XHRResponseURL.text",
-      skip: isInWorker(),
-      reason: "cross-origin redirect request not works on Workers, see bug 882458"
     },
     {
       message: "request to same-origin redirects several times and finally go to cross-origin URL",
       requestURL: "http://mochi.test:8888/tests/dom/base/test/file_XHRResponseURL.sjs?url=" + encodeURIComponent("http://mochi.test:8888/tests/dom/base/test/file_XHRResponseURL.sjs?url=http://example.com/tests/dom/base/test/file_XHRResponseURL.text"),
       responseURL: "http://example.com/tests/dom/base/test/file_XHRResponseURL.text",
-      skip: isInWorker(),
-      reason: "cross-origin redirect request not works on Workers, see bug 882458"
     },
 
     // tests that start with cross-origin request
     {
       message: "request to cross-origin without redirect",
       requestURL: "http://example.com/tests/dom/base/test/file_XHRResponseURL.text",
       responseURL: "http://example.com/tests/dom/base/test/file_XHRResponseURL.text"
     },
@@ -146,53 +120,41 @@ function testSuccessResponse() {
       message: "request to cross-origin redirect back to same-origin URL",
       requestURL: "http://example.com/tests/dom/base/test/file_XHRResponseURL.sjs?url=http://mochi.test:8888/tests/dom/base/test/file_XHRResponseURL.text",
       responseURL: "http://mochi.test:8888/tests/dom/base/test/file_XHRResponseURL.text"
     },
     {
       message: "request to cross-origin redirect to the same cross-origin URL",
       requestURL: "http://example.com/tests/dom/base/test/file_XHRResponseURL.sjs?url=http://example.com/tests/dom/base/test/file_XHRResponseURL.text",
       responseURL: "http://example.com/tests/dom/base/test/file_XHRResponseURL.text",
-      skip: isInWorker(),
-      reason: "cross-origin redirect request not works on Workers, see bug 882458"
     },
     {
       message: "request to cross-origin redirect to another cross-origin URL",
       requestURL: "http://example.com/tests/dom/base/test/file_XHRResponseURL.sjs?url=http://example.org/tests/dom/base/test/file_XHRResponseURL.text",
       responseURL: "http://example.org/tests/dom/base/test/file_XHRResponseURL.text",
-      skip: isInWorker(),
-      reason: "cross-origin redirect request not works on Workers, see bug 882458"
     },
     {
       message: "request to cross-origin redirects several times and finally go to same-origin URL",
       requestURL: "http://example.com/tests/dom/base/test/file_XHRResponseURL.sjs?url=" + encodeURIComponent("http://example.com/tests/dom/base/test/file_XHRResponseURL.sjs?url=http://mochi.test:8888/tests/dom/base/test/file_XHRResponseURL.text"),
       responseURL: "http://mochi.test:8888/tests/dom/base/test/file_XHRResponseURL.text",
-      skip: isInWorker(),
-      reason: "cross-origin redirect request not works on Workers, see bug 882458"
     },
     {
       message: "request to cross-origin redirects several times and finally go to the same cross-origin URL",
       requestURL: "http://example.com/tests/dom/base/test/file_XHRResponseURL.sjs?url=" + encodeURIComponent("http://example.com/tests/dom/base/test/file_XHRResponseURL.sjs?url=http://example.com/tests/dom/base/test/file_XHRResponseURL.text"),
       responseURL: "http://example.com/tests/dom/base/test/file_XHRResponseURL.text",
-      skip: isInWorker(),
-      reason: "cross-origin redirect request not works on Workers, see bug 882458"
     },
     {
       message: "request to cross-origin redirects several times and finally go to another cross-origin URL",
       requestURL: "http://example.com/tests/dom/base/test/file_XHRResponseURL.sjs?url=" + encodeURIComponent("http://example.com/tests/dom/base/test/file_XHRResponseURL.sjs?url=http://example.org/tests/dom/base/test/file_XHRResponseURL.text"),
       responseURL: "http://example.org/tests/dom/base/test/file_XHRResponseURL.text",
-      skip: isInWorker(),
-      reason: "cross-origin redirect request not works on Workers, see bug 882458"
     },
     {
       message: "request to cross-origin redirects to another cross-origin and finally go to the other cross-origin URL",
       requestURL: "http://example.com/tests/dom/base/test/file_XHRResponseURL.sjs?url=" + encodeURIComponent("http://example.org/tests/dom/base/test/file_XHRResponseURL.sjs?url=http://test1.example.com/tests/dom/base/test/file_XHRResponseURL.text"),
       responseURL: "http://test1.example.com/tests/dom/base/test/file_XHRResponseURL.text",
-      skip: isInWorker(),
-      reason: "cross-origin redirect request not works on Workers, see bug 882458"
     },
     {
       message: "request URL has fragment",
       requestURL: "http://mochi.test:8888/tests/dom/base/test/file_XHRResponseURL.text#fragment",
       responseURL: "http://mochi.test:8888/tests/dom/base/test/file_XHRResponseURL.text"
     },
 
     // tests for non-http(s) URL
@@ -204,23 +166,18 @@ function testSuccessResponse() {
     {
       message: "request to blob: URL",
       requestURL: blobURL,
       responseURL: blobURL
     }
   ];
 
   var sequence = createSequentialRequest(parameters, function (aXHR, aParam) {
-    if (aParam.skip) {
-      todo(aXHR.succeeded, aParam.reason);
-      todo_is(aXHR.responseURL, aParam.responseURL, aParam.reason);
-    } else {
-      ok(aXHR.succeeded, "assert request succeeded");
-      is(aXHR.responseURL, aParam.responseURL, aParam.message);
-    }
+    ok(aXHR.succeeded, "assert request succeeded");
+    is(aXHR.responseURL, aParam.responseURL, aParam.message);
   });
 
   sequence.then(function () {
     URL.revokeObjectURL(blobURL);
   });
 
   return sequence;
 }
--- a/dom/security/nsContentSecurityManager.cpp
+++ b/dom/security/nsContentSecurityManager.cpp
@@ -167,19 +167,23 @@ DoContentSecurityChecks(nsIURI* aURI, ns
 
     case nsIContentPolicy::TYPE_XMLHTTPREQUEST: {
       // alias nsIContentPolicy::TYPE_DATAREQUEST:
       requestingContext = aLoadInfo->LoadingNode();
       MOZ_ASSERT(!requestingContext ||
                  requestingContext->NodeType() == nsIDOMNode::DOCUMENT_NODE,
                  "type_xml requires requestingContext of type Document");
 
+      // We're checking for the external TYPE_XMLHTTPREQUEST here in case
+      // an addon creates a request with that type.
       if (internalContentPolicyType ==
-            nsIContentPolicy::TYPE_INTERNAL_XMLHTTPREQUEST) {
-        mimeTypeGuess = NS_LITERAL_CSTRING("application/xml");
+            nsIContentPolicy::TYPE_INTERNAL_XMLHTTPREQUEST ||
+          internalContentPolicyType ==
+            nsIContentPolicy::TYPE_XMLHTTPREQUEST) {
+        mimeTypeGuess = EmptyCString();
       }
       else {
         MOZ_ASSERT(internalContentPolicyType ==
                    nsIContentPolicy::TYPE_INTERNAL_EVENTSOURCE,
                    "can not set mime type guess for unexpected internal type");
         mimeTypeGuess = NS_LITERAL_CSTRING(TEXT_EVENT_STREAM);
       }
       break;
--- a/dom/security/test/mixedcontentblocker/file_main.html
+++ b/dom/security/test/mixedcontentblocker/file_main.html
@@ -150,39 +150,30 @@ https://bugzilla.mozilla.org/show_bug.cg
   }
   iframe.onerror = function() {
     parent.postMessage({"test": "iframe", "msg": "insecure iframe blocked"}, "http://mochi.test:8888");
   };
   testContent.appendChild(iframe);
 
 
   // Test 1e: insecure xhr
-  var xhrsuccess = true;
   var xhr = new XMLHttpRequest;
   try {
     xhr.open("GET", baseUrl + "?type=xhr", true);
-  } catch(ex) {
-     xhrsuccess = false;
-     parent.postMessage({"test": "xhr", "msg": "insecure xhr blocked"}, "http://mochi.test:8888");
-  }
-
-  if(xhrsuccess) {
-    xhr.onreadystatechange = function (oEvent) {
-      var result = false;
-      if (xhr.readyState == 4) {
-        if (xhr.status == 200) {
-          parent.postMessage({"test": "xhr", "msg": "insecure xhr loaded"}, "http://mochi.test:8888");
-        }
-        else {
-          parent.postMessage({"test": "xhr", "msg": "insecure xhr blocked"}, "http://mochi.test:8888");
-        }
+    xhr.send();
+    xhr.onloadend = function (oEvent) {
+      if (xhr.status == 200) {
+        parent.postMessage({"test": "xhr", "msg": "insecure xhr loaded"}, "http://mochi.test:8888");
+      }
+      else {
+        parent.postMessage({"test": "xhr", "msg": "insecure xhr blocked"}, "http://mochi.test:8888");
       }
     }
-
-    xhr.send(null);
+  } catch(ex) {
+     parent.postMessage({"test": "xhr", "msg": "insecure xhr blocked"}, "http://mochi.test:8888");
   }
 
   /* Part 2: Mixed Display tests */
 
   // Shorthand for all image test variants
   function imgHandlers(img, test) {
     img.onload = function () {
       parent.postMessage({"test": test, "msg": "insecure image loaded"}, "http://mochi.test:8888");
--- a/dom/workers/test/test_xhr_responseURL.html
+++ b/dom/workers/test/test_xhr_responseURL.html
@@ -40,26 +40,16 @@ worker.addEventListener("message", funct
     worker.postMessage("pong");
     return;
   }
   if (data.type == "info") {
     SimpleTest.info(data.message);
     worker.postMessage("pong");
     return;
   }
-  if (data.type === "todo") {
-    SimpleTest.todo(data.bool, data.message);
-    worker.postMessage("pong");
-    return;
-  }
-  if (data.type === "todo_is") {
-    SimpleTest.todo_is(data.actual, data.expected, data.message);
-    worker.postMessage("pong");
-    return;
-  }
   if (data.type === "redirect_test") {
     if (data.status === "start") {
       SpecialPowers.addObserver(requestObserver, "specialpowers-http-notify-request", false);
       return;
     }
     if (data.status === "end") {
       SpecialPowers.removeObserver(requestObserver, "specialpowers-http-notify-request");
       return;
--- a/netwerk/base/LoadInfo.cpp
+++ b/netwerk/base/LoadInfo.cpp
@@ -185,16 +185,24 @@ LoadInfo::LoadingNode()
 
 NS_IMETHODIMP
 LoadInfo::GetSecurityFlags(nsSecurityFlags* aResult)
 {
   *aResult = mSecurityFlags;
   return NS_OK;
 }
 
+void
+LoadInfo::SetWithCredentialsSecFlag()
+{
+  MOZ_ASSERT(!mEnforceSecurity,
+             "Request should not have been opened yet");
+  mSecurityFlags |= nsILoadInfo::SEC_REQUIRE_CORS_WITH_CREDENTIALS;
+}
+
 NS_IMETHODIMP
 LoadInfo::GetSecurityMode(uint32_t *aFlags)
 {
   *aFlags = (mSecurityFlags &
               (nsILoadInfo::SEC_REQUIRE_SAME_ORIGIN_DATA_INHERITS |
                nsILoadInfo::SEC_REQUIRE_SAME_ORIGIN_DATA_IS_BLOCKED |
                nsILoadInfo::SEC_ALLOW_CROSS_ORIGIN_DATA_INHERITS |
                nsILoadInfo::SEC_ALLOW_CROSS_ORIGIN_DATA_IS_NULL |
--- a/netwerk/base/LoadInfo.h
+++ b/netwerk/base/LoadInfo.h
@@ -10,16 +10,17 @@
 #include "nsIContentPolicy.h"
 #include "nsILoadInfo.h"
 #include "nsIPrincipal.h"
 #include "nsIWeakReferenceUtils.h" // for nsWeakPtr
 #include "nsIURI.h"
 #include "nsTArray.h"
 
 class nsINode;
+class nsXMLHttpRequest;
 
 namespace mozilla {
 
 namespace net {
 class OptionalLoadInfoArgs;
 } // namespace net
 
 namespace ipc {
@@ -72,16 +73,22 @@ private:
 
   friend nsresult
   mozilla::ipc::LoadInfoArgsToLoadInfo(
     const mozilla::net::OptionalLoadInfoArgs& aLoadInfoArgs,
     nsILoadInfo** outLoadInfo);
 
   ~LoadInfo();
 
+  // This function is the *only* function which can change the securityflags
+  // of a loadinfo. It only exists because of the XHR code. Don't call it
+  // from anywhere else!
+  void SetWithCredentialsSecFlag();
+  friend class ::nsXMLHttpRequest;
+
   nsCOMPtr<nsIPrincipal>           mLoadingPrincipal;
   nsCOMPtr<nsIPrincipal>           mTriggeringPrincipal;
   nsWeakPtr                        mLoadingContext;
   nsSecurityFlags                  mSecurityFlags;
   nsContentPolicyType              mInternalContentPolicyType;
   bool                             mUpgradeInsecureRequests;
   uint64_t                         mInnerWindowID;
   uint64_t                         mOuterWindowID;
--- a/netwerk/protocol/http/nsCORSListenerProxy.cpp
+++ b/netwerk/protocol/http/nsCORSListenerProxy.cpp
@@ -409,18 +409,17 @@ nsPreflightCache::GetCacheKey(nsIURI* aU
   return true;
 }
 
 //////////////////////////////////////////////////////////////////////////
 // nsCORSListenerProxy
 
 NS_IMPL_ISUPPORTS(nsCORSListenerProxy, nsIStreamListener,
                   nsIRequestObserver, nsIChannelEventSink,
-                  nsIInterfaceRequestor, nsIAsyncVerifyRedirectCallback,
-                  nsIThreadRetargetableStreamListener)
+                  nsIInterfaceRequestor, nsIThreadRetargetableStreamListener)
 
 /* static */
 void
 nsCORSListenerProxy::Startup()
 {
   Preferences::AddBoolVarCache(&gDisableCORS,
                                "content.cors.disable");
   Preferences::AddBoolVarCache(&gDisableCORSPrivateData,
@@ -587,19 +586,16 @@ NS_IMETHODIMP
 nsCORSListenerProxy::OnStopRequest(nsIRequest* aRequest,
                                    nsISupports* aContext,
                                    nsresult aStatusCode)
 {
   MOZ_ASSERT(mInited, "nsCORSListenerProxy has not been initialized properly");
   nsresult rv = mOuterListener->OnStopRequest(aRequest, aContext, aStatusCode);
   mOuterListener = nullptr;
   mOuterNotificationCallbacks = nullptr;
-  mRedirectCallback = nullptr;
-  mOldRedirectChannel = nullptr;
-  mNewRedirectChannel = nullptr;
   return rv;
 }
 
 NS_IMETHODIMP
 nsCORSListenerProxy::OnDataAvailable(nsIRequest* aRequest,
                                      nsISupports* aContext,
                                      nsIInputStream* aInputStream,
                                      uint64_t aOffset,
@@ -645,17 +641,17 @@ nsCORSListenerProxy::GetInterface(const 
     mOuterNotificationCallbacks->GetInterface(aIID, aResult) :
     NS_ERROR_NO_INTERFACE;
 }
 
 NS_IMETHODIMP
 nsCORSListenerProxy::AsyncOnChannelRedirect(nsIChannel *aOldChannel,
                                             nsIChannel *aNewChannel,
                                             uint32_t aFlags,
-                                            nsIAsyncVerifyRedirectCallback *cb)
+                                            nsIAsyncVerifyRedirectCallback *aCb)
 {
   nsresult rv;
   if (!NS_IsInternalSameURIRedirect(aOldChannel, aNewChannel, aFlags) &&
       !NS_IsHSTSUpgradeRedirect(aOldChannel, aNewChannel, aFlags)) {
     rv = CheckRequestApproved(aOldChannel);
     if (NS_FAILED(rv)) {
       nsCOMPtr<nsIURI> oldURI;
       NS_GetFinalChannelURI(aOldChannel, getter_AddRefs(oldURI));
@@ -709,64 +705,34 @@ nsCORSListenerProxy::AsyncOnChannelRedir
         }
       }
 
       if (NS_FAILED(rv)) {
         aOldChannel->Cancel(rv);
         return rv;
       }
     }
+
+    rv = UpdateChannel(aNewChannel, DataURIHandling::Disallow);
+    if (NS_FAILED(rv)) {
+        NS_WARNING("nsCORSListenerProxy::AsyncOnChannelRedirect: "
+                   "UpdateChannel() returned failure");
+      aOldChannel->Cancel(rv);
+      return rv;
+    }
   }
 
-  // Prepare to receive callback
-  mRedirectCallback = cb;
-  mOldRedirectChannel = aOldChannel;
-  mNewRedirectChannel = aNewChannel;
-
   nsCOMPtr<nsIChannelEventSink> outer =
     do_GetInterface(mOuterNotificationCallbacks);
   if (outer) {
-    rv = outer->AsyncOnChannelRedirect(aOldChannel, aNewChannel, aFlags, this);
-    if (NS_FAILED(rv)) {
-        aOldChannel->Cancel(rv); // is this necessary...?
-        mRedirectCallback = nullptr;
-        mOldRedirectChannel = nullptr;
-        mNewRedirectChannel = nullptr;
-    }
-    return rv;  
+    return outer->AsyncOnChannelRedirect(aOldChannel, aNewChannel, aFlags, aCb);
   }
 
-  (void) OnRedirectVerifyCallback(NS_OK);
-  return NS_OK;
-}
-
-NS_IMETHODIMP
-nsCORSListenerProxy::OnRedirectVerifyCallback(nsresult result)
-{
-  NS_ASSERTION(mRedirectCallback, "mRedirectCallback not set in callback");
-  NS_ASSERTION(mOldRedirectChannel, "mOldRedirectChannel not set in callback");
-  NS_ASSERTION(mNewRedirectChannel, "mNewRedirectChannel not set in callback");
+  aCb->OnRedirectVerifyCallback(NS_OK);
 
-  if (NS_SUCCEEDED(result)) {
-    nsresult rv = UpdateChannel(mNewRedirectChannel, DataURIHandling::Disallow);
-      if (NS_FAILED(rv)) {
-          NS_WARNING("nsCORSListenerProxy::OnRedirectVerifyCallback: "
-                     "UpdateChannel() returned failure");
-      }
-      result = rv;
-  }
-
-  if (NS_FAILED(result)) {
-    mOldRedirectChannel->Cancel(result);
-  }
-
-  mOldRedirectChannel = nullptr;
-  mNewRedirectChannel = nullptr;
-  mRedirectCallback->OnRedirectVerifyCallback(result);
-  mRedirectCallback   = nullptr;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsCORSListenerProxy::CheckListenerChain()
 {
   MOZ_ASSERT(NS_IsMainThread());
 
--- a/netwerk/protocol/http/nsCORSListenerProxy.h
+++ b/netwerk/protocol/http/nsCORSListenerProxy.h
@@ -35,30 +35,28 @@ enum class DataURIHandling
 {
   Allow,
   Disallow
 };
 
 class nsCORSListenerProxy final : public nsIStreamListener,
                                   public nsIInterfaceRequestor,
                                   public nsIChannelEventSink,
-                                  public nsIAsyncVerifyRedirectCallback,
                                   public nsIThreadRetargetableStreamListener
 {
 public:
   nsCORSListenerProxy(nsIStreamListener* aOuter,
                       nsIPrincipal* aRequestingPrincipal,
                       bool aWithCredentials);
 
   NS_DECL_ISUPPORTS
   NS_DECL_NSIREQUESTOBSERVER
   NS_DECL_NSISTREAMLISTENER
   NS_DECL_NSIINTERFACEREQUESTOR
   NS_DECL_NSICHANNELEVENTSINK
-  NS_DECL_NSIASYNCVERIFYREDIRECTCALLBACK
   NS_DECL_NSITHREADRETARGETABLESTREAMLISTENER
 
   // Must be called at startup.
   static void Startup();
 
   static void Shutdown();
 
   nsresult Init(nsIChannel* aChannel, DataURIHandling aAllowDataURI);
@@ -98,14 +96,11 @@ private:
   // Please note that the member variable mHasBeenCrossSite may rely on the
   // promise that the CSP directive 'upgrade-insecure-requests' upgrades
   // an http: request to https: in nsHttpChannel::Connect() and hence
   // a request might not be marked as cross site request based on that promise.
   bool mHasBeenCrossSite;
 #ifdef DEBUG
   bool mInited;
 #endif
-  nsCOMPtr<nsIAsyncVerifyRedirectCallback> mRedirectCallback;
-  nsCOMPtr<nsIChannel> mOldRedirectChannel;
-  nsCOMPtr<nsIChannel> mNewRedirectChannel;
 };
 
 #endif
deleted file mode 100644
--- a/testing/web-platform/meta/content-security-policy/blink-contrib/shared-worker-connect-src-allowed.sub.html.ini
+++ /dev/null
@@ -1,5 +0,0 @@
-[shared-worker-connect-src-allowed.sub.html]
-  type: testharness
-  [Expecting alerts: ["xhr allowed","TEST COMPLETE"\]]
-    expected: FAIL
-
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/meta/content-security-policy/blink-contrib/shared-worker-connect-src-blocked.sub.html.ini
@@ -0,0 +1,4 @@
+[shared-worker-connect-src-blocked.sub.html]
+  type: testharness
+  [Expecting alerts: ["xhr blocked","TEST COMPLETE"\]]
+    expected: FAIL
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/meta/content-security-policy/blink-contrib/worker-connect-src-blocked.sub.html.ini
@@ -0,0 +1,4 @@
+[worker-connect-src-blocked.sub.html]
+  type: testharness
+  [Expecting alerts: ["xhr blocked","TEST COMPLETE"\]]
+    expected: FAIL
deleted file mode 100644
--- a/testing/web-platform/meta/cors/redirect-userinfo.htm.ini
+++ /dev/null
@@ -1,12 +0,0 @@
-[redirect-userinfo.htm]
-  type: testharness
-  expected: TIMEOUT
-  [Disallow redirect with userinfo (//user:pass@)]
-    expected: TIMEOUT
-
-  [Disallow redirect with userinfo (//user:@)]
-    expected: TIMEOUT
-
-  [Disallow redirect with userinfo (//user@)]
-    expected: TIMEOUT
-
--- a/testing/web-platform/tests/content-security-policy/blink-contrib/connect-src-xmlhttprequest-blocked.sub.html
+++ b/testing/web-platform/tests/content-security-policy/blink-contrib/connect-src-xmlhttprequest-blocked.sub.html
@@ -13,17 +13,23 @@ connect-src 'self'; script-src 'self' 'u
 -->
 </head>
 
 <body>
     <script>
         try {
             var xhr = new XMLHttpRequest;
             xhr.open("GET", "http://www1.{{host}}:{{ports[http][0]}}/content-security-policy/support/fail.png", true);
-            log("Fail");
+            xhr.send();
+            xhr.onload = function() {
+                log("Fail");
+            }
+            xhr.onerror = function() {
+                log("Pass");
+            }
         } catch (e) {
             log("Pass");
         }
 
     </script>
     <div id="log"></div>
     <script async defer src="../support/checkReport.sub.js?reportExists=true&amp;reportField=violated-directive&amp;reportValue=connect-src%20&apos;self&apos;"></script>
 </body>