Bug 1363848 P2 Do not intercept internal redirects regardless of LOAD_BYPASS_SERVICE_WORKER flag. r=dragana a=lizzard
authorBen Kelly <ben@wanderview.com>
Fri, 16 Jun 2017 11:17:36 -0700
changeset 413996 a2ec6f92b8df102c0849762cdc77c406c9e88503
parent 413995 2bb8cf5019229b773d15c65a537f414e99376431
child 413997 74875a638dcd11ab0fc2bb329cb72914712a6475
push id1490
push usermtabara@mozilla.com
push dateMon, 31 Jul 2017 14:08:16 +0000
treeherdermozilla-release@70e32e6bf15e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdragana, lizzard
bugs1363848
milestone55.0
Bug 1363848 P2 Do not intercept internal redirects regardless of LOAD_BYPASS_SERVICE_WORKER flag. r=dragana a=lizzard
netwerk/protocol/http/HttpBaseChannel.cpp
--- a/netwerk/protocol/http/HttpBaseChannel.cpp
+++ b/netwerk/protocol/http/HttpBaseChannel.cpp
@@ -2931,17 +2931,36 @@ HttpBaseChannel::BypassServiceWorker() c
 }
 
 bool
 HttpBaseChannel::ShouldIntercept(nsIURI* aURI)
 {
   nsCOMPtr<nsINetworkInterceptController> controller;
   GetCallback(controller);
   bool shouldIntercept = false;
-  if (controller && !BypassServiceWorker() && mLoadInfo) {
+
+  // We should never intercept internal redirects.  The ServiceWorker code
+  // can trigger interntal redirects as the result of a FetchEvent.  If
+  // we re-intercept then an infinite loop can occur.
+  //
+  // Its also important that we do not set the LOAD_BYPASS_SERVICE_WORKER
+  // flag because an internal redirect occurs.  Its possible that another
+  // interception should occur after the internal redirect.  For example,
+  // if the ServiceWorker chooses not to call respondWith() the channel
+  // will be reset with an internal redirect.  If the request is a navigation
+  // and the network then triggers a redirect its possible the new URL
+  // should be intercepted again.
+  //
+  // Note, HSTS upgrade redirects are often treated the same as internal
+  // redirects.  In this case, however, we intentionally allow interception
+  // of HSTS upgrade redirects.  This matches the expected spec behavior and
+  // does not run the risk of infinite loops as described above.
+  bool internalRedirect = mLastRedirectFlags & nsIChannelEventSink::REDIRECT_INTERNAL;
+
+  if (controller && mLoadInfo && !BypassServiceWorker() && !internalRedirect) {
     nsresult rv = controller->ShouldPrepareForIntercept(aURI ? aURI : mURI.get(),
                                                         nsContentUtils::IsNonSubresourceRequest(this),
                                                         &shouldIntercept);
     if (NS_FAILED(rv)) {
       return false;
     }
   }
   return shouldIntercept;