Nix HpackVarintDecoder::Max{ExtensionBytes,Offset}

The motivation is to prepare HpackVarintDecoder to conditionally support
62 bit integers as required by the QPACK specs.  This will of course
have to be flag protected.  HpackVarintDecoder::MaxExtensionBytes() and
MaxOffset() would need to return different values depending on the value
of the flag.  It's a lot easier to get rid of them instead.

In HpackVarintDecoder::Resume(), use a locally defined const instead of
MaxOffset().

In MarkDone() et al., use numeric_limits::max() instead of |MaxOffset()
+ 7|.  This value is so large that it does not matter if
HpackVarintDecoder is in up-to-28-bit-mode or up-to-62-bit-mode.  Avoid
using a method to cut down on clutter in release builds.

In HpackVarintDecoderTest.ValueTooLarge, hardcode the large value.  It
does not hurt to reduce logic in tests anyway.

This CL lands server change 214703557 by bnc.

BUG=488484

Change-Id: Ie12bfda99b478549bcc5b126be54e51804c0d85c
Reviewed-on: https://chromium-review.googlesource.com/c/1291592
Commit-Queue: Dianna Hu <diannahu@chromium.org>
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601269}

net/third_party/http2/hpack/varint/hpack_varint_decoder.cc

@@ -42,13 +42,14 @@ DecodeStatus HpackVarintDecoder::StartExtended(uint8_t prefix_length,
 }
 
 DecodeStatus HpackVarintDecoder::Resume(DecodeBuffer* db) {
+  const uint32_t kMaxOffset = 28;
   CheckNotDone();
   do {
     if (db->Empty()) {
       return DecodeStatus::kDecodeInProgress;
     }
     uint8_t byte = db->DecodeUInt8();
-    if (offset_ == MaxOffset() && byte != 0)
+    if (offset_ == kMaxOffset && byte != 0)
       break;
     value_ += (byte & 0x7f) << offset_;
     if ((byte & 0x80) == 0) {
@@ -56,7 +57,7 @@ DecodeStatus HpackVarintDecoder::Resume(DecodeBuffer* db) {
       return DecodeStatus::kDecodeDone;
     }
     offset_ += 7;
-  } while (offset_ <= MaxOffset());
+  } while (offset_ <= kMaxOffset);
   DLOG(WARNING) << "Variable length int encoding is too large or too long. "
                 << DebugString();
   MarkDone();

net/third_party/http2/hpack/varint/hpack_varint_decoder.h

@@ -19,6 +19,8 @@
 // For details of the encoding, see:
 //        http://httpwg.org/specs/rfc7541.html#integer.representation
 //
+// The decoder supports decoding integers up to 2^28 + 2^prefix_length - 2.
+//
 // TODO(jamessynge): Consider dropping support for encodings of more than 4
 // bytes, including the prefix byte, as in practice we only see at most 3 bytes,
 // and 4 bytes would cover any desire to support large (but not ridiculously
@@ -28,6 +30,7 @@
 #define NET_THIRD_PARTY_HTTP2_HPACK_VARINT_HPACK_VARINT_DECODER_H_
 
 #include <cstdint>
+#include <limits>
 
 #include "base/logging.h"
 #include "net/third_party/http2/decoder/decode_buffer.h"
@@ -84,30 +87,23 @@ class HTTP2_EXPORT_PRIVATE HpackVarintDecoder {
   DecodeStatus StartExtendedForTest(uint8_t prefix_length, DecodeBuffer* db);
   DecodeStatus ResumeForTest(DecodeBuffer* db);
 
-  static constexpr uint32_t MaxExtensionBytes() { return 5; }
-
  private:
   // Protection in case Resume is called when it shouldn't be.
   void MarkDone() {
 #ifndef NDEBUG
-    // We support up to 5 extension bytes, so offset_ should never be > 28 when
-    // it makes sense to call Resume().
-    offset_ = MaxOffset() + 7;
+    offset_ = std::numeric_limits<uint32_t>::max();
 #endif
   }
   void CheckNotDone() const {
 #ifndef NDEBUG
-    DCHECK_LE(offset_, MaxOffset());
+    DCHECK_NE(std::numeric_limits<uint32_t>::max(), offset_);
 #endif
   }
   void CheckDone() const {
 #ifndef NDEBUG
-    DCHECK_GT(offset_, MaxOffset());
+    DCHECK_EQ(std::numeric_limits<uint32_t>::max(), offset_);
 #endif
   }
-  static constexpr uint32_t MaxOffset() {
-    return 7 * (MaxExtensionBytes() - 1);
-  }
 
   // These fields are initialized just to keep ASAN happy about reading
   // them from DebugString().

net/third_party/http2/hpack/varint/hpack_varint_round_trip_test.cc

@@ -336,12 +336,11 @@ TEST_F(HpackVarintRoundTripTest, ValidateFourExtensionBytes) {
   }
 }
 
-// Test *some* values that require too many extension bytes.
+// Test the value one larger than the largest that can be decoded.
 TEST_F(HpackVarintRoundTripTest, ValueTooLarge) {
-  const uint32_t expected_offset = HpackVarintDecoder::MaxExtensionBytes() + 1;
   for (prefix_length_ = 3; prefix_length_ <= 7; ++prefix_length_) {
-    uint64_t too_large = HiValueOfExtensionBytes(
-        HpackVarintDecoder::MaxExtensionBytes() + 3, prefix_length_);
+    const uint64_t too_large = (1 << 28) + (1 << prefix_length_) - 1;
+    const uint32_t expected_offset = 6;
     HpackBlockBuilder bb;
     bb.AppendHighBitsAndVarint(0, prefix_length_, too_large);
     buffer_ = bb.buffer();