Bug 1436610 - Send "TE: Trailers" header on h2 requests. r?mcmanus draft
authorNicholas Hurley <hurley@mozilla.com>
Thu, 22 Feb 2018 10:57:01 -0800
changeset 793115 db0ccfdb2be6d22320f93148c163bb38e6ac0f91
parent 793056 9294f67b3f3bd4a3dd898961148cecd8bfc1ce9c
push id109282
push userbmo:hurley@mozilla.com
push dateWed, 09 May 2018 14:19:03 +0000
reviewersmcmanus
bugs1436610
milestone62.0a1
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);