Bug 1645901 - handle cookie setting in child process for Set-Cookie response, r=baku,mayhemer,necko-reviewers
authorJunior Hsu <juhsu@mozilla.com>
Tue, 07 Jul 2020 15:19:09 +0000
changeset 539258 14420a624d0a6b882e6ef06e717e2231d4b437a2
parent 539257 8417742ed987b40fcbeef0881c80556380443d6d
child 539259 4208ed5fc3b02fddc3624805ff3937a68a4aa4f6
push id121019
push userjuhsu@mozilla.com
push dateWed, 08 Jul 2020 02:30:41 +0000
treeherderautoland@14420a624d0a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbaku, mayhemer, necko-reviewers
bugs1645901
milestone80.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 1645901 - handle cookie setting in child process for Set-Cookie response, r=baku,mayhemer,necko-reviewers Differential Revision: https://phabricator.services.mozilla.com/D82130
netwerk/protocol/http/HttpChannelChild.cpp
netwerk/protocol/http/HttpChannelParams.ipdlh
netwerk/protocol/http/HttpChannelParent.cpp
netwerk/protocol/http/HttpChannelParent.h
netwerk/protocol/http/nsHttpChannel.cpp
--- a/netwerk/protocol/http/HttpChannelChild.cpp
+++ b/netwerk/protocol/http/HttpChannelChild.cpp
@@ -542,16 +542,20 @@ void HttpChannelChild::OnStartRequest(
     // arguments passed in
     // nsHttpChannel::ReEvaluateReferrerAfterTrackingStatusIsKnown(), except for
     // aRespectBeforeConnect which we pass false here since we're intentionally
     // overriding the referrer after BeginConnect().
     Unused << SetReferrerInfoInternal(aArgs.overrideReferrerInfo(), false, true,
                                       false);
   }
 
+  if (!aArgs.cookie().IsEmpty()) {
+    SetCookie(aArgs.cookie());
+  }
+
   if (aArgs.shouldWaitForOnStartRequestSent() &&
       !mRecvOnStartRequestSentCalled) {
     LOG(("  > pending DoOnStartRequest until RecvOnStartRequestSent\n"));
     MOZ_ASSERT(NS_IsMainThread());
 
     mEventQ->Suspend();
     mSuspendedByWaitingForPermissionCookie = true;
     mEventQ->PrependEvent(MakeUnique<NeckoTargetChannelFunctionEvent>(
--- a/netwerk/protocol/http/HttpChannelParams.ipdlh
+++ b/netwerk/protocol/http/HttpChannelParams.ipdlh
@@ -44,12 +44,13 @@ struct HttpChannelOnStartRequestArgs
   bool allRedirectsSameOrigin;
   uint32_t? multiPartID;
   bool isLastPartOfMultiPart;
   CrossOriginOpenerPolicy openerPolicy;
   nsCString appCacheGroupId;
   nsCString appCacheClientId;
   nsIReferrerInfo overrideReferrerInfo;
   bool shouldWaitForOnStartRequestSent;
+  nsCString cookie;
 };
 
 } // namespace ipc
 } // namespace mozilla
--- a/netwerk/protocol/http/HttpChannelParent.cpp
+++ b/netwerk/protocol/http/HttpChannelParent.cpp
@@ -1518,16 +1518,19 @@ HttpChannelParent::OnStartRequest(nsIReq
   chan->GetAllRedirectsSameOrigin(&args.allRedirectsSameOrigin());
   chan->GetCrossOriginOpenerPolicy(&args.openerPolicy());
   args.selfAddr() = chan->GetSelfAddr();
   args.peerAddr() = chan->GetPeerAddr();
   args.timing() = GetTimingAttributes(mChannel);
   if (mOverrideReferrerInfo) {
     args.overrideReferrerInfo() = ToRefPtr(std::move(mOverrideReferrerInfo));
   }
+  if (!mCookie.IsEmpty()) {
+    args.cookie() = std::move(mCookie);
+  }
 
   nsHttpRequestHead* requestHead = chan->GetRequestHead();
   // !!! We need to lock headers and please don't forget to unlock them !!!
   requestHead->Enter();
 
   nsHttpHeaderArray cleanedUpRequestHeaders;
   bool cleanedUpRequest = false;
   if (requestHead->HasHeader(nsHttp::Cookie)) {
@@ -2681,10 +2684,17 @@ auto HttpChannelParent::AttachStreamFilt
   // If IPC channel is open, background channel should be ready to send
   // SendAttachStreamFilter.
   MOZ_ASSERT(mBgParent);
   return InvokeAsync(mBgParent->GetBackgroundTarget(), mBgParent.get(),
                      __func__, &HttpBackgroundChannelParent::AttachStreamFilter,
                      std::move(aParentEndpoint), std::move(aChildEndpoint));
 }
 
+void HttpChannelParent::SetCookie(nsCString&& aCookie) {
+  LOG(("HttpChannelParent::SetCookie [this=%p]", this));
+  MOZ_ASSERT(!mAfterOnStartRequestBegun);
+  MOZ_ASSERT(mCookie.IsEmpty());
+  mCookie = std::move(aCookie);
+}
+
 }  // namespace net
 }  // namespace mozilla
--- a/netwerk/protocol/http/HttpChannelParent.h
+++ b/netwerk/protocol/http/HttpChannelParent.h
@@ -129,16 +129,25 @@ class HttpChannelParent final : public n
   void OnBackgroundParentDestroyed();
 
   base::ProcessId OtherPid() const;
 
   // Inform the child actor that our referrer info was modified late during
   // BeginConnect.
   void OverrideReferrerInfoDuringBeginConnect(nsIReferrerInfo* aReferrerInfo);
 
+  // Set the cookie string, which will be informed to the child actor during
+  // PHttpBackgroundChannel::OnStartRequest. Note that CookieService also sends
+  // the information to all actors via PContent, a main thread IPC, which could
+  // be slower than background IPC PHttpBackgroundChannel::OnStartRequest.
+  // Therefore, another cookie notification via PBackground is needed to
+  // guarantee the listener in child has the necessary cookies before
+  // OnStartRequest.
+  void SetCookie(nsCString&& aCookie);
+
   using ChildEndpointPromise =
       MozPromise<ipc::Endpoint<extensions::PStreamFilterChild>, bool, true>;
   [[nodiscard]] RefPtr<ChildEndpointPromise> AttachStreamFilter(
       Endpoint<extensions::PStreamFilterParent>&& aParentEndpoint,
       Endpoint<extensions::PStreamFilterChild>&& aChildEndpoint);
 
  protected:
   // used to connect redirected-to channel in parent with just created
@@ -321,16 +330,20 @@ class HttpChannelParent final : public n
 
   // Set to the canceled status value if the main channel was canceled.
   nsresult mStatus;
 
   // The referrer info, set during nsHttpChannel::BeginConnect, to override the
   // original one. This info will be sent in OnStartRequest.
   nsCOMPtr<nsIReferrerInfo> mOverrideReferrerInfo;
 
+  // The cookie string in Set-Cookie header. This info will be sent in
+  // OnStartRequest.
+  nsCString mCookie;
+
   // OnStatus is always called before OnProgress.
   // Set true in OnStatus if next OnProgress can be ignored
   // since the information can be recontructed from ODA.
   uint8_t mIgnoreProgress : 1;
 
   uint8_t mSentRedirect1BeginFailed : 1;
   uint8_t mReceivedRedirect2Verify : 1;
   uint8_t mHasSuspendedByBackPressure : 1;
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -2597,19 +2597,25 @@ nsresult nsHttpChannel::ContinueProcessR
 
   uint32_t httpStatus = mResponseHead->Status();
 
   // STS, Cookies and Alt-Service should not be handled on proxy failure.
   // If proxy CONNECT response needs to complete, wait to process connection
   // for Strict-Transport-Security.
   if (!(mTransaction && mTransaction->ProxyConnectFailed()) &&
       (httpStatus != 407)) {
-    nsAutoCString cookie;
-    if (NS_SUCCEEDED(mResponseHead->GetHeader(nsHttp::Set_Cookie, cookie))) {
+    if (nsAutoCString cookie;
+        NS_SUCCEEDED(mResponseHead->GetHeader(nsHttp::Set_Cookie, cookie))) {
       SetCookie(cookie);
+      nsCOMPtr<nsIParentChannel> parentChannel;
+      NS_QueryNotificationCallbacks(this, parentChannel);
+      if (RefPtr<HttpChannelParent> httpParent =
+              do_QueryObject(parentChannel)) {
+        httpParent->SetCookie(std::move(cookie));
+      }
     }
 
     // Given a successful connection, process any STS or PKP data that's
     // relevant.
     DebugOnly<nsresult> rv = ProcessSecurityHeaders();
     MOZ_ASSERT(NS_SUCCEEDED(rv), "ProcessSTSHeader failed, continuing load.");
 
     if ((httpStatus < 500) && (httpStatus != 421)) {