From b98b2f6c82bfb6bc06a5af2e10c8eb0c7247df3b Mon Sep 17 00:00:00 2001 From: Tom Tung Date: Wed, 12 Sep 2018 12:44:30 +0000 Subject: [PATCH] Bug 1486445 - P1 - Propagate the sw internally redirected URL to fetching request; r=asuth Bug 1222008 didn't propagate a sw redirected URL to outer response properly. To fix that this patch mainly make a redirecting request be overwritten while it's redirected by a service worker. This patch also add a private setter function for InternalRequest and a public checking function to avoid that function from being used widely. Differential Revision: https://phabricator.services.mozilla.com/D4762 --HG-- extra : moz-landing-system : lando --- dom/fetch/FetchDriver.cpp | 42 ++++++++++++++++++++----------------- dom/fetch/InternalRequest.h | 26 +++++++++++++++++++++++ 2 files changed, 49 insertions(+), 19 deletions(-) diff --git a/dom/fetch/FetchDriver.cpp b/dom/fetch/FetchDriver.cpp index 9be53d035b30..d0c35eec23a8 100644 --- a/dom/fetch/FetchDriver.cpp +++ b/dom/fetch/FetchDriver.cpp @@ -1296,27 +1296,31 @@ FetchDriver::AsyncOnChannelRedirect(nsIChannel* aOldChannel, // However, ignore internal redirects here. We don't want to flip // Response.redirected to true if an internal redirect occurs. These // should be transparent to script. + nsCOMPtr uri; + MOZ_ALWAYS_SUCCEEDS(aNewChannel->GetURI(getter_AddRefs(uri))); + + nsCOMPtr uriClone; + nsresult rv = NS_GetURIWithoutRef(uri, getter_AddRefs(uriClone)); + if(NS_WARN_IF(NS_FAILED(rv))){ + return rv; + } + nsCString spec; + rv = uriClone->GetSpec(spec); + if(NS_WARN_IF(NS_FAILED(rv))){ + return rv; + } + nsCString fragment; + rv = uri->GetRef(fragment); + if(NS_WARN_IF(NS_FAILED(rv))){ + return rv; + } + if (!(aFlags & nsIChannelEventSink::REDIRECT_INTERNAL)) { - nsCOMPtr uri; - MOZ_ALWAYS_SUCCEEDS(aNewChannel->GetURI(getter_AddRefs(uri))); - - nsCOMPtr uriClone; - nsresult rv = NS_GetURIWithoutRef(uri, getter_AddRefs(uriClone)); - if(NS_WARN_IF(NS_FAILED(rv))){ - return rv; - } - nsCString spec; - rv = uriClone->GetSpec(spec); - if(NS_WARN_IF(NS_FAILED(rv))){ - return rv; - } - nsCString fragment; - rv = uri->GetRef(fragment); - if(NS_WARN_IF(NS_FAILED(rv))){ - return rv; - } - mRequest->AddURL(spec, fragment); + } else { + // Overwrite the URL only when the request is redirected by a service + // worker. + mRequest->SetURLForInternalRedirect(aFlags, spec, fragment); } NS_ConvertUTF8toUTF16 tRPHeaderValue(tRPHeaderCValue); diff --git a/dom/fetch/InternalRequest.h b/dom/fetch/InternalRequest.h index 64c10fe53fda..6be978f2d23c 100644 --- a/dom/fetch/InternalRequest.h +++ b/dom/fetch/InternalRequest.h @@ -133,6 +133,20 @@ public: return mURLList.LastElement(); } + + // A safe guard for ensuring that request's URL is only allowed to be set in a + // sw internal redirect. + void + SetURLForInternalRedirect(const uint32_t aFlag, + const nsACString& aURL, + const nsACString& aFragment) + { + // Only check in debug build to prevent it from being used unexpectedly. + MOZ_ASSERT(aFlag & nsIChannelEventSink::REDIRECT_INTERNAL); + + return SetURL(aURL, aFragment); + } + // AddURL should append the url into url list. // Normally we strip the fragment from the URL in Request::Constructor and // pass the fragment as the second argument into it. @@ -578,6 +592,18 @@ private: static bool IsWorkerContentPolicy(nsContentPolicyType aContentPolicyType); + // It should only be called while there is a service-worker-internal-redirect. + void + SetURL(const nsACString& aURL, const nsACString& aFragment) + { + MOZ_ASSERT(!aURL.IsEmpty()); + MOZ_ASSERT(!aURL.Contains('#')); + MOZ_ASSERT(mURLList.Length() > 0); + + mURLList.LastElement() = aURL; + mFragment.Assign(aFragment); + } + nsCString mMethod; // mURLList: a list of one or more fetch URLs nsTArray mURLList;