Commit 30dd84d5 authored by Dianna Hu's avatar Dianna Hu Committed by Commit Bot

Make HpackVarintDecoder support 64-bit integers.

This feature is guarded by --chromium_flag_http2_varint_decode_64_bits.

Before this change, HpackVarintDecoder only decoded integers up to 2^28
+ 2^prefix_length - 2, where prefix_length is between 3 and 7.  Trying
to decode larger integers resulted in an error.  After this change,
HpackVarintDecoder decodes any integer that can be represented on 64
bits (unsigned).  Any input that HpackVarintDecoder decoded before this
change is still decoded.

Decoded integers are used to address entries in the static and dynamic
tables, to specify string lengths, and to update the size limit of the
dynamic table, both in HTTP/2 and in QUIC.  For all of these values
there is already a limit in place that is much lower than 2^28
(otherwise a malicious client could waste hundreds of megabytes of
server memory; and if there is no such limit in place, then we already
have a problem), so making the decoder decode larger values should not
change behavior at higher levels.

To merge this feature, this change also adds an HTTP/2 platform API for
flags,  maintaining parity with SPDY and QUIC (and keeping the
separation between SPDY and HTTP/2 for ease of merging to Chromium).
This change then uses this platform API in the related files referencing
the feature flag and adds a presubmit check.

This change borrows heavily from the pre-existing SPDY and QUIC platform
API for flags.

This CL lands server changes 216463549 and 221087569 by bnc, and
218390247 by diannahu.

BUG=488484

Change-Id: Ie0788f117aba9c6d41fc31148e28c9b49e122602
Reviewed-on: https://chromium-review.googlesource.com/c/1310613Reviewed-by: default avatarBence Béky <bnc@chromium.org>
Commit-Queue: Dianna Hu <diannahu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607349}
parent f27d45e0
...@@ -1206,6 +1206,7 @@ component("net") { ...@@ -1206,6 +1206,7 @@ component("net") {
"third_party/http2/http2_structures.h", "third_party/http2/http2_structures.h",
"third_party/http2/platform/api/http2_arraysize.h", "third_party/http2/platform/api/http2_arraysize.h",
"third_party/http2/platform/api/http2_export.h", "third_party/http2/platform/api/http2_export.h",
"third_party/http2/platform/api/http2_flags.h",
"third_party/http2/platform/api/http2_optional.h", "third_party/http2/platform/api/http2_optional.h",
"third_party/http2/platform/api/http2_ptr_util.h", "third_party/http2/platform/api/http2_ptr_util.h",
"third_party/http2/platform/api/http2_reconstruct_object.h", "third_party/http2/platform/api/http2_reconstruct_object.h",
...@@ -1214,6 +1215,8 @@ component("net") { ...@@ -1214,6 +1215,8 @@ component("net") {
"third_party/http2/platform/api/http2_string_utils.h", "third_party/http2/platform/api/http2_string_utils.h",
"third_party/http2/platform/impl/http2_arraysize_impl.h", "third_party/http2/platform/impl/http2_arraysize_impl.h",
"third_party/http2/platform/impl/http2_export_impl.h", "third_party/http2/platform/impl/http2_export_impl.h",
"third_party/http2/platform/impl/http2_flags_impl.cc",
"third_party/http2/platform/impl/http2_flags_impl.h",
"third_party/http2/platform/impl/http2_optional_impl.h", "third_party/http2/platform/impl/http2_optional_impl.h",
"third_party/http2/platform/impl/http2_ptr_util_impl.h", "third_party/http2/platform/impl/http2_ptr_util_impl.h",
"third_party/http2/platform/impl/http2_reconstruct_object_impl.h", "third_party/http2/platform/impl/http2_reconstruct_object_impl.h",
......
...@@ -29,7 +29,8 @@ void HpackBlockBuilder::AppendHighBitsAndVarint(uint8_t high_bits, ...@@ -29,7 +29,8 @@ void HpackBlockBuilder::AppendHighBitsAndVarint(uint8_t high_bits,
// After the prefix, at most 63 bits can remain to be encoded. // After the prefix, at most 63 bits can remain to be encoded.
// Each octet holds 7 bits, so at most 9 octets are necessary. // Each octet holds 7 bits, so at most 9 octets are necessary.
varint_encoder.ResumeEncoding(/* max_encoded_bytes = */ 9, &buffer_); // TODO(bnc): Move this into a constant in HpackVarintEncoder.
varint_encoder.ResumeEncoding(/* max_encoded_bytes = */ 10, &buffer_);
DCHECK(!varint_encoder.IsEncodingInProgress()); DCHECK(!varint_encoder.IsEncodingInProgress());
} }
......
...@@ -42,7 +42,77 @@ DecodeStatus HpackVarintDecoder::StartExtended(uint8_t prefix_length, ...@@ -42,7 +42,77 @@ DecodeStatus HpackVarintDecoder::StartExtended(uint8_t prefix_length,
} }
DecodeStatus HpackVarintDecoder::Resume(DecodeBuffer* db) { DecodeStatus HpackVarintDecoder::Resume(DecodeBuffer* db) {
const uint32_t kMaxOffset = 28; if (decode_64_bits_) {
// There can be at most 10 continuation bytes. Offset is zero for the
// first one and increases by 7 for each subsequent one.
const uint8_t kMaxOffset = 63;
CheckNotDone();
// Process most extension bytes without the need for overflow checking.
while (offset_ < kMaxOffset) {
if (db->Empty()) {
return DecodeStatus::kDecodeInProgress;
}
uint8_t byte = db->DecodeUInt8();
uint64_t summand = byte & 0x7f;
// Shifting a 7 bit value to the left by at most 56 places can never
// overflow on uint64_t.
DCHECK_LE(offset_, 56);
DCHECK_LE(summand, std::numeric_limits<uint64_t>::max() >> offset_);
summand <<= offset_;
// At this point,
// |value_| is at most (2^prefix_length - 1) + (2^49 - 1), and
// |summand| is at most 255 << 56 (which is smaller than 2^63),
// so adding them can never overflow on uint64_t.
DCHECK_LE(value_, std::numeric_limits<uint64_t>::max() - summand);
value_ += summand;
// Decoding ends if continuation flag is not set.
if ((byte & 0x80) == 0) {
MarkDone();
return DecodeStatus::kDecodeDone;
}
offset_ += 7;
}
if (db->Empty()) {
return DecodeStatus::kDecodeInProgress;
}
DCHECK_EQ(kMaxOffset, offset_);
uint8_t byte = db->DecodeUInt8();
// No more extension bytes are allowed after this.
if ((byte & 0x80) == 0) {
uint64_t summand = byte & 0x7f;
// Check for overflow in left shift.
if (summand <= std::numeric_limits<uint64_t>::max() >> offset_) {
summand <<= offset_;
// Check for overflow in addition.
if (value_ <= std::numeric_limits<uint64_t>::max() - summand) {
value_ += summand;
MarkDone();
return DecodeStatus::kDecodeDone;
}
}
}
// Signal error if value is too large or there are too many extension bytes.
DLOG(WARNING) << "Variable length int encoding is too large or too long. "
<< DebugString();
MarkDone();
return DecodeStatus::kDecodeError;
}
// Old code path. TODO(bnc): remove.
DCHECK(!decode_64_bits_);
const uint8_t kMaxOffset = 28;
CheckNotDone(); CheckNotDone();
do { do {
if (db->Empty()) { if (db->Empty()) {
...@@ -51,6 +121,9 @@ DecodeStatus HpackVarintDecoder::Resume(DecodeBuffer* db) { ...@@ -51,6 +121,9 @@ DecodeStatus HpackVarintDecoder::Resume(DecodeBuffer* db) {
uint8_t byte = db->DecodeUInt8(); uint8_t byte = db->DecodeUInt8();
if (offset_ == kMaxOffset && byte != 0) if (offset_ == kMaxOffset && byte != 0)
break; break;
DCHECK(offset_ <= kMaxOffset - 7 || byte == 0);
// Shifting a 7 bit value to the left by at most 21 places can never
// overflow on uint32_t. Shifting 0 to the left cannot overflow either.
value_ += (byte & 0x7f) << offset_; value_ += (byte & 0x7f) << offset_;
if ((byte & 0x80) == 0) { if ((byte & 0x80) == 0) {
MarkDone(); MarkDone();
...@@ -64,12 +137,12 @@ DecodeStatus HpackVarintDecoder::Resume(DecodeBuffer* db) { ...@@ -64,12 +137,12 @@ DecodeStatus HpackVarintDecoder::Resume(DecodeBuffer* db) {
return DecodeStatus::kDecodeError; return DecodeStatus::kDecodeError;
} }
uint32_t HpackVarintDecoder::value() const { uint64_t HpackVarintDecoder::value() const {
CheckDone(); CheckDone();
return value_; return value_;
} }
void HpackVarintDecoder::set_value(uint32_t v) { void HpackVarintDecoder::set_value(uint64_t v) {
MarkDone(); MarkDone();
value_ = v; value_ = v;
} }
......
...@@ -2,29 +2,34 @@ ...@@ -2,29 +2,34 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
// HpackVarintDecoder decodes HPACK variable length unsigned integers. These // HpackVarintDecoder decodes HPACK variable length unsigned integers. In HPACK,
// integers are used to identify static or dynamic table index entries, to // these integers are used to identify static or dynamic table index entries, to
// specify string lengths, and to update the size limit of the dynamic table. // specify string lengths, and to update the size limit of the dynamic table.
// In QPACK, in addition to these uses, these integers also identify streams.
// //
// The caller will need to validate that the decoded value is in an acceptable // The caller will need to validate that the decoded value is in an acceptable
// range. // range.
// //
// In order to support naive encoders (i.e. which always output 5 extension
// bytes for a uint32 that is >= prefix_mask), the decoder supports an an
// encoding with up to 5 extension bytes, and a maximum value of 268,435,582
// (4 "full" extension bytes plus the maximum for a prefix, 127). It could be
// modified to support a lower maximum value (by requiring that extensions bytes
// be "empty"), or a larger value if valuable for some reason I can't see.
//
// For details of the encoding, see: // For details of the encoding, see:
// http://httpwg.org/specs/rfc7541.html#integer.representation // http://httpwg.org/specs/rfc7541.html#integer.representation
// //
// The decoder supports decoding integers up to 2^28 + 2^prefix_length - 2. // If GetHttp2ReloadableFlag(http2_varint_decode_64_bits) is true, then this
// decoder supports decoding any integer that can be represented on uint64_t,
// thereby exceeding the requirements for QPACK: "QPACK implementations MUST be
// able to decode integers up to 62 bits long." See
// https://quicwg.org/base-drafts/draft-ietf-quic-qpack.html#rfc.section.5.1.1
//
// If GetHttp2ReloadableFlag(http2_varint_decode_64_bits) is false, then this
// decoder supports decoding integers up to 2^28 + 2^prefix_length - 2.
// //
// TODO(jamessynge): Consider dropping support for encodings of more than 4 // This decoder supports at most 10 extension bytes (bytes following the prefix,
// bytes, including the prefix byte, as in practice we only see at most 3 bytes, // also called continuation bytes) if
// and 4 bytes would cover any desire to support large (but not ridiculously // GetHttp2ReloadableFlag(http2_varint_decode_64_bits) is true, and at most 5
// large) header values. // extension bytes if GetHttp2ReloadableFlag(http2_varint_decode_64_bits) is
// false. An encoder is allowed to zero pad the encoded integer on the left,
// thereby increasing the number of extension bytes. If an encoder uses so much
// padding that the number of extension bytes exceeds the limit, then this
// decoder signals an error.
#ifndef NET_THIRD_PARTY_HTTP2_HPACK_VARINT_HPACK_VARINT_DECODER_H_ #ifndef NET_THIRD_PARTY_HTTP2_HPACK_VARINT_HPACK_VARINT_DECODER_H_
#define NET_THIRD_PARTY_HTTP2_HPACK_VARINT_HPACK_VARINT_DECODER_H_ #define NET_THIRD_PARTY_HTTP2_HPACK_VARINT_HPACK_VARINT_DECODER_H_
...@@ -36,9 +41,18 @@ ...@@ -36,9 +41,18 @@
#include "net/third_party/http2/decoder/decode_buffer.h" #include "net/third_party/http2/decoder/decode_buffer.h"
#include "net/third_party/http2/decoder/decode_status.h" #include "net/third_party/http2/decoder/decode_status.h"
#include "net/third_party/http2/platform/api/http2_export.h" #include "net/third_party/http2/platform/api/http2_export.h"
#include "net/third_party/http2/platform/api/http2_flags.h"
#include "net/third_party/http2/platform/api/http2_string.h" #include "net/third_party/http2/platform/api/http2_string.h"
namespace http2 { namespace http2 {
// Sentinel value for |HpackVarintDecoder::offset_| to signify that decoding is
// completed. Only used in debug builds.
#ifndef NDEBUG
const uint8_t kHpackVarintDecoderOffsetDone =
std::numeric_limits<uint8_t>::max();
#endif
// Decodes an HPACK variable length unsigned integer, in a resumable fashion // Decodes an HPACK variable length unsigned integer, in a resumable fashion
// so it can handle running out of input in the DecodeBuffer. Call Start or // so it can handle running out of input in the DecodeBuffer. Call Start or
// StartExtended the first time (when decoding the byte that contains the // StartExtended the first time (when decoding the byte that contains the
...@@ -69,11 +83,11 @@ class HTTP2_EXPORT_PRIVATE HpackVarintDecoder { ...@@ -69,11 +83,11 @@ class HTTP2_EXPORT_PRIVATE HpackVarintDecoder {
// call to Start or StartExtended returned kDecodeInProgress. // call to Start or StartExtended returned kDecodeInProgress.
DecodeStatus Resume(DecodeBuffer* db); DecodeStatus Resume(DecodeBuffer* db);
uint32_t value() const; uint64_t value() const;
// This supports optimizations for the case of a varint with zero extension // This supports optimizations for the case of a varint with zero extension
// bytes, where the handling of the prefix is done by the caller. // bytes, where the handling of the prefix is done by the caller.
void set_value(uint32_t v); void set_value(uint64_t v);
// All the public methods below are for supporting assertions and tests. // All the public methods below are for supporting assertions and tests.
...@@ -91,24 +105,38 @@ class HTTP2_EXPORT_PRIVATE HpackVarintDecoder { ...@@ -91,24 +105,38 @@ class HTTP2_EXPORT_PRIVATE HpackVarintDecoder {
// Protection in case Resume is called when it shouldn't be. // Protection in case Resume is called when it shouldn't be.
void MarkDone() { void MarkDone() {
#ifndef NDEBUG #ifndef NDEBUG
offset_ = std::numeric_limits<uint32_t>::max(); offset_ = kHpackVarintDecoderOffsetDone;
#endif #endif
} }
void CheckNotDone() const { void CheckNotDone() const {
#ifndef NDEBUG #ifndef NDEBUG
DCHECK_NE(std::numeric_limits<uint32_t>::max(), offset_); DCHECK_NE(kHpackVarintDecoderOffsetDone, offset_);
#endif #endif
} }
void CheckDone() const { void CheckDone() const {
#ifndef NDEBUG #ifndef NDEBUG
DCHECK_EQ(std::numeric_limits<uint32_t>::max(), offset_); DCHECK_EQ(kHpackVarintDecoderOffsetDone, offset_);
#endif #endif
} }
// If true, decode integers up to 2^64 - 1, and accept at most 10 extension
// bytes (some of which might be padding).
// If false, decode integers up to 2^28 + 2^prefix_length - 2, and accept at
// most 5 extension bytes (some of which might be padding).
bool decode_64_bits_ = GetHttp2ReloadableFlag(http2_varint_decode_64_bits);
// These fields are initialized just to keep ASAN happy about reading // These fields are initialized just to keep ASAN happy about reading
// them from DebugString(). // them from DebugString().
uint32_t value_ = 0;
uint32_t offset_ = 0; // The encoded integer is being accumulated in |value_|. When decoding is
// complete, |value_| holds the result.
uint64_t value_ = 0;
// Each extension byte encodes in its lowest 7 bits a segment of the integer.
// |offset_| is the number of places this segment has to be shifted to the
// left for decoding. It is zero for the first extension byte, and increases
// by 7 for each subsequent extension byte.
uint8_t offset_ = 0;
}; };
} // namespace http2 } // namespace http2
......
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef NET_THIRD_PARTY_HTTP2_PLATFORM_API_HTTP2_FLAGS_H_
#define NET_THIRD_PARTY_HTTP2_PLATFORM_API_HTTP2_FLAGS_H_
#include "net/third_party/http2/platform/impl/http2_flags_impl.h"
#define GetHttp2ReloadableFlag(flag) GetHttp2ReloadableFlagImpl(flag)
#define SetHttp2ReloadableFlag(flag, value) \
SetHttp2ReloadableFlagImpl(flag, value)
#endif // NET_THIRD_PARTY_HTTP2_PLATFORM_API_HTTP2_FLAGS_H_
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "net/third_party/http2/platform/impl/http2_flags_impl.h"
bool FLAGS_chromium_flag_http2_varint_decode_64_bits = true;
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef NET_THIRD_PARTY_HTTP2_PLATFORM_IMPL_HTTP2_FLAGS_IMPL_H_
#define NET_THIRD_PARTY_HTTP2_PLATFORM_IMPL_HTTP2_FLAGS_IMPL_H_
#include "net/third_party/http2/platform/api/http2_export.h"
HTTP2_EXPORT_PRIVATE extern bool
FLAGS_chromium_flag_http2_varint_decode_64_bits;
namespace http2 {
inline bool GetHttp2FlagImpl(bool flag) {
return flag;
}
inline void SetHttp2FlagImpl(bool* f, bool v) {
*f = v;
}
#define HTTP2_RELOADABLE_FLAG(flag) FLAGS_chromium_flag_##flag
#define GetHttp2ReloadableFlagImpl(flag) \
GetHttp2FlagImpl(HTTP2_RELOADABLE_FLAG(flag))
#define SetHttp2ReloadableFlagImpl(flag, value) \
SetHttp2FlagImpl(&HTTP2_RELOADABLE_FLAG(flag), value)
} // namespace http2
#endif // NET_THIRD_PARTY_HTTP2_PLATFORM_IMPL_HTTP2_FLAGS_IMPL_H_
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef NET_THIRD_PARTY_HTTP2_TEST_TOOLS_HTTP2_RANDOM_H_ #ifndef NET_THIRD_PARTY_HTTP2_TEST_TOOLS_HTTP2_RANDOM_H_
#define NET_THIRD_PARTY_HTTP2_TEST_TOOLS_HTTP2_RANDOM_H_ #define NET_THIRD_PARTY_HTTP2_TEST_TOOLS_HTTP2_RANDOM_H_
...@@ -36,7 +40,7 @@ class Http2Random { ...@@ -36,7 +40,7 @@ class Http2Random {
// Return a uniformly distrubted random number in [0, n). // Return a uniformly distrubted random number in [0, n).
int32_t Uniform(int32_t n) { return Rand64() % n; } int32_t Uniform(int32_t n) { return Rand64() % n; }
// Return a uniformly distrubted random number in [lo, hi). // Return a uniformly distrubted random number in [lo, hi).
size_t UniformInRange(size_t lo, size_t hi) { int64_t UniformInRange(int64_t lo, int64_t hi) {
return lo + Rand64() % (hi - lo); return lo + Rand64() % (hi - lo);
} }
// Return an integer of logarithmically random scale. // Return an integer of logarithmically random scale.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment