Bug 1209095 - Accept opaqueredirection fetch results if the request redirection type is manual. r=bkelly
authorJosh Matthews <josh@joshmatthews.net>
Wed, 16 Dec 2015 19:06:54 -0500
changeset 332551 b068d21a21087b21042d106a9fab94cab3f3bcf0
parent 332550 5680a55b2ec1293dc2d9b5f8568a345d3d809d7b
child 332552 72235ad4fc17dead99e9ff021c1860c5e54ceffd
push id6048
push userkmoir@mozilla.com
push dateMon, 06 Jun 2016 19:02:08 +0000
treeherdermozilla-beta@46d72a56c57d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbkelly
bugs1209095
milestone48.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 1209095 - Accept opaqueredirection fetch results if the request redirection type is manual. r=bkelly
dom/workers/ServiceWorkerEvents.cpp
testing/web-platform/tests/service-workers/service-worker/fetch-event-redirect.https.html
testing/web-platform/tests/service-workers/service-worker/resources/fetch-event-redirect-iframe.html
--- a/dom/workers/ServiceWorkerEvents.cpp
+++ b/dom/workers/ServiceWorkerEvents.cpp
@@ -262,43 +262,43 @@ public:
     return decision == nsIContentPolicy::ACCEPT;
   }
 };
 
 class RespondWithHandler final : public PromiseNativeHandler
 {
   nsMainThreadPtrHandle<nsIInterceptedChannel> mInterceptedChannel;
   const RequestMode mRequestMode;
+  const RequestRedirect mRequestRedirectMode;
 #ifdef DEBUG
   const bool mIsClientRequest;
 #endif
-  const bool mIsNavigationRequest;
   const nsCString mScriptSpec;
   const nsString mRequestURL;
   const nsCString mRespondWithScriptSpec;
   const uint32_t mRespondWithLineNumber;
   const uint32_t mRespondWithColumnNumber;
   bool mRequestWasHandled;
 public:
   NS_DECL_ISUPPORTS
 
   RespondWithHandler(nsMainThreadPtrHandle<nsIInterceptedChannel>& aChannel,
                      RequestMode aRequestMode, bool aIsClientRequest,
-                     bool aIsNavigationRequest,
+                     RequestRedirect aRedirectMode,
                      const nsACString& aScriptSpec,
                      const nsAString& aRequestURL,
                      const nsACString& aRespondWithScriptSpec,
                      uint32_t aRespondWithLineNumber,
                      uint32_t aRespondWithColumnNumber)
     : mInterceptedChannel(aChannel)
     , mRequestMode(aRequestMode)
+    , mRequestRedirectMode(aRedirectMode)
 #ifdef DEBUG
     , mIsClientRequest(aIsClientRequest)
 #endif
-    , mIsNavigationRequest(aIsNavigationRequest)
     , mScriptSpec(aScriptSpec)
     , mRequestURL(aRequestURL)
     , mRespondWithScriptSpec(aRespondWithScriptSpec)
     , mRespondWithLineNumber(aRespondWithLineNumber)
     , mRespondWithColumnNumber(aRespondWithColumnNumber)
     , mRequestWasHandled(false)
   {
   }
@@ -564,21 +564,21 @@ RespondWithHandler::ResolvedCallback(JSC
                                            mRequestURL, valueString);
     return;
   }
 
   WorkerPrivate* worker = GetCurrentThreadWorkerPrivate();
   MOZ_ASSERT(worker);
   worker->AssertIsOnWorkerThread();
 
-  // Section "HTTP Fetch", step 2.2:
+  // Section "HTTP Fetch", step 3.3:
   //  If one of the following conditions is true, return a network error:
   //    * response's type is "error".
   //    * request's mode is not "no-cors" and response's type is "opaque".
-  //    * request is not a navigation request and response's type is
+  //    * request's redirect mode is not "manual" and response's type is
   //      "opaqueredirect".
 
   if (response->Type() == ResponseType::Error) {
     autoCancel.SetCancelMessage(
       NS_LITERAL_CSTRING("InterceptedErrorResponseWithURL"), mRequestURL);
     return;
   }
 
@@ -591,17 +591,18 @@ RespondWithHandler::ResolvedCallback(JSC
                                       RequestModeValues::strings[mode].length);
 
     autoCancel.SetCancelMessage(
       NS_LITERAL_CSTRING("BadOpaqueInterceptionRequestModeWithURL"),
       mRequestURL, modeString);
     return;
   }
 
-  if (!mIsNavigationRequest && response->Type() == ResponseType::Opaqueredirect) {
+  if (mRequestRedirectMode != RequestRedirect::Manual &&
+      response->Type() == ResponseType::Opaqueredirect) {
     autoCancel.SetCancelMessage(
       NS_LITERAL_CSTRING("BadOpaqueRedirectInterceptionWithURL"), mRequestURL);
     return;
   }
 
   if (NS_WARN_IF(response->BodyUsed())) {
     autoCancel.SetCancelMessage(
       NS_LITERAL_CSTRING("InterceptedUsedResponseWithURL"), mRequestURL);
@@ -733,17 +734,17 @@ FetchEvent::RespondWith(JSContext* aCx, 
 
   nsAutoCString requestURL;
   ir->GetURL(requestURL);
 
   StopImmediatePropagation();
   mWaitToRespond = true;
   RefPtr<RespondWithHandler> handler =
     new RespondWithHandler(mChannel, mRequest->Mode(), ir->IsClientRequest(),
-                           ir->IsNavigationRequest(), mScriptSpec,
+                           mRequest->Redirect(), mScriptSpec,
                            NS_ConvertUTF8toUTF16(requestURL),
                            spec, line, column);
   aArg.AppendNativeHandler(handler);
 
   // Append directly to the lifecycle promises array.  Don't call WaitUntil()
   // because that will lead to double-reporting any errors.
   mPromises.AppendElement(&aArg);
 }
--- a/testing/web-platform/tests/service-workers/service-worker/fetch-event-redirect.https.html
+++ b/testing/web-platform/tests/service-workers/service-worker/fetch-event-redirect.https.html
@@ -60,16 +60,17 @@ function redirect_fetch_test(t, test) {
         } else {
           resolve(e.data.detail);
         }
       };
       frame.contentWindow.postMessage({
         url: url,
         request_init: test.request_init,
         redirect_dest: test.redirect_dest,
+        expected_type: test.expected_type,
       }, '*', [channel.port2]);
     });
 
     if (test.should_reject) {
       return assert_promise_rejects(p);
     }
 
     return p.then(function(result) {
@@ -220,39 +221,35 @@ async_test(function(t) {
     name: 'nonav-manual-nocors-redirects-to-nocors-nocreds',
     redirect_dest: 'no-cors',
     url_credentials: false,
     expected_type: 'opaqueredirect',
     request_init: {
       redirect: 'manual',
       mode: 'no-cors'
     },
-    // should reject because only navigations can be intercepted with
-    // opaqueredirect responses
-    should_reject: true
+    should_reject: false
   });
 }, 'Non-navigation, manual redirect, no-cors mode Request redirected to ' +
-   'no-cors without credentials should fail opaqueredirect interception');
+   'no-cors without credentials should succeed interception');
 
 async_test(function(t) {
   redirect_fetch_test(t, {
     name: 'nonav-manual-nocors-redirects-to-cors-nocreds',
     redirect_dest: 'cors',
     url_credentials: false,
     expected_type: 'opaqueredirect',
     request_init: {
       redirect: 'manual',
       mode: 'no-cors'
     },
-    // should reject because only navigations can be intercepted with
-    // opaqueredirect responses
-    should_reject: true
+    should_reject: false
   });
 }, 'Non-navigation, manual redirect, no-cors mode Request redirected to ' +
-   'cors without credentials should fail opaqueredirect interception');
+   'cors without credentials should succeed interception');
 
 async_test(function(t) {
   redirect_fetch_test(t, {
     name: 'nonav-manual-cors-redirects-to-sameorigin-creds',
     redirect_dest: 'same-origin',
     url_credentials: true,
     expected_type: 'opaqueredirect',
     request_init: {
@@ -373,39 +370,35 @@ async_test(function(t) {
     name: 'nonav-manual-nocors-redirects-to-nocors-creds',
     redirect_dest: 'no-cors',
     url_credentials: true,
     expected_type: 'opaqueredirect',
     request_init: {
       redirect: 'manual',
       mode: 'no-cors'
     },
-    // should reject because only navigations can be intercepted with
-    // opaqueredirect responses
-    should_reject: true
+    should_reject: false
   });
 }, 'Non-navigation, manual redirect, no-cors mode Request redirected to ' +
-   'no-cors with credentials should fail opaqueredirect interception');
+   'no-cors with credentials should succeed interception');
 
 async_test(function(t) {
   redirect_fetch_test(t, {
     name: 'nonav-manual-nocors-redirects-to-cors-creds',
     redirect_dest: 'cors',
     url_credentials: true,
     expected_type: 'opaqueredirect',
     request_init: {
       redirect: 'manual',
       mode: 'no-cors'
     },
-    // should reject because only navigations can be intercepted with
-    // opaqueredirect responses
-    should_reject: true
+    should_reject: false
   });
 }, 'Non-navigation, manual redirect, no-cors mode Request redirected to ' +
-   'cors with credentials should fail opaqueredirect interception');
+   'cors with credentials should succeed interception');
 
 async_test(function(t) {
   redirect_fetch_test(t, {
     name: 'nonav-follow-cors-redirects-to-sameorigin-nocreds',
     redirect_dest: 'same-origin',
     url_credentials: false,
     expected_type: 'basic',
     request_init: {
--- a/testing/web-platform/tests/service-workers/service-worker/resources/fetch-event-redirect-iframe.html
+++ b/testing/web-platform/tests/service-workers/service-worker/resources/fetch-event-redirect-iframe.html
@@ -1,18 +1,19 @@
 <script>
 window.addEventListener('message', function(evt) {
   var port = evt.ports[0];
   var data = evt.data;
   fetch(new Request(data.url, data.request_init)).then(function(response) {
     if (data.request_init.mode === 'no-cors' && data.redirect_dest != 'same-origin') {
-      if (response.type === 'opaque') {
+      if (response.type === data.expected_type &&
+          (response.type === 'opaque' || response.type === 'opaqueredirect')) {
         return {result: 'success', detail: ''};
       } else {
-        return {result: 'failure', detail: 'expected opaque response'};
+        return {result: 'failure', detail: 'expected ' + data.expected_type + ' response'};
       }
     }
     return response.json();
   }).then(function(body) {
     port.postMessage({result: body.result, detail: body.detail});
   }).catch(function(e) {
     port.postMessage({result: 'reject', detail: e.toString()});
   });