Bug 1300148 - Fix handling of HPACK dynamic size update r=mcmanus
authorNicholas Hurley <hurley@todesschaf.org>
Fri, 02 Sep 2016 10:50:00 -0700
changeset 354134 262bcc5637ed0c886844c23013afb53e77d8544f
parent 354105 4ee5ddeeee281b9b6f564fc56efb1713da8d7eac
child 354135 5ee0ea599ba3a80cc14da0dc80a235d767c8e7c3
push id6570
push userraliiev@mozilla.com
push dateMon, 14 Nov 2016 12:26:13 +0000
treeherdermozilla-beta@f455459b2ae5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmcmanus
bugs1300148
milestone51.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 1300148 - Fix handling of HPACK dynamic size update r=mcmanus MozReview-Commit-ID: 46n3gWvyW0P
netwerk/protocol/http/Http2Compression.cpp
netwerk/protocol/http/Http2Compression.h
netwerk/protocol/http/Http2Session.cpp
--- a/netwerk/protocol/http/Http2Compression.cpp
+++ b/netwerk/protocol/http/Http2Compression.cpp
@@ -275,16 +275,17 @@ nvFIFO::operator[] (size_t index) const
     return static_cast<nvPair *>(mTable.ObjectAt(index - gStaticHeaders->GetSize()));
   }
   return static_cast<nvPair *>(gStaticHeaders->ObjectAt(index));
 }
 
 Http2BaseCompressor::Http2BaseCompressor()
   : mOutput(nullptr)
   , mMaxBuffer(kDefaultMaxBuffer)
+  , mMaxBufferSetting(kDefaultMaxBuffer)
 {
   mDynamicReporter = new HpackDynamicTableReporter(this);
   RegisterStrongMemoryReporter(mDynamicReporter);
 }
 
 Http2BaseCompressor::~Http2BaseCompressor()
 {
   UnregisterStrongMemoryReporter(mDynamicReporter);
@@ -337,16 +338,33 @@ Http2BaseCompressor::DumpState()
   for (i = 0; i < length; ++i) {
     const nvPair *pair = mHeaderTable[i];
     // NWGH - make this <= staticLength
     LOG(("%sindex %u: %s %s", i < staticLength ? "static " : "", i,
          pair->mName.get(), pair->mValue.get()));
   }
 }
 
+void
+Http2BaseCompressor::SetMaxBufferSizeInternal(uint32_t maxBufferSize)
+{
+  MOZ_ASSERT(maxBufferSize <= mMaxBufferSetting);
+
+  uint32_t removedCount = 0;
+
+  LOG(("Http2BaseCompressor::SetMaxBufferSizeInternal %u called", maxBufferSize));
+
+  while (mHeaderTable.VariableLength() && (mHeaderTable.ByteCount() > maxBufferSize)) {
+    mHeaderTable.RemoveElement();
+    ++removedCount;
+  }
+
+  mMaxBuffer = maxBufferSize;
+}
+
 nsresult
 Http2Decompressor::DecodeHeaderBlock(const uint8_t *data, uint32_t datalen,
                                      nsACString &output, bool isPush)
 {
   mOffset = 0;
   mData = data;
   mDataLen = datalen;
   mOutput = &output;
@@ -944,24 +962,40 @@ Http2Decompressor::DoLiteralNeverIndexed
 }
 
 nsresult
 Http2Decompressor::DoContextUpdate()
 {
   // This starts with 001 bit pattern
   MOZ_ASSERT((mData[mOffset] & 0xE0) == 0x20);
 
-  // Getting here means we have to adjust the max table size
+  // Getting here means we have to adjust the max table size, because the
+  // compressor on the other end has signaled to us through HPACK (not H2)
+  // that it's using a size different from the currently-negotiated size.
+  // This change could either come about because we've sent a
+  // SETTINGS_HEADER_TABLE_SIZE, or because the encoder has decided that
+  // the current negotiated size doesn't fit its needs (for whatever reason)
+  // and so it needs to change it (either up to the max allowed by our SETTING,
+  // or down to some value below that)
   uint32_t newMaxSize;
   nsresult rv = DecodeInteger(5, newMaxSize);
   LOG(("Http2Decompressor::DoContextUpdate new maximum size %u", newMaxSize));
   if (NS_FAILED(rv)) {
     return rv;
   }
-  return mCompressor->SetMaxBufferSizeInternal(newMaxSize);
+
+  if (newMaxSize > mMaxBufferSetting) {
+    // This is fatal to the session - peer is trying to use a table larger
+    // than we have made available.
+    return NS_ERROR_FAILURE;
+  }
+
+  SetMaxBufferSizeInternal(newMaxSize);
+
+  return NS_OK;
 }
 
 /////////////////////////////////////////////////////////////////
 
 nsresult
 Http2Compressor::EncodeHeaderBlock(const nsCString &nvInput,
                                    const nsACString &method, const nsACString &path,
                                    const nsACString &host, const nsACString &scheme,
@@ -1354,31 +1388,10 @@ Http2Compressor::SetMaxBufferSize(uint32
   if (!mBufferSizeChangeWaiting) {
     mBufferSizeChangeWaiting = true;
     mLowestBufferSizeWaiting = maxBufferSize;
   } else if (maxBufferSize < mLowestBufferSizeWaiting) {
     mLowestBufferSizeWaiting = maxBufferSize;
   }
 }
 
-nsresult
-Http2Compressor::SetMaxBufferSizeInternal(uint32_t maxBufferSize)
-{
-  if (maxBufferSize > mMaxBufferSetting) {
-    return NS_ERROR_FAILURE;
-  }
-
-  uint32_t removedCount = 0;
-
-  LOG(("Http2Compressor::SetMaxBufferSizeInternal %u called", maxBufferSize));
-
-  while (mHeaderTable.VariableLength() && (mHeaderTable.ByteCount() > maxBufferSize)) {
-    mHeaderTable.RemoveElement();
-    ++removedCount;
-  }
-
-  mMaxBuffer = maxBufferSize;
-
-  return NS_OK;
-}
-
 } // namespace net
 } // namespace mozilla
--- a/netwerk/protocol/http/Http2Compression.h
+++ b/netwerk/protocol/http/Http2Compression.h
@@ -67,21 +67,23 @@ public:
   size_t SizeOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf) const;
 
 protected:
   const static uint32_t kDefaultMaxBuffer = 4096;
 
   virtual void ClearHeaderTable();
   virtual void MakeRoom(uint32_t amount, const char *direction);
   virtual void DumpState();
+  virtual void SetMaxBufferSizeInternal(uint32_t maxBufferSize);
 
   nsACString *mOutput;
   nvFIFO mHeaderTable;
 
   uint32_t mMaxBuffer;
+  uint32_t mMaxBufferSetting;
 
 private:
   RefPtr<HpackDynamicTableReporter> mDynamicReporter;
 };
 
 class Http2Compressor;
 
 class Http2Decompressor final : public Http2BaseCompressor
@@ -94,17 +96,16 @@ public:
   nsresult DecodeHeaderBlock(const uint8_t *data, uint32_t datalen,
                              nsACString &output, bool isPush);
 
   void GetStatus(nsACString &hdr) { hdr = mHeaderStatus; }
   void GetHost(nsACString &hdr) { hdr = mHeaderHost; }
   void GetScheme(nsACString &hdr) { hdr = mHeaderScheme; }
   void GetPath(nsACString &hdr) { hdr = mHeaderPath; }
   void GetMethod(nsACString &hdr) { hdr = mHeaderMethod; }
-  void SetCompressor(Http2Compressor *compressor) { mCompressor = compressor; }
 
 private:
   nsresult DoIndexed();
   nsresult DoLiteralWithoutIndex();
   nsresult DoLiteralWithIncremental();
   nsresult DoLiteralInternal(nsACString &, nsACString &, uint32_t);
   nsresult DoLiteralNeverIndexed();
   nsresult DoContextUpdate();
@@ -117,18 +118,16 @@ private:
   nsresult CopyStringFromInput(uint32_t index, nsACString &val);
   uint8_t ExtractByte(uint8_t bitsLeft, uint32_t &bytesConsumed);
   nsresult CopyHuffmanStringFromInput(uint32_t index, nsACString &val);
   nsresult DecodeHuffmanCharacter(const HuffmanIncomingTable *table, uint8_t &c,
                                   uint32_t &bytesConsumed, uint8_t &bitsLeft);
   nsresult DecodeFinalHuffmanCharacter(const HuffmanIncomingTable *table,
                                        uint8_t &c, uint8_t &bitsLeft);
 
-  Http2Compressor *mCompressor;
-
   nsCString mHeaderStatus;
   nsCString mHeaderHost;
   nsCString mHeaderScheme;
   nsCString mHeaderPath;
   nsCString mHeaderMethod;
 
   // state variables when DecodeBlock() is on the stack
   uint32_t mOffset;
@@ -138,33 +137,31 @@ private:
   bool mIsPush;
 };
 
 
 class Http2Compressor final : public Http2BaseCompressor
 {
 public:
   Http2Compressor() : mParsedContentLength(-1),
-                      mMaxBufferSetting(kDefaultMaxBuffer),
                       mBufferSizeChangeWaiting(false),
                       mLowestBufferSizeWaiting(0)
   { };
   virtual ~Http2Compressor() { }
 
   // HTTP/1 formatted header block as input - HTTP/2 formatted
   // header block as output
   nsresult EncodeHeaderBlock(const nsCString &nvInput,
                              const nsACString &method, const nsACString &path,
                              const nsACString &host, const nsACString &scheme,
                              bool connectForm, nsACString &output);
 
   int64_t GetParsedContentLength() { return mParsedContentLength; } // -1 on not found
 
   void SetMaxBufferSize(uint32_t maxBufferSize);
-  nsresult SetMaxBufferSizeInternal(uint32_t maxBufferSize);
 
 private:
   enum outputCode {
     kNeverIndexedLiteral,
     kPlainLiteral,
     kIndexedLiteral,
     kIndex
   };
@@ -173,17 +170,16 @@ private:
                 const class nvPair *pair, uint32_t index);
   void EncodeInteger(uint32_t prefixLen, uint32_t val);
   void ProcessHeader(const nvPair inputPair, bool noLocalIndex,
                      bool neverIndex);
   void HuffmanAppend(const nsCString &value);
   void EncodeTableSizeChange(uint32_t newMaxSize);
 
   int64_t mParsedContentLength;
-  uint32_t mMaxBufferSetting;
   bool mBufferSizeChangeWaiting;
   uint32_t mLowestBufferSizeWaiting;
 };
 
 } // namespace net
 } // namespace mozilla
 
 #endif // mozilla_net_Http2Compression_Internal_h
--- a/netwerk/protocol/http/Http2Session.cpp
+++ b/netwerk/protocol/http/Http2Session.cpp
@@ -118,17 +118,16 @@ Http2Session::Http2Session(nsISocketTran
   static uint64_t sSerial;
   mSerial = ++sSerial;
 
   LOG3(("Http2Session::Http2Session %p serial=0x%X\n", this, mSerial));
 
   mInputFrameBuffer = MakeUnique<char[]>(mInputFrameBufferSize);
   mOutputQueueBuffer = MakeUnique<char[]>(mOutputQueueSize);
   mDecompressBuffer.SetCapacity(kDefaultBufferSize);
-  mDecompressor.SetCompressor(&mCompressor);
 
   mPushAllowance = gHttpHandler->SpdyPushAllowance();
   mInitialRwin = std::max(gHttpHandler->SpdyPullAllowance(), mPushAllowance);
   mMaxConcurrent = gHttpHandler->DefaultSpdyConcurrent();
   mSendingChunkSize = gHttpHandler->SpdySendingChunkSize();
   SendHello();
 
   mLastDataReadEpoch = mLastReadEpoch;