Bug 1222008 - P1: Propagate the URLs from respondWith() if the response's URL is different from the request's. r=bkelly

--HG--
extra : rebase_source : 97bc74f18e734673ab598925410c8b284c4aaba8

dom/workers/ServiceWorkerEvents.cpp

@@ -342,8 +342,8 @@ public:
     RefPtr<BodyCopyHandle> copyHandle;
     copyHandle = new BodyCopyHandle(Move(mClosure));
 
-    rv = mChannel->StartSynthesizedResponse(body, copyHandle,
+    rv = mChannel->StartSynthesizedResponse(body, copyHandle, mResponseURLSpec,
-                                            mResponseURLSpec);
+                                            mInternalResponse->IsRedirected());
     if (NS_WARN_IF(NS_FAILED(rv))) {
       mChannel->CancelInterception(NS_ERROR_INTERCEPTION_FAILED);
       return NS_OK;
@@ -658,22 +658,31 @@ RespondWithHandler::ResolvedCallback(JSContext* aCx, JS::Handle<JS::Value> aValu
   if (NS_WARN_IF(!ir)) {
     return;
   }
-  // When an opaque response is encountered, we need the original channel's principal
+
-  // to reflect the final URL. Non-opaque responses are either same-origin or CORS-enabled
+  // An extra safety check to make sure our invariant that opaque and cors
-  // cross-origin responses, which are treated as same-origin by consumers.
+  // responses always have a URL does not break.
-  nsCString responseURL;
+  if (NS_WARN_IF((response->Type() == ResponseType::Opaque ||
-  if (response->Type() == ResponseType::Opaque) {
+                  response->Type() == ResponseType::Cors) &&
-    responseURL = ir->GetUnfilteredURL();
+                 ir->GetUnfilteredURL().IsEmpty())) {
-    if (NS_WARN_IF(responseURL.IsEmpty())) {
+    MOZ_DIAGNOSTIC_ASSERT(false, "Cors or opaque Response without a URL");
     return;
   }
-  }
 
   Telemetry::ScalarAdd(Telemetry::ScalarID::SW_SYNTHESIZED_RES_COUNT, 1);
 
   if (mRequestMode == RequestMode::Same_origin &&
       response->Type() == ResponseType::Cors) {
     Telemetry::ScalarAdd(Telemetry::ScalarID::SW_CORS_RES_FOR_SO_REQ_COUNT, 1);
+
+    // XXXtt: quirkResponse, will be implement in the follow-up patch.
+  }
+
+  // Propagate the URL to the content if the request mode is not "navigate".
+  // Note that, we only reflect the final URL if the response.redirected is
+  // false. We propagate all the URLs if the response.redirected is true.
+  nsCString responseURL;
+  if (mRequestMode != RequestMode::Navigate) {
+    responseURL = ir->GetUnfilteredURL();
   }
 
   UniquePtr<RespondWithClosure> closure(new RespondWithClosure(mInterceptedChannel,

netwerk/base/nsINetworkInterceptController.idl

@@ -78,10 +78,15 @@ interface nsIInterceptedChannel : nsISupports
      * - The caller may optionally pass a spec for a URL that this response
      *   originates from; an empty string will cause the original
      *   intercepted request's URL to be used instead.
+     * - The responseRedirected flag is false will cause the channel do an
+     *   internal redirect when the original intercepted reauest's URL is
+     *   different from the response's URL. The flag is true will cause the
+     *   chaanel do a non-internal redirect when the URLs are different.
      */
     void startSynthesizedResponse(in nsIInputStream body,
                                   in nsIInterceptedBodyCallback callback,
-                                  in ACString finalURLSpec);
+                                  in ACString finalURLSpec,
+                                  in bool responseRedirected);
 
     /**
      * Instruct a channel that has been intercepted that response synthesis

netwerk/protocol/http/HttpChannelChild.cpp

@@ -1728,14 +1728,23 @@ HttpChannelChild::Redirect1Begin(const uint32_t& registrarId,
 
 void
 HttpChannelChild::BeginNonIPCRedirect(nsIURI* responseURI,
-                                      const nsHttpResponseHead* responseHead)
+                                      const nsHttpResponseHead* responseHead,
+                                      bool aResponseRedirected)
 {
   LOG(("HttpChannelChild::BeginNonIPCRedirect [this=%p]\n", this));
 
+  // If the response has been redirected, propagate all the URLs to content.
+  // Thus, the exact value of the redirect flag does not matter as long as it's
+  // not REDIRECT_INTERNAL.
+  const uint32_t redirectFlag =
+    aResponseRedirected ? nsIChannelEventSink::REDIRECT_TEMPORARY
+                        : nsIChannelEventSink::REDIRECT_INTERNAL;
+
+
   nsCOMPtr<nsIChannel> newChannel;
   nsresult rv = SetupRedirect(responseURI,
                               responseHead,
-                              nsIChannelEventSink::REDIRECT_INTERNAL,
+                              redirectFlag,
                               getter_AddRefs(newChannel));
 
   if (NS_SUCCEEDED(rv)) {
@@ -1754,7 +1763,7 @@ HttpChannelChild::BeginNonIPCRedirect(nsIURI* responseURI,
 
     rv = gHttpHandler->AsyncOnChannelRedirect(this,
                                               newChannel,
-                                              nsIChannelEventSink::REDIRECT_INTERNAL,
+                                              redirectFlag,
                                               target);
   }
 

netwerk/protocol/http/HttpChannelChild.h

@@ -432,7 +432,8 @@ private:
 
   // Perform a redirection without communicating with the parent process at all.
   void BeginNonIPCRedirect(nsIURI* responseURI,
-                           const nsHttpResponseHead* responseHead);
+                           const nsHttpResponseHead* responseHead,
+                           bool responseRedirected);
 
   // Override the default security info pointer during a non-IPC redirection.
   void OverrideSecurityInfoForNonIPCRedirect(nsISupports* securityInfo);

netwerk/protocol/http/HttpChannelParentListener.cpp

@@ -318,7 +318,7 @@ public:
   {
     // The URL passed as an argument here doesn't matter, since the child will
     // receive a redirection notification as a result of this synthesized response.
-    mChannel->StartSynthesizedResponse(nullptr, nullptr, EmptyCString());
+    mChannel->StartSynthesizedResponse(nullptr, nullptr, EmptyCString(), false);
     mChannel->FinishSynthesizedResponse();
     return NS_OK;
   }

netwerk/protocol/http/InterceptedChannel.cpp

@@ -275,7 +275,8 @@ InterceptedChannelContent::SynthesizeHeader(const nsACString& aName, const nsACS
 NS_IMETHODIMP
 InterceptedChannelContent::StartSynthesizedResponse(nsIInputStream* aBody,
                                                     nsIInterceptedBodyCallback* aBodyCallback,
-                                                    const nsACString& aFinalURLSpec)
+                                                    const nsACString& aFinalURLSpec,
+                                                    bool aResponseRedirected)
 {
   if (NS_WARN_IF(mClosed)) {
     return NS_ERROR_NOT_AVAILABLE;
@@ -302,7 +303,8 @@ InterceptedChannelContent::StartSynthesizedResponse(nsIInputStream* aBody,
   originalURI->Equals(responseURI, &equal);
   if (!equal) {
     mChannel->ForceIntercepted(aBody, aBodyCallback);
-    mChannel->BeginNonIPCRedirect(responseURI, *mSynthesizedResponseHead.ptr());
+    mChannel->BeginNonIPCRedirect(responseURI, *mSynthesizedResponseHead.ptr(),
+                                  aResponseRedirected);
   } else {
     mChannel->OverrideWithSynthesizedResponse(mSynthesizedResponseHead.ref(),
                                               aBody, aBodyCallback,

netwerk/protocol/http/InterceptedChannel.h

@@ -182,7 +182,8 @@ public:
   NS_IMETHOD ResetInterception() override;
   NS_IMETHOD StartSynthesizedResponse(nsIInputStream* aBody,
                                       nsIInterceptedBodyCallback* aBodyCallback,
-                                      const nsACString& aFinalURLSpec) override;
+                                      const nsACString& aFinalURLSpec,
+                                      bool aResponseRedirected) override;
   NS_IMETHOD FinishSynthesizedResponse() override;
   NS_IMETHOD GetChannel(nsIChannel** aChannel) override;
   NS_IMETHOD GetSecureUpgradedChannelURI(nsIURI** aURI) override;

netwerk/protocol/http/InterceptedHttpChannel.cpp

@@ -229,13 +229,15 @@ InterceptedHttpChannel::FollowSyntheticRedirect()
 }
 
 nsresult
-InterceptedHttpChannel::RedirectForOpaqueResponse(nsIURI* aResponseURI)
+InterceptedHttpChannel::RedirectForResponseURL(nsIURI* aResponseURI,
+                                               bool aResponseRedirected)
 {
-  // Perform an internal redirect to another InterceptedHttpChannel using
+  // Perform a service worker redirect to another InterceptedHttpChannel using
-  // the given cross-origin response URL.  The resulting channel will then
+  // the given response URL. It allows content to see the final URL where
-  // process the synthetic response as normal.  This extra redirect is
+  // appropriate and also helps us enforce cross-origin restrictions. The
-  // performed so that listeners treat the result as unsafe cross-origin
+  // resulting channel will then process the synthetic response as normal. This
-  // data.
+  // extra redirect is performed so that listeners treat the result as unsafe
+  // cross-origin data.
 
   nsresult rv = NS_OK;
 
@@ -253,7 +255,11 @@ InterceptedHttpChannel::RedirectForOpaqueResponse(nsIURI* aResponseURI)
                         static_cast<nsProxyInfo*>(mProxyInfo.get()),
                         mProxyResolveFlags, mProxyURI, mChannelId);
 
-  uint32_t flags = nsIChannelEventSink::REDIRECT_INTERNAL;
+  // If the response has been redirected, propagate all the URLs to content.
+  // Thus, the exact value of the redirect flag does not matter as long as it's
+  // not REDIRECT_INTERNAL.
+  uint32_t flags = aResponseRedirected ? nsIChannelEventSink::REDIRECT_TEMPORARY
+                                       : nsIChannelEventSink::REDIRECT_INTERNAL;
 
   nsCOMPtr<nsILoadInfo> redirectLoadInfo =
     CloneLoadInfoForRedirect(aResponseURI, flags);
@@ -717,7 +723,8 @@ InterceptedHttpChannel::SynthesizeHeader(const nsACString& aName,
 NS_IMETHODIMP
 InterceptedHttpChannel::StartSynthesizedResponse(nsIInputStream* aBody,
                                                  nsIInterceptedBodyCallback* aBodyCallback,
-                                                 const nsACString& aFinalURLSpec)
+                                                 const nsACString& aFinalURLSpec,
+                                                 bool aResponseRedirected)
 {
   nsresult rv = NS_OK;
 
@@ -783,7 +790,7 @@ InterceptedHttpChannel::StartSynthesizedResponse(nsIInputStream* aBody,
   bool equal = false;
   Unused << mURI->Equals(responseURI, &equal);
   if (!equal) {
-    rv = RedirectForOpaqueResponse(responseURI);
+    rv = RedirectForResponseURL(responseURI, aResponseRedirected);
     NS_ENSURE_SUCCESS(rv, rv);
 
     return NS_OK;

netwerk/protocol/http/InterceptedHttpChannel.h

@@ -113,8 +113,12 @@ private:
   nsresult
   FollowSyntheticRedirect();
 
+  // If the response's URL is different from the request's then do a service
+  // worker redirect. If Response.redirected is false we do an internal
+  // redirect. Otherwise, if Response.redirect is true do a non-internal
+  // redirect so end consumers detect the redirected state.
   nsresult
-  RedirectForOpaqueResponse(nsIURI* aResponseURI);
+  RedirectForResponseURL(nsIURI* aResponseURI, bool aResponseRedirected);
 
   nsresult
   StartPump();