Bug 1204596 - Part 1: Avoid overriding the channel final URI when it gets intercepted; r=nsm,bkelly
☠☠ backed out by 004e30faaea8 ☠ ☠
authorEhsan Akhgari <ehsan@mozilla.com>
Mon, 14 Sep 2015 17:06:26 -0400
changeset 295541 d10c6f32ce461a3aef76868eaa78de009145657d
parent 295540 c150293cf55c0fe19435d7eb9ede01a39796fe2d
child 295542 cfc4c4ecbbf520970834fe7cd6e2c017bcd86852
push id5245
push userraliiev@mozilla.com
push dateThu, 29 Oct 2015 11:30:51 +0000
treeherdermozilla-beta@dac831dc1bd0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnsm, bkelly
bugs1204596
milestone43.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 1204596 - Part 1: Avoid overriding the channel final URI when it gets intercepted; r=nsm,bkelly
dom/cache/DBSchema.cpp
dom/fetch/ChannelInfo.cpp
dom/fetch/ChannelInfo.h
dom/fetch/ChannelInfo.ipdlh
dom/tests/mochitest/fetch/fetch_test_framework.js
dom/tests/mochitest/fetch/reroute.html
dom/tests/mochitest/fetch/reroute.js
dom/tests/mochitest/fetch/test_fetch_basic_http.js
dom/tests/mochitest/fetch/test_fetch_cors.js
netwerk/protocol/http/HttpBaseChannel.cpp
netwerk/protocol/http/HttpBaseChannel.h
--- a/dom/cache/DBSchema.cpp
+++ b/dom/cache/DBSchema.cpp
@@ -1598,18 +1598,16 @@ InsertEntry(mozIStorageConnection* aConn
       "response_type, "
       "response_url, "
       "response_status, "
       "response_status_text, "
       "response_headers_guard, "
       "response_body_id, "
       "response_security_info_id, "
       "response_principal_info, "
-      "response_redirected, "
-      "response_redirected_url, "
       "cache_id "
     ") VALUES ("
       ":request_method, "
       ":request_url_no_query, "
       ":request_url_no_query_hash, "
       ":request_url_query, "
       ":request_url_query_hash, "
       ":request_referrer, "
@@ -1623,18 +1621,16 @@ InsertEntry(mozIStorageConnection* aConn
       ":response_type, "
       ":response_url, "
       ":response_status, "
       ":response_status_text, "
       ":response_headers_guard, "
       ":response_body_id, "
       ":response_security_info_id, "
       ":response_principal_info, "
-      ":response_redirected, "
-      ":response_redirected_url, "
       ":cache_id "
     ");"
   ), getter_AddRefs(state));
   if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
 
   rv = state->BindUTF8StringByName(NS_LITERAL_CSTRING("request_method"),
                                    aRequest.method());
   if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
@@ -1741,24 +1737,16 @@ InsertEntry(mozIStorageConnection* aConn
     attrs.CreateSuffix(suffix);
     serializedInfo.Append(suffix);
   }
 
   rv = state->BindUTF8StringByName(NS_LITERAL_CSTRING("response_principal_info"),
                                    serializedInfo);
   if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
 
-  rv = state->BindInt32ByName(NS_LITERAL_CSTRING("response_redirected"),
-                              aResponse.channelInfo().redirected() ? 1 : 0);
-  if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
-
-  rv = state->BindUTF8StringByName(NS_LITERAL_CSTRING("response_redirected_url"),
-                                   aResponse.channelInfo().redirectedURI());
-  if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
-
   rv = state->BindInt64ByName(NS_LITERAL_CSTRING("cache_id"), aCacheId);
   if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
 
   rv = state->Execute();
   if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
 
   rv = aConn->CreateStatement(NS_LITERAL_CSTRING(
     "SELECT last_insert_rowid()"
@@ -1841,18 +1829,16 @@ ReadResponse(mozIStorageConnection* aCon
     "SELECT "
       "entries.response_type, "
       "entries.response_url, "
       "entries.response_status, "
       "entries.response_status_text, "
       "entries.response_headers_guard, "
       "entries.response_body_id, "
       "entries.response_principal_info, "
-      "entries.response_redirected, "
-      "entries.response_redirected_url, "
       "security_info.data "
     "FROM entries "
     "LEFT OUTER JOIN security_info "
     "ON entries.response_security_info_id=security_info.id "
     "WHERE entries.id=:id;"
   ), getter_AddRefs(state));
   if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
 
@@ -1907,24 +1893,17 @@ ReadResponse(mozIStorageConnection* aCon
       NS_WARNING("Something went wrong parsing a serialized principal!");
       return NS_ERROR_FAILURE;
     }
 
     aSavedResponseOut->mValue.principalInfo() =
       mozilla::ipc::ContentPrincipalInfo(attrs.mAppId, attrs.mInBrowser, originNoSuffix);
   }
 
-  int32_t redirected;
-  rv = state->GetInt32(7, &redirected);
-  aSavedResponseOut->mValue.channelInfo().redirected() = !!redirected;
-
-  rv = state->GetUTF8String(8, aSavedResponseOut->mValue.channelInfo().redirectedURI());
-  if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
-
-  rv = state->GetBlobAsUTF8String(9, aSavedResponseOut->mValue.channelInfo().securityInfo());
+  rv = state->GetBlobAsUTF8String(7, aSavedResponseOut->mValue.channelInfo().securityInfo());
   if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
 
   rv = aConn->CreateStatement(NS_LITERAL_CSTRING(
     "SELECT "
       "name, "
       "value "
     "FROM response_headers "
     "WHERE entry_id=:entry_id;"
--- a/dom/fetch/ChannelInfo.cpp
+++ b/dom/fetch/ChannelInfo.cpp
@@ -26,76 +26,53 @@ ChannelInfo::InitFromDocument(nsIDocumen
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(!mInited, "Cannot initialize the object twice");
 
   nsCOMPtr<nsISupports> securityInfo = aDoc->GetSecurityInfo();
   if (securityInfo) {
     SetSecurityInfo(securityInfo);
   }
 
-  // mRedirected flag and mRedirectedURISpec are only important for maintaining
-  // the channel's redirected status.  If the ChannelInfo is initialized from
-  // a document, that document has already asked the channel from which it was
-  // loaded about the current channel URI, so it won't matter if a future
-  // ResurrectInfoOnChannel() call misses whether the channel was redirected.
-  mRedirected = false;
   mInited = true;
 }
 
 void
 ChannelInfo::InitFromChannel(nsIChannel* aChannel)
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(!mInited, "Cannot initialize the object twice");
 
   nsCOMPtr<nsISupports> securityInfo;
   aChannel->GetSecurityInfo(getter_AddRefs(securityInfo));
   if (securityInfo) {
     SetSecurityInfo(securityInfo);
   }
 
-  nsLoadFlags loadFlags = 0;
-  aChannel->GetLoadFlags(&loadFlags);
-  mRedirected = (loadFlags & nsIChannel::LOAD_REPLACE);
-  if (mRedirected) {
-    // Save the spec and not the nsIURI object itself, since those objects are
-    // not thread-safe, and releasing them somewhere other than the main thread
-    // is not possible.
-    nsCOMPtr<nsIURI> redirectedURI;
-    aChannel->GetURI(getter_AddRefs(redirectedURI));
-    if (redirectedURI) {
-      redirectedURI->GetSpec(mRedirectedURISpec);
-    }
-  }
-
   mInited = true;
 }
 
 void
 ChannelInfo::InitFromChromeGlobal(nsIGlobalObject* aGlobal)
 {
   MOZ_ASSERT(!mInited, "Cannot initialize the object twice");
   MOZ_ASSERT(aGlobal);
 
   MOZ_RELEASE_ASSERT(
     nsContentUtils::IsSystemPrincipal(aGlobal->PrincipalOrNull()));
 
   mSecurityInfo.Truncate();
-  mRedirected = false;
   mInited = true;
 }
 
 void
 ChannelInfo::InitFromIPCChannelInfo(const mozilla::ipc::IPCChannelInfo& aChannelInfo)
 {
   MOZ_ASSERT(!mInited, "Cannot initialize the object twice");
 
   mSecurityInfo = aChannelInfo.securityInfo();
-  mRedirectedURISpec = aChannelInfo.redirectedURI();
-  mRedirected = aChannelInfo.redirected();
 
   mInited = true;
 }
 
 void
 ChannelInfo::SetSecurityInfo(nsISupports* aSecurityInfo)
 {
   MOZ_ASSERT(mSecurityInfo.IsEmpty(), "security info should only be set once");
@@ -137,54 +114,25 @@ ChannelInfo::ResurrectInfoOnChannel(nsIC
       if (NS_WARN_IF(!jarChannel)) {
         return NS_ERROR_FAILURE;
       }
       static_cast<nsJARChannel*>(jarChannel.get())->
         OverrideSecurityInfo(infoObj);
     }
   }
 
-  if (mRedirected) {
-    nsLoadFlags flags = 0;
-    aChannel->GetLoadFlags(&flags);
-    flags |= nsIChannel::LOAD_REPLACE;
-    aChannel->SetLoadFlags(flags);
-
-    nsCOMPtr<nsIURI> redirectedURI;
-    nsresult rv = NS_NewURI(getter_AddRefs(redirectedURI),
-                            mRedirectedURISpec);
-    if (NS_WARN_IF(NS_FAILED(rv))) {
-      return rv;
-    }
-    if (httpChannel) {
-      net::HttpBaseChannel* httpBaseChannel =
-        static_cast<net::HttpBaseChannel*>(httpChannel.get());
-      rv = httpBaseChannel->OverrideURI(redirectedURI);
-      if (NS_WARN_IF(NS_FAILED(rv))) {
-        return rv;
-      }
-    } else {
-      if (NS_WARN_IF(!jarChannel)) {
-        return NS_ERROR_FAILURE;
-      }
-      static_cast<nsJARChannel*>(jarChannel.get())->OverrideURI(redirectedURI);
-    }
-  }
-
   return NS_OK;
 }
 
 mozilla::ipc::IPCChannelInfo
 ChannelInfo::AsIPCChannelInfo() const
 {
   // This may be called when mInited is false, for example if we try to store
   // a synthesized Response object into the Cache.  Uninitialized and empty
   // ChannelInfo objects are indistinguishable at the IPC level, so this is
   // fine.
 
   IPCChannelInfo ipcInfo;
 
   ipcInfo.securityInfo() = mSecurityInfo;
-  ipcInfo.redirectedURI() = mRedirectedURISpec;
-  ipcInfo.redirected() = mRedirected;
 
   return ipcInfo;
 }
--- a/dom/fetch/ChannelInfo.h
+++ b/dom/fetch/ChannelInfo.h
@@ -41,35 +41,30 @@ namespace dom {
 // initialized.  There are assertions ensuring these invariants.
 class ChannelInfo final
 {
 public:
   typedef mozilla::ipc::IPCChannelInfo IPCChannelInfo;
 
   ChannelInfo()
     : mInited(false)
-    , mRedirected(false)
   {
   }
 
   ChannelInfo(const ChannelInfo& aRHS)
     : mSecurityInfo(aRHS.mSecurityInfo)
-    , mRedirectedURISpec(aRHS.mRedirectedURISpec)
     , mInited(aRHS.mInited)
-    , mRedirected(aRHS.mRedirected)
   {
   }
 
   ChannelInfo&
   operator=(const ChannelInfo& aRHS)
   {
     mSecurityInfo = aRHS.mSecurityInfo;
-    mRedirectedURISpec = aRHS.mRedirectedURISpec;
     mInited = aRHS.mInited;
-    mRedirected = aRHS.mRedirected;
     return *this;
   }
 
   void InitFromDocument(nsIDocument* aDoc);
   void InitFromChannel(nsIChannel* aChannel);
   void InitFromChromeGlobal(nsIGlobalObject* aGlobal);
   void InitFromIPCChannelInfo(const IPCChannelInfo& aChannelInfo);
 
@@ -84,17 +79,15 @@ public:
 
   IPCChannelInfo AsIPCChannelInfo() const;
 
 private:
   void SetSecurityInfo(nsISupports* aSecurityInfo);
 
 private:
   nsCString mSecurityInfo;
-  nsCString mRedirectedURISpec;
   bool mInited;
-  bool mRedirected;
 };
 
 } // namespace dom
 } // namespace mozilla
 
 #endif // mozilla_dom_ChannelInfo_h
--- a/dom/fetch/ChannelInfo.ipdlh
+++ b/dom/fetch/ChannelInfo.ipdlh
@@ -3,14 +3,12 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 namespace mozilla {
 namespace ipc {
 
 struct IPCChannelInfo
 {
   nsCString securityInfo;
-  nsCString redirectedURI;
-  bool redirected;
 };
 
 } // namespace ipc
 } // namespace mozilla
--- a/dom/tests/mochitest/fetch/fetch_test_framework.js
+++ b/dom/tests/mochitest/fetch/fetch_test_framework.js
@@ -1,9 +1,14 @@
 function testScript(script) {
+  // reroute.html should have set this variable if a service worker is present!
+  if (!("isSWPresent" in window)) {
+    window.isSWPresent = false;
+  }
+
   function setupPrefs() {
     return new Promise(function(resolve, reject) {
       SpecialPowers.pushPrefEnv({
         "set": [["dom.requestcontext.enabled", true],
                 ["dom.serviceWorkers.enabled", true],
                 ["dom.serviceWorkers.testing.enabled", true],
                 ["dom.serviceWorkers.exemptFromPerDomainMax", true]]
       }, resolve);
--- a/dom/tests/mochitest/fetch/reroute.html
+++ b/dom/tests/mochitest/fetch/reroute.html
@@ -1,10 +1,11 @@
 <!DOCTYPE html>
 <script>
 ["SimpleTest", "ok", "info", "is", "$"]
   .forEach((v) => window[v] = window.parent[v]);
 </script>
 <script type="text/javascript" src="utils.js"> </script>
 <script type="text/javascript" src="fetch_test_framework.js"> </script>
 <script>
+window.isSWPresent = true;
 testScript(location.search.substring(1) + ".js");
 </script>
--- a/dom/tests/mochitest/fetch/reroute.js
+++ b/dom/tests/mochitest/fetch/reroute.js
@@ -3,17 +3,16 @@ onfetch = function(e) {
     // Silently rewrite the referrer so the referrer test passes since the
     // document/worker isn't aware of this service worker.
     var url = e.request.url.substring(0, e.request.url.indexOf('?'));
     url += '?headers=' + ({ 'Referer': self.location.href }).toSource();
 
     e.respondWith(fetch(url, {
       method: e.request.method,
       headers: e.request.headers,
-      body: e.request.body,
       mode: e.request.mode,
       credentials: e.request.credentials,
       cache: e.request.cache,
     }));
     return;
   }
   e.respondWith(fetch(e.request));
 };
--- a/dom/tests/mochitest/fetch/test_fetch_basic_http.js
+++ b/dom/tests/mochitest/fetch/test_fetch_basic_http.js
@@ -7,18 +7,18 @@ var passFiles = [['file_XHR_pass1.xml', 
 
 function testURL() {
   var promises = [];
   passFiles.forEach(function(entry) {
     var p = fetch(path + entry[0]).then(function(res) {
       ok(res.type !== "error", "Response should not be an error for " + entry[0]);
       is(res.status, entry[2], "Status should match expected for " + entry[0]);
       is(res.statusText, entry[3], "Status text should match expected for " + entry[0]);
-      // This file redirects to pass2
-      if (entry[0] != "file_XHR_pass3.txt")
+      // This file redirects to pass2, but that is invisible if a SW is present.
+      if (entry[0] != "file_XHR_pass3.txt" || isSWPresent)
         ok(res.url.endsWith(path + entry[0]), "Response url should match request for simple fetch for " + entry[0]);
       else
         ok(res.url.endsWith(path + "file_XHR_pass2.txt"), "Response url should match request for simple fetch for " + entry[0]);
       is(res.headers.get('content-type'), entry[4], "Response should have content-type for " + entry[0]);
     });
     promises.push(p);
   });
 
@@ -44,17 +44,18 @@ function testURLFail() {
 function testRequestGET() {
   var promises = [];
   passFiles.forEach(function(entry) {
     var req = new Request(path + entry[0], { method: entry[1] });
     var p = fetch(req).then(function(res) {
       ok(res.type !== "error", "Response should not be an error for " + entry[0]);
       is(res.status, entry[2], "Status should match expected for " + entry[0]);
       is(res.statusText, entry[3], "Status text should match expected for " + entry[0]);
-      if (entry[0] != "file_XHR_pass3.txt")
+      // This file redirects to pass2, but that is invisible if a SW is present.
+      if (entry[0] != "file_XHR_pass3.txt" || isSWPresent)
         ok(res.url.endsWith(path + entry[0]), "Response url should match request for simple fetch for " + entry[0]);
       else
         ok(res.url.endsWith(path + "file_XHR_pass2.txt"), "Response url should match request for simple fetch for " + entry[0]);
       is(res.headers.get('content-type'), entry[4], "Response should have content-type for " + entry[0]);
     });
     promises.push(p);
   });
 
--- a/dom/tests/mochitest/fetch/test_fetch_cors.js
+++ b/dom/tests/mochitest/fetch/test_fetch_cors.js
@@ -1272,17 +1272,27 @@ function testRedirects() {
     var request = new Request(req.url, { method: req.method,
                                          headers: req.headers,
                                          body: req.body });
     fetches.push((function(request, test) {
       return fetch(request).then(function(res) {
         ok(test.pass, "Expected test to pass for " + test.toSource());
         is(res.status, 200, "wrong status in test for " + test.toSource());
         is(res.statusText, "OK", "wrong status text for " + test.toSource());
-        is((new URL(res.url)).host, (new URL(test.hops[test.hops.length-1].server)).host, "Response URL should be redirected URL");
+        var reqHost = (new URL(req.url)).host;
+        // If there is a service worker present, the redirections will be
+        // transparent, assuming that the original request is to the current
+        // site and would be intercepted.
+        if (isSWPresent) {
+          if (reqHost === location.host) {
+            is((new URL(res.url)).host, reqHost, "Response URL should be original URL with a SW present");
+          }
+        } else {
+          is((new URL(res.url)).host, (new URL(test.hops[test.hops.length-1].server)).host, "Response URL should be redirected URL");
+        }
         return res.text().then(function(v) {
           is(v, "<res>hello pass</res>\n",
              "wrong responseText in test for " + test.toSource());
         });
       }, function(e) {
         ok(!test.pass, "Expected test failure for " + test.toSource());
         ok(e instanceof TypeError, "Exception should be TypeError for " + test.toSource());
       });
--- a/netwerk/protocol/http/HttpBaseChannel.cpp
+++ b/netwerk/protocol/http/HttpBaseChannel.cpp
@@ -1556,38 +1556,16 @@ HttpBaseChannel::OverrideSecurityInfo(ns
          "[this=%p]\n", this));
     return NS_ERROR_UNEXPECTED;
   }
 
   mSecurityInfo = aSecurityInfo;
   return NS_OK;
 }
 
-nsresult
-HttpBaseChannel::OverrideURI(nsIURI* aRedirectedURI)
-{
-  MOZ_ASSERT(mLoadFlags & LOAD_REPLACE,
-             "This can only happen if the LOAD_REPLACE flag is set");
-  MOZ_ASSERT(ShouldIntercept(),
-             "This can only be called on channels that can be intercepted");
-  if (!(mLoadFlags & LOAD_REPLACE)) {
-    LOG(("HttpBaseChannel::OverrideURI LOAD_REPLACE flag not set! [this=%p]\n",
-         this));
-    return NS_ERROR_UNEXPECTED;
-  }
-  if (!mResponseCouldBeSynthesized) {
-    LOG(("HttpBaseChannel::OverrideURI channel cannot be intercepted! "
-         "[this=%p]\n", this));
-    return NS_ERROR_UNEXPECTED;
-  }
-
-  mURI = aRedirectedURI;
-  return NS_OK;
-}
-
 NS_IMETHODIMP
 HttpBaseChannel::IsNoStoreResponse(bool *value)
 {
   if (!mResponseHead)
     return NS_ERROR_NOT_AVAILABLE;
   *value = mResponseHead->NoStore();
   return NS_OK;
 }
--- a/netwerk/protocol/http/HttpBaseChannel.h
+++ b/netwerk/protocol/http/HttpBaseChannel.h
@@ -251,17 +251,16 @@ public:
 
     nsHttpResponseHead * GetResponseHead() const { return mResponseHead; }
     nsHttpRequestHead * GetRequestHead() { return &mRequestHead; }
 
     const NetAddr& GetSelfAddr() { return mSelfAddr; }
     const NetAddr& GetPeerAddr() { return mPeerAddr; }
 
     nsresult OverrideSecurityInfo(nsISupports* aSecurityInfo);
-    nsresult OverrideURI(nsIURI* aRedirectedURI);
 
 public: /* Necko internal use only... */
     bool IsNavigation();
 
     // Return whether upon a redirect code of httpStatus for method, the
     // request method should be rewritten to GET.
     static bool ShouldRewriteRedirectToGET(uint32_t httpStatus,
                                            nsHttpRequestHead::ParsedMethodType method);