Bug 1144249 - fix fetch no-cors mode. r=bkelly
authorNikhil Marathe <nsm.nikhil@gmail.com>
Tue, 17 Mar 2015 08:47:04 -0700
changeset 234166 ca19388684ac
parent 234165 5fb84103d54c
child 234167 d3380179efc1
push id57055
push usernsm.nikhil@gmail.com
push date2015-03-18 03:39 +0000
treeherdermozilla-inbound@ca19388684ac [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbkelly
bugs1144249
milestone39.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 1144249 - fix fetch no-cors mode. r=bkelly A no-cors mode request should not go through a nsCORSProxyListener, which can only handle same-origin and cors mode. The test for no-cors was passing allowOrigin, which was converting it into a CORS request, which is why it was passing. I also realized that it is port 8888 and not 8000, so fixed that in some of the redirect tests.
dom/fetch/FetchDriver.cpp
dom/tests/mochitest/fetch/test_fetch_cors.js
--- a/dom/fetch/FetchDriver.cpp
+++ b/dom/fetch/FetchDriver.cpp
@@ -480,44 +480,52 @@ FetchDriver::HttpFetch(bool aCORSFlag, b
   // Set skip serviceworker flag.
   // While the spec also gates on the client being a ServiceWorker, we can't
   // infer that here. Instead we rely on callers to set the flag correctly.
   if (mRequest->SkipServiceWorker()) {
     nsCOMPtr<nsIHttpChannelInternal> internalChan = do_QueryInterface(httpChan);
     internalChan->ForceNoIntercept();
   }
 
-  // Set up a CORS proxy that will handle the various requirements of the CORS
-  // protocol. It handles the preflight cache and CORS response headers.
-  // If the request is allowed, it will start our original request
-  // and our observer will be notified. On failure, our observer is notified
-  // directly.
-  nsRefPtr<nsCORSListenerProxy> corsListener =
-    new nsCORSListenerProxy(this, mPrincipal, useCredentials);
-  rv = corsListener->Init(chan, true /* allow data uri */);
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    return FailWithNetworkError();
+  nsCOMPtr<nsIStreamListener> listener = this;
+
+  // Unless the cors mode is explicitly no-cors, we set up a cors proxy even in
+  // the same-origin case, since the proxy does not enforce cors header checks
+  // in the same-origin case.
+  if (mRequest->Mode() != RequestMode::No_cors) {
+    // Set up a CORS proxy that will handle the various requirements of the CORS
+    // protocol. It handles the preflight cache and CORS response headers.
+    // If the request is allowed, it will start our original request
+    // and our observer will be notified. On failure, our observer is notified
+    // directly.
+    nsRefPtr<nsCORSListenerProxy> corsListener =
+      new nsCORSListenerProxy(this, mPrincipal, useCredentials);
+    rv = corsListener->Init(chan, true /* allow data uri */);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return FailWithNetworkError();
+    }
+    listener = corsListener.forget();
   }
 
   // If preflight is required, start a "CORS preflight fetch"
   // https://fetch.spec.whatwg.org/#cors-preflight-fetch-0. All the
   // implementation is handled by NS_StartCORSPreflight, we just set up the
   // unsafeHeaders so they can be verified against the response's
   // "Access-Control-Allow-Headers" header.
   if (aCORSPreflightFlag) {
     nsCOMPtr<nsIChannel> preflightChannel;
     nsAutoTArray<nsCString, 5> unsafeHeaders;
     mRequest->Headers()->GetUnsafeHeaders(unsafeHeaders);
 
-    rv = NS_StartCORSPreflight(chan, corsListener, mPrincipal,
+    rv = NS_StartCORSPreflight(chan, listener, mPrincipal,
                                useCredentials,
                                unsafeHeaders,
                                getter_AddRefs(preflightChannel));
   } else {
-   rv = chan->AsyncOpen(corsListener, nullptr);
+   rv = chan->AsyncOpen(listener, nullptr);
   }
 
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return FailWithNetworkError();
   }
 
   // Step 4 onwards of "HTTP Fetch" is handled internally by Necko.
   return NS_OK;
@@ -770,17 +778,17 @@ FetchDriver::AsyncOnChannelRedirect(nsIC
   // The only thing we do is to check if the request requires a preflight (part
   // of step 4.9), in which case we abort. This part cannot be done by
   // nsCORSListenerProxy since it does not have access to mRequest.
   // which case. Step 4.10.3 is handled by OnRedirectVerifyCallback(), and all
   // the other steps are handled by nsCORSListenerProxy.
   if (!NS_IsInternalSameURIRedirect(aOldChannel, aNewChannel, aFlags)) {
     rv = DoesNotRequirePreflight(aNewChannel);
     if (NS_FAILED(rv)) {
-      NS_WARNING("nsXMLHttpRequest::OnChannelRedirect: "
+      NS_WARNING("FetchDriver::OnChannelRedirect: "
                  "DoesNotRequirePreflight returned failure");
       return rv;
     }
   }
 
   mRedirectCallback = aCallback;
   mOldRedirectChannel = aOldChannel;
   mNewRedirectChannel = aNewChannel;
--- a/dom/tests/mochitest/fetch/test_fetch_cors.js
+++ b/dom/tests/mochitest/fetch/test_fetch_cors.js
@@ -41,19 +41,21 @@ function testNoCorsCtor() {
   r.headers.append("DNT", "value");
   ok(!r.headers.has("DNT"), "Appending forbidden header should fail");
 }
 
 var corsServerPath = "/tests/dom/base/test/file_CrossSiteXHR_server.sjs?";
 function testModeNoCors() {
   // Fetch spec, section 4, step 4, response tainting should be set opaque, so
   // that fetching leads to an opaque filtered response in step 8.
-  var r = new Request("http://example.com" + corsServerPath + "status=200&allowOrigin=*", { mode: "no-cors" });
+  var r = new Request("http://example.com" + corsServerPath + "status=200", { mode: "no-cors" });
   return fetch(r).then(function(res) {
     ok(isOpaqueResponse(res), "no-cors Request fetch should result in opaque response");
+  }, function(e) {
+    ok(false, "no-cors Request fetch should not error");
   });
 }
 
 function testSameOriginCredentials() {
   var cookieStr = "type=chocolatechip";
   var tests = [
               {
                 // Initialize by setting a cookie.
@@ -1014,33 +1016,33 @@ function testRedirects() {
                     },
                     ],
            },
            { pass: 0,
              method: "GET",
              hops: [{ server: "http://example.com",
                       allowOrigin: origin
                     },
-                    { server: "http://test2.mochi.test:8000",
+                    { server: "http://test2.mochi.test:8888",
                       allowOrigin: origin
                     },
                     { server: "http://sub2.xn--lt-uia.mochi.test:8888",
                       allowOrigin: origin
                     },
                     { server: "http://sub1.test1.mochi.test:8888",
                       allowOrigin: origin
                     },
                     ],
            },
            { pass: 0,
              method: "GET",
              hops: [{ server: "http://example.com",
                       allowOrigin: origin
                     },
-                    { server: "http://test2.mochi.test:8000",
+                    { server: "http://test2.mochi.test:8888",
                       allowOrigin: origin
                     },
                     { server: "http://sub2.xn--lt-uia.mochi.test:8888",
                       allowOrigin: "*"
                     },
                     { server: "http://sub1.test1.mochi.test:8888",
                       allowOrigin: "*"
                     },
@@ -1062,49 +1064,49 @@ function testRedirects() {
                     },
                     ],
            },
            { pass: 0,
              method: "GET",
              hops: [{ server: "http://example.com",
                       allowOrigin: origin
                     },
-                    { server: "http://test2.mochi.test:8000",
+                    { server: "http://test2.mochi.test:8888",
                       allowOrigin: origin
                     },
                     { server: "http://sub2.xn--lt-uia.mochi.test:8888",
                       allowOrigin: "x"
                     },
                     { server: "http://sub1.test1.mochi.test:8888",
                       allowOrigin: origin
                     },
                     ],
            },
            { pass: 0,
              method: "GET",
              hops: [{ server: "http://example.com",
                       allowOrigin: origin
                     },
-                    { server: "http://test2.mochi.test:8000",
+                    { server: "http://test2.mochi.test:8888",
                       allowOrigin: origin
                     },
                     { server: "http://sub2.xn--lt-uia.mochi.test:8888",
                       allowOrigin: "*"
                     },
                     { server: "http://sub1.test1.mochi.test:8888",
                       allowOrigin: origin
                     },
                     ],
            },
            { pass: 0,
              method: "GET",
              hops: [{ server: "http://example.com",
                       allowOrigin: origin
                     },
-                    { server: "http://test2.mochi.test:8000",
+                    { server: "http://test2.mochi.test:8888",
                       allowOrigin: origin
                     },
                     { server: "http://sub2.xn--lt-uia.mochi.test:8888",
                       allowOrigin: "*"
                     },
                     { server: "http://sub1.test1.mochi.test:8888",
                     },
                     ],