Bug 1405739 P1 Don't expose internal redirect start/end/count through performance timing API. r=valentin
authorBen Kelly <ben@wanderview.com>
Fri, 06 Oct 2017 09:04:54 -0700
changeset 384762 6c5488ade824e460bfe19d4b347b78cfa8960a79
parent 384761 fd99f3454c510f766a19f7c6ea86f6278227b5a0
child 384763 ba89a80854b5064befa08ee115a36ff0e5ac2a9e
push id95860
push userbkelly@mozilla.com
push dateFri, 06 Oct 2017 16:05:03 +0000
treeherdermozilla-inbound@0d8fa7a51ad0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersvalentin
bugs1405739
milestone58.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 1405739 P1 Don't expose internal redirect start/end/count through performance timing API. r=valentin
dom/performance/PerformanceTiming.cpp
dom/performance/PerformanceTiming.h
netwerk/base/nsITimedChannel.idl
netwerk/protocol/http/HttpBaseChannel.cpp
netwerk/protocol/http/HttpBaseChannel.h
netwerk/protocol/http/HttpChannelParent.cpp
netwerk/protocol/http/NullHttpChannel.cpp
netwerk/protocol/http/nsHttpChannel.cpp
--- a/dom/performance/PerformanceTiming.cpp
+++ b/dom/performance/PerformanceTiming.cpp
@@ -173,17 +173,17 @@ PerformanceTiming::CheckAllowedOrigin(ns
 }
 
 bool
 PerformanceTiming::TimingAllowed() const
 {
   return mTimingAllowed;
 }
 
-uint16_t
+uint8_t
 PerformanceTiming::GetRedirectCount() const
 {
   if (!nsContentUtils::IsPerformanceTimingEnabled() || !IsInitialized() ||
       nsContentUtils::ShouldResistFingerprinting()) {
     return 0;
   }
   if (!mAllRedirectsSameOrigin) {
     return 0;
--- a/dom/performance/PerformanceTiming.h
+++ b/dom/performance/PerformanceTiming.h
@@ -136,17 +136,17 @@ public:
   {
     if (!nsContentUtils::IsPerformanceTimingEnabled() ||
         nsContentUtils::ShouldResistFingerprinting()) {
       return 0;
     }
     return GetDOMTiming()->GetUnloadEventEnd();
   }
 
-  uint16_t GetRedirectCount() const;
+  uint8_t GetRedirectCount() const;
 
   // Checks if the resource is either same origin as the page that started
   // the load, or if the response contains the Timing-Allow-Origin header
   // with a value of * or matching the domain of the loading Principal
   bool CheckAllowedOrigin(nsIHttpChannel* aResourceChannel, nsITimedChannel* aChannel);
 
   // Cached result of CheckAllowedOrigin. If false, security sensitive
   // attributes of the resourceTiming object will be set to 0
@@ -280,17 +280,17 @@ private:
   TimeStamp mConnectStart;
   TimeStamp mSecureConnectionStart;
   TimeStamp mConnectEnd;
   TimeStamp mRequestStart;
   TimeStamp mResponseStart;
   TimeStamp mCacheReadStart;
   TimeStamp mResponseEnd;
   TimeStamp mCacheReadEnd;
-  uint16_t mRedirectCount;
+  uint8_t mRedirectCount;
   bool mTimingAllowed;
   bool mAllRedirectsSameOrigin;
   bool mInitialized;
 
   // If the resourceTiming object should have non-zero redirectStart and
   // redirectEnd attributes. It is false if there were no redirects, or if
   // any of the responses didn't pass the timing-allow-check
   bool mReportCrossOriginRedirect;
--- a/netwerk/base/nsITimedChannel.idl
+++ b/netwerk/base/nsITimedChannel.idl
@@ -16,17 +16,18 @@ native TimeStamp(mozilla::TimeStamp);
 [scriptable, uuid(ca63784d-959c-4c3a-9a59-234a2a520de0)]
 interface nsITimedChannel : nsISupports {
   // Set this attribute to true to enable collection of timing data.
   // channelCreationTime will be available even with this attribute set to
   // false.
   attribute boolean timingEnabled;
 
   // The number of redirects
-  attribute uint16_t redirectCount;
+  attribute uint8_t redirectCount;
+  attribute uint8_t internalRedirectCount;
 
   [noscript] readonly attribute TimeStamp channelCreation;
   [noscript] readonly attribute TimeStamp asyncOpen;
 
   // The following are only set when the request is intercepted by a service
   // worker no matter the response is synthesized.
   [noscript] attribute TimeStamp launchServiceWorkerStart;
   [noscript] attribute TimeStamp launchServiceWorkerEnd;
--- a/netwerk/protocol/http/HttpBaseChannel.cpp
+++ b/netwerk/protocol/http/HttpBaseChannel.cpp
@@ -188,16 +188,17 @@ HttpBaseChannel::HttpBaseChannel()
   , mTlsFlags(0)
   , mSuspendCount(0)
   , mInitialRwin(0)
   , mProxyResolveFlags(0)
   , mContentDispositionHint(UINT32_MAX)
   , mHttpHandler(gHttpHandler)
   , mReferrerPolicy(NS_GetDefaultReferrerPolicy())
   , mRedirectCount(0)
+  , mInternalRedirectCount(0)
   , mForcePending(false)
   , mCorsIncludeCredentials(false)
   , mCorsMode(nsIHttpChannelInternal::CORS_MODE_NO_CORS)
   , mRedirectMode(nsIHttpChannelInternal::REDIRECT_MODE_FOLLOW)
   , mFetchCacheMode(nsIHttpChannelInternal::FETCH_CACHE_MODE_DEFAULT)
   , mOnStartRequestCalled(false)
   , mOnStopRequestCalled(false)
   , mAfterOnStartRequestBegun(false)
@@ -3483,23 +3484,16 @@ HttpBaseChannel::SetupReplacementChannel
   // convey the referrer if one was used for this channel to the next one
   if (mReferrer) {
     rv = httpChannel->SetReferrerWithPolicy(mReferrer, mReferrerPolicy);
     MOZ_ASSERT(NS_SUCCEEDED(rv));
   }
   // convey the mAllowSTS flags
   rv = httpChannel->SetAllowSTS(mAllowSTS);
   MOZ_ASSERT(NS_SUCCEEDED(rv));
-  // convey the new redirection limit
-  // make sure we don't underflow
-  uint32_t redirectionLimit = mRedirectionLimit
-    ? mRedirectionLimit - 1
-    : 0;
-  rv = httpChannel->SetRedirectionLimit(redirectionLimit);
-  MOZ_ASSERT(NS_SUCCEEDED(rv));
 
   // convey the Accept header value
   {
     nsAutoCString oldAcceptValue;
     nsresult hasHeader = mRequestHead.GetHeader(nsHttp::Accept, oldAcceptValue);
     if (NS_SUCCEEDED(hasHeader)) {
       rv = httpChannel->SetRequestHeader(NS_LITERAL_CSTRING("Accept"),
                                          oldAcceptValue,
@@ -3597,33 +3591,50 @@ HttpBaseChannel::SetupReplacementChannel
   }
 
   // Transfer the timing data (if we are dealing with an nsITimedChannel).
   nsCOMPtr<nsITimedChannel> newTimedChannel(do_QueryInterface(newChannel));
   nsCOMPtr<nsITimedChannel> oldTimedChannel(
       do_QueryInterface(static_cast<nsIHttpChannel*>(this)));
   if (oldTimedChannel && newTimedChannel) {
     newTimedChannel->SetTimingEnabled(mTimingEnabled);
-    newTimedChannel->SetRedirectCount(mRedirectCount + 1);
+
+    if (redirectFlags & nsIChannelEventSink::REDIRECT_INTERNAL) {
+      int8_t newCount = mInternalRedirectCount + 1;
+      newTimedChannel->SetInternalRedirectCount(
+        std::max(newCount, mInternalRedirectCount));
+    } else {
+      int8_t newCount = mRedirectCount + 1;
+      newTimedChannel->SetRedirectCount(
+        std::max(newCount, mRedirectCount));
+    }
 
     // If the RedirectStart is null, we will use the AsyncOpen value of the
     // previous channel (this is the first redirect in the redirects chain).
     if (mRedirectStartTimeStamp.IsNull()) {
-      TimeStamp asyncOpen;
-      oldTimedChannel->GetAsyncOpen(&asyncOpen);
-      newTimedChannel->SetRedirectStart(asyncOpen);
-    }
-    else {
+      // Only do this for real redirects.  Internal redirects should be hidden.
+      if (!(redirectFlags & nsIChannelEventSink::REDIRECT_INTERNAL)) {
+        TimeStamp asyncOpen;
+        oldTimedChannel->GetAsyncOpen(&asyncOpen);
+        newTimedChannel->SetRedirectStart(asyncOpen);
+      }
+    } else {
       newTimedChannel->SetRedirectStart(mRedirectStartTimeStamp);
     }
 
-    // The RedirectEnd timestamp is equal to the previous channel response end.
-    TimeStamp prevResponseEnd;
-    oldTimedChannel->GetResponseEnd(&prevResponseEnd);
-    newTimedChannel->SetRedirectEnd(prevResponseEnd);
+    // For internal redirects just propagate the last redirect end time
+    // forward.  Otherwise the new redirect end time is the last response
+    // end time.
+    TimeStamp newRedirectEnd;
+    if (redirectFlags & nsIChannelEventSink::REDIRECT_INTERNAL) {
+      oldTimedChannel->GetRedirectEnd(&newRedirectEnd);
+    } else {
+      oldTimedChannel->GetResponseEnd(&newRedirectEnd);
+    }
+    newTimedChannel->SetRedirectEnd(newRedirectEnd);
 
     nsAutoString initiatorType;
     oldTimedChannel->GetInitiatorType(initiatorType);
     newTimedChannel->SetInitiatorType(initiatorType);
 
     // Check whether or not this was a cross-domain redirect.
     newTimedChannel->SetAllRedirectsSameOrigin(
         mAllRedirectsSameOrigin && SameOriginWithOriginalUri(newURI));
@@ -3736,30 +3747,44 @@ HttpBaseChannel::GetAsyncOpen(TimeStamp*
   return NS_OK;
 }
 
 /**
  * @return the number of redirects. There is no check for cross-domain
  * redirects. This check must be done by the consumers.
  */
 NS_IMETHODIMP
-HttpBaseChannel::GetRedirectCount(uint16_t *aRedirectCount)
+HttpBaseChannel::GetRedirectCount(uint8_t *aRedirectCount)
 {
   *aRedirectCount = mRedirectCount;
   return NS_OK;
 }
 
 NS_IMETHODIMP
-HttpBaseChannel::SetRedirectCount(uint16_t aRedirectCount)
+HttpBaseChannel::SetRedirectCount(uint8_t aRedirectCount)
 {
   mRedirectCount = aRedirectCount;
   return NS_OK;
 }
 
 NS_IMETHODIMP
+HttpBaseChannel::GetInternalRedirectCount(uint8_t *aRedirectCount)
+{
+  *aRedirectCount = mInternalRedirectCount;
+  return NS_OK;
+}
+
+NS_IMETHODIMP
+HttpBaseChannel::SetInternalRedirectCount(uint8_t aRedirectCount)
+{
+  mInternalRedirectCount = aRedirectCount;
+  return NS_OK;
+}
+
+NS_IMETHODIMP
 HttpBaseChannel::GetRedirectStart(TimeStamp* _retval)
 {
   *_retval = mRedirectStartTimeStamp;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 HttpBaseChannel::SetRedirectStart(TimeStamp aRedirectStart)
--- a/netwerk/protocol/http/HttpBaseChannel.h
+++ b/netwerk/protocol/http/HttpBaseChannel.h
@@ -566,17 +566,19 @@ protected:
 
   uint32_t                          mReferrerPolicy;
 
   // Performance tracking
   // The initiator type (for this resource) - how was the resource referenced in
   // the HTML file.
   nsString                          mInitiatorType;
   // Number of redirects that has occurred.
-  int16_t                           mRedirectCount;
+  int8_t                            mRedirectCount;
+  // Number of internal redirects that has occurred.
+  int8_t                            mInternalRedirectCount;
   // A time value equal to the starting time of the fetch that initiates the
   // redirect.
   mozilla::TimeStamp                mRedirectStartTimeStamp;
   // A time value equal to the time immediately after receiving the last byte of
   // the response of the last redirect.
   mozilla::TimeStamp                mRedirectEndTimeStamp;
 
   PRTime                            mChannelCreationTime;
--- a/netwerk/protocol/http/HttpChannelParent.cpp
+++ b/netwerk/protocol/http/HttpChannelParent.cpp
@@ -1468,17 +1468,17 @@ HttpChannelParent::OnStartRequest(nsIReq
   mCacheEntry = do_QueryInterface(cacheEntry);
 
   nsresult channelStatus = NS_OK;
   chan->GetStatus(&channelStatus);
 
   nsCString secInfoSerialization;
   UpdateAndSerializeSecurityInfo(secInfoSerialization);
 
-  uint16_t redirectCount = 0;
+  uint8_t redirectCount = 0;
   chan->GetRedirectCount(&redirectCount);
 
   nsCOMPtr<nsISupports> cacheKey;
   chan->GetCacheKey(getter_AddRefs(cacheKey));
   uint32_t cacheKeyValue = 0;
   if (cacheKey) {
     nsCOMPtr<nsISupportsPRUint32> container = do_QueryInterface(cacheKey);
     if (!container) {
--- a/netwerk/protocol/http/NullHttpChannel.cpp
+++ b/netwerk/protocol/http/NullHttpChannel.cpp
@@ -559,23 +559,35 @@ NullHttpChannel::GetTimingEnabled(bool *
 
 NS_IMETHODIMP
 NullHttpChannel::SetTimingEnabled(bool aTimingEnabled)
 {
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
 NS_IMETHODIMP
-NullHttpChannel::GetRedirectCount(uint16_t *aRedirectCount)
+NullHttpChannel::GetRedirectCount(uint8_t *aRedirectCount)
 {
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
 NS_IMETHODIMP
-NullHttpChannel::SetRedirectCount(uint16_t aRedirectCount)
+NullHttpChannel::SetRedirectCount(uint8_t aRedirectCount)
+{
+  return NS_ERROR_NOT_IMPLEMENTED;
+}
+
+NS_IMETHODIMP
+NullHttpChannel::GetInternalRedirectCount(uint8_t *aRedirectCount)
+{
+  return NS_ERROR_NOT_IMPLEMENTED;
+}
+
+NS_IMETHODIMP
+NullHttpChannel::SetInternalRedirectCount(uint8_t aRedirectCount)
 {
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
 NS_IMETHODIMP
 NullHttpChannel::GetChannelCreation(mozilla::TimeStamp *aChannelCreation)
 {
   *aChannelCreation = mChannelCreationTimestamp;
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -5691,17 +5691,17 @@ nsHttpChannel::AsyncProcessRedirection(u
     if (NS_FAILED(mResponseHead->GetHeader(nsHttp::Location, location)))
         return NS_ERROR_FAILURE;
 
     // make sure non-ASCII characters in the location header are escaped.
     nsAutoCString locationBuf;
     if (NS_EscapeURL(location.get(), -1, esc_OnlyNonASCII, locationBuf))
         location = locationBuf;
 
-    if (mRedirectionLimit == 0) {
+    if (mRedirectCount >= mRedirectionLimit || mInternalRedirectCount >= mRedirectionLimit) {
         LOG(("redirection limit reached!\n"));
         return NS_ERROR_REDIRECT_LOOP;
     }
 
     mRedirectType = redirectType;
 
     LOG(("redirecting to: %s [redirection-limit=%u]\n",
         location.get(), uint32_t(mRedirectionLimit)));