Bug 1436610 - Send "TE: Trailers" header on h2 requests. r=mcmanus
authorNicholas Hurley <hurley@mozilla.com>
Thu, 22 Feb 2018 10:57:01 -0800
changeset 471861 a17f755742df099e2fa64024b57bfcae77bacb24
parent 471860 c10e876269a946c970da95bbd9f8063f3fd5f364
child 471862 92ba1ca3bc76c0ddf6535c8687786c58b42777f1
child 471940 80cabc246b5fc247f04be3ba172bde254f509443
push id9374
push userjlund@mozilla.com
push dateMon, 18 Jun 2018 21:43:20 +0000
treeherdermozilla-beta@160e085dfb0b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmcmanus
bugs1436610
milestone62.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 1436610 - Send "TE: Trailers" header on h2 requests. r=mcmanus MozReview-Commit-ID: 6Hpj5RcCb4I
netwerk/protocol/http/Http2Compression.cpp
netwerk/protocol/http/nsHttpTransaction.cpp
--- a/netwerk/protocol/http/Http2Compression.cpp
+++ b/netwerk/protocol/http/Http2Compression.cpp
@@ -1204,16 +1204,27 @@ Http2Compressor::EncodeHeaderBlock(const
       }
     } else {
       // allow indexing of every non-cookie except authorization
       ProcessHeader(nvPair(name, value), false,
                     name.EqualsLiteral("authorization"));
     }
   }
 
+  // NB: This is a *really* ugly hack, but to do this in the right place (the
+  // transaction) would require totally reworking how/when the transaction
+  // creates its request stream, which is not worth the effort and risk of
+  // breakage just to add one header only to h2 connections.
+  if (!connectForm) {
+    // Add in TE: trailers for regular requests
+    nsAutoCString te("te");
+    nsAutoCString trailers("trailers");
+    ProcessHeader(nvPair(te, trailers), false, false);
+  }
+
   mOutput = nullptr;
   LOG(("Compressor state after EncodeHeaderBlock"));
   DumpState();
   return NS_OK;
 }
 
 void
 Http2Compressor::DoOutput(Http2Compressor::outputCode code,
--- a/netwerk/protocol/http/nsHttpTransaction.cpp
+++ b/netwerk/protocol/http/nsHttpTransaction.cpp
@@ -542,16 +542,37 @@ void
 nsHttpTransaction::OnActivated()
 {
     MOZ_ASSERT(OnSocketThread());
 
     if (mActivated) {
         return;
     }
 
+    if (mConnection && mRequestHead) {
+        // So this is fun. On http/2, we want to send TE: Trailers, to be
+        // spec-compliant. So we add it to the request head here. The fun part
+        // is that adding a header to the request head at this point has no
+        // effect on what we send on the wire, as the headers are already
+        // flattened (in Init()) by the time we get here. So the *real* adding
+        // of the header happens in the h2 compression code. We still have to
+        // add the header to the request head here, though, so that devtools can
+        // show that we sent the header. FUN!
+        // Oh, and we can't just check for version >= NS_HTTP_VERSION_2_0 because
+        // right now, mConnection->Version() returns HTTP_VERSION_2 (=5) instead
+        // of NS_HTTP_VERSION_2_0 (=20) for... reasons.
+        bool isOldHttp = (mConnection->Version() == NS_HTTP_VERSION_0_9 ||
+                          mConnection->Version() == NS_HTTP_VERSION_1_0 ||
+                          mConnection->Version() == NS_HTTP_VERSION_1_1 ||
+                          mConnection->Version() == NS_HTTP_VERSION_UNKNOWN);
+        if (!isOldHttp) {
+            Unused << mRequestHead->SetHeader(nsHttp::TE, NS_LITERAL_CSTRING("Trailers"));
+        }
+    }
+
     mActivated = true;
     gHttpHandler->ConnMgr()->AddActiveTransaction(this);
 }
 
 void
 nsHttpTransaction::GetSecurityCallbacks(nsIInterfaceRequestor **cb)
 {
     MutexAutoLock lock(mLock);