Bug 1204596 - Part 1: Avoid overriding the channel final URI when it gets intercepted; r=nsm,bkelly
☠☠ backed out by 83e1183fff66 ☠ ☠
authorEhsan Akhgari <ehsan@mozilla.com>
Mon, 14 Sep 2015 17:06:26 -0400
changeset 296278 33a968e122bc207129970f57ea11a45127f0bd78
parent 296277 512e9862f1a0176e95060a66fb05d0f065519a62
child 296279 daa0e457650e98f7fd9060eab3df0430f26820e1
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.0a2
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/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/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);