Factor out QPACK absolute, relative, and post-base index conversion functions, and add new ones. gfe-relnote: n/a, change to QUIC v99-only code. Protected by existing disabled gfe2_reloadable_flag_quic_enable_version_99. PiperOrigin-RevId: 263421099 Change-Id: I762c3105da671f9d01ba5971c57a2f89f130ebbf
diff --git a/quic/core/qpack/qpack_decoder.cc b/quic/core/qpack/qpack_decoder.cc index 8e3103c..7fac1f2 100644 --- a/quic/core/qpack/qpack_decoder.cc +++ b/quic/core/qpack/qpack_decoder.cc
@@ -4,6 +4,7 @@ #include "net/third_party/quiche/src/quic/core/qpack/qpack_decoder.h" +#include "net/third_party/quiche/src/quic/core/qpack/qpack_index_conversions.h" #include "net/third_party/quiche/src/quic/platform/api/quic_logging.h" #include "net/third_party/quiche/src/quic/platform/api/quic_ptr_util.h" @@ -48,7 +49,8 @@ } uint64_t absolute_index; - if (!EncoderStreamRelativeIndexToAbsoluteIndex(name_index, &absolute_index)) { + if (!QpackEncoderStreamRelativeIndexToAbsoluteIndex( + name_index, header_table_.inserted_entry_count(), &absolute_index)) { encoder_stream_error_delegate_->OnEncoderStreamError( "Invalid relative index."); return; @@ -79,7 +81,8 @@ void QpackDecoder::OnDuplicate(uint64_t index) { uint64_t absolute_index; - if (!EncoderStreamRelativeIndexToAbsoluteIndex(index, &absolute_index)) { + if (!QpackEncoderStreamRelativeIndexToAbsoluteIndex( + index, header_table_.inserted_entry_count(), &absolute_index)) { encoder_stream_error_delegate_->OnEncoderStreamError( "Invalid relative index."); return; @@ -110,17 +113,6 @@ encoder_stream_error_delegate_->OnEncoderStreamError(error_message); } -bool QpackDecoder::EncoderStreamRelativeIndexToAbsoluteIndex( - uint64_t relative_index, - uint64_t* absolute_index) const { - if (relative_index >= header_table_.inserted_entry_count()) { - return false; - } - - *absolute_index = header_table_.inserted_entry_count() - relative_index - 1; - return true; -} - std::unique_ptr<QpackProgressiveDecoder> QpackDecoder::CreateProgressiveDecoder( QuicStreamId stream_id, QpackProgressiveDecoder::HeadersHandlerInterface* handler) {
diff --git a/quic/core/qpack/qpack_decoder.h b/quic/core/qpack/qpack_decoder.h index c00d777..1ec2d39 100644 --- a/quic/core/qpack/qpack_decoder.h +++ b/quic/core/qpack/qpack_decoder.h
@@ -85,14 +85,6 @@ } private: - // The encoder stream uses relative index (but different from the kind of - // relative index used on a request stream). This method converts relative - // index to absolute index (zero based). It returns true on success, or false - // if conversion fails due to overflow/underflow. - bool EncoderStreamRelativeIndexToAbsoluteIndex( - uint64_t relative_index, - uint64_t* absolute_index) const; - EncoderStreamErrorDelegate* const encoder_stream_error_delegate_; QpackEncoderStreamReceiver encoder_stream_receiver_; QpackDecoderStreamSender decoder_stream_sender_;
diff --git a/quic/core/qpack/qpack_index_conversions.cc b/quic/core/qpack/qpack_index_conversions.cc new file mode 100644 index 0000000..8f1d52f --- /dev/null +++ b/quic/core/qpack/qpack_index_conversions.cc
@@ -0,0 +1,62 @@ +// Copyright (c) 2019 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/quiche/src/quic/core/qpack/qpack_index_conversions.h" + +#include <limits> + +#include "net/third_party/quiche/src/quic/platform/api/quic_logging.h" + +namespace quic { + +uint64_t QpackAbsoluteIndexToEncoderStreamRelativeIndex( + uint64_t absolute_index, + uint64_t inserted_entry_count) { + DCHECK_LT(absolute_index, inserted_entry_count); + + return inserted_entry_count - absolute_index - 1; +} + +uint64_t QpackAbsoluteIndexToRequestStreamRelativeIndex(uint64_t absolute_index, + uint64_t base) { + DCHECK_LT(absolute_index, base); + + return base - absolute_index - 1; +} + +bool QpackEncoderStreamRelativeIndexToAbsoluteIndex( + uint64_t relative_index, + uint64_t inserted_entry_count, + uint64_t* absolute_index) { + if (relative_index >= inserted_entry_count) { + return false; + } + + *absolute_index = inserted_entry_count - relative_index - 1; + return true; +} + +bool QpackRequestStreamRelativeIndexToAbsoluteIndex(uint64_t relative_index, + uint64_t base, + uint64_t* absolute_index) { + if (relative_index >= base) { + return false; + } + + *absolute_index = base - relative_index - 1; + return true; +} + +bool QpackPostBaseIndexToAbsoluteIndex(uint64_t post_base_index, + uint64_t base, + uint64_t* absolute_index) { + if (post_base_index >= std::numeric_limits<uint64_t>::max() - base) { + return false; + } + + *absolute_index = base + post_base_index; + return true; +} + +} // namespace quic
diff --git a/quic/core/qpack/qpack_index_conversions.h b/quic/core/qpack/qpack_index_conversions.h new file mode 100644 index 0000000..f05a477 --- /dev/null +++ b/quic/core/qpack/qpack_index_conversions.h
@@ -0,0 +1,65 @@ +// Copyright (c) 2019 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. + +// Utility methods to convert between absolute indexing (used in the dynamic +// table), relative indexing used on the encoder stream, and relative indexing +// and post-base indexing used on request streams (in header blocks). See: +// https://quicwg.org/base-drafts/draft-ietf-quic-qpack.html#indexing +// https://quicwg.org/base-drafts/draft-ietf-quic-qpack.html#relative-indexing +// https://quicwg.org/base-drafts/draft-ietf-quic-qpack.html#post-base + +#ifndef QUICHE_QUIC_CORE_QPACK_QPACK_INDEX_CONVERSIONS_H_ +#define QUICHE_QUIC_CORE_QPACK_QPACK_INDEX_CONVERSIONS_H_ + +#include <cstdint> + +namespace quic { + +// Conversion functions used in the encoder do not check for overflow/underflow. +// Since the maximum index is limited by maximum dynamic table capacity +// (represented on uint64_t) divided by minimum header field size (defined to be +// 32 bytes), overflow is not possible. The caller is responsible for providing +// input that does not underflow. + +uint64_t QpackAbsoluteIndexToEncoderStreamRelativeIndex( + uint64_t absolute_index, + uint64_t inserted_entry_count); + +uint64_t QpackAbsoluteIndexToRequestStreamRelativeIndex(uint64_t absolute_index, + uint64_t base); + +// Conversion functions used in the decoder operate on input received from the +// network. These functions return false on overflow or underflow. + +// TODO The encoder stream uses relative index (but different from the kind of +// relative index used on a request stream). This method converts relative +// index to absolute index (zero based). It returns true on success, or false +// if conversion fails due to overflow/underflow. + +bool QpackEncoderStreamRelativeIndexToAbsoluteIndex( + uint64_t relative_index, + uint64_t inserted_entry_count, + uint64_t* absolute_index); + +// TODO The request stream can use relative index (but different from the kind +// of relative index used on the encoder stream), and post-base index. These +// methods convert relative index and post-base index to absolute index (one +// based). They return true on success, or false if conversion fails due to +// overflow/underflow. + +// On success, |*absolute_index| is guaranteed to be strictly less than +// std::numeric_limits<uint64_t>::max(). +bool QpackRequestStreamRelativeIndexToAbsoluteIndex(uint64_t relative_index, + uint64_t base, + uint64_t* absolute_index); + +// On success, |*absolute_index| is guaranteed to be strictly less than +// std::numeric_limits<uint64_t>::max(). +bool QpackPostBaseIndexToAbsoluteIndex(uint64_t post_base_index, + uint64_t base, + uint64_t* absolute_index); + +} // namespace quic + +#endif // QUICHE_QUIC_CORE_QPACK_QPACK_INDEX_CONVERSIONS_H_
diff --git a/quic/core/qpack/qpack_index_conversions_test.cc b/quic/core/qpack/qpack_index_conversions_test.cc new file mode 100644 index 0000000..214dff5 --- /dev/null +++ b/quic/core/qpack/qpack_index_conversions_test.cc
@@ -0,0 +1,99 @@ +// Copyright (c) 2019 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/quiche/src/quic/core/qpack/qpack_index_conversions.h" + +#include "net/third_party/quiche/src/quic/platform/api/quic_test.h" + +namespace quic { +namespace test { +namespace { + +struct { + uint64_t relative_index; + uint64_t inserted_entry_count; + uint64_t expected_absolute_index; +} kEncoderStreamRelativeIndexTestData[] = {{0, 1, 0}, {0, 2, 1}, {1, 2, 0}, + {0, 10, 9}, {5, 10, 4}, {9, 10, 0}}; + +TEST(QpackIndexConversions, EncoderStreamRelativeIndex) { + for (const auto& test_data : kEncoderStreamRelativeIndexTestData) { + uint64_t absolute_index = 42; + EXPECT_TRUE(QpackEncoderStreamRelativeIndexToAbsoluteIndex( + test_data.relative_index, test_data.inserted_entry_count, + &absolute_index)); + EXPECT_EQ(test_data.expected_absolute_index, absolute_index); + + EXPECT_EQ(test_data.relative_index, + QpackAbsoluteIndexToEncoderStreamRelativeIndex( + absolute_index, test_data.inserted_entry_count)); + } +} + +struct { + uint64_t relative_index; + uint64_t base; + uint64_t expected_absolute_index; +} kRequestStreamRelativeIndexTestData[] = {{0, 1, 0}, {0, 2, 1}, {1, 2, 0}, + {0, 10, 9}, {5, 10, 4}, {9, 10, 0}}; + +TEST(QpackIndexConversions, RequestStreamRelativeIndex) { + for (const auto& test_data : kRequestStreamRelativeIndexTestData) { + uint64_t absolute_index = 42; + EXPECT_TRUE(QpackRequestStreamRelativeIndexToAbsoluteIndex( + test_data.relative_index, test_data.base, &absolute_index)); + EXPECT_EQ(test_data.expected_absolute_index, absolute_index); + + EXPECT_EQ(test_data.relative_index, + QpackAbsoluteIndexToRequestStreamRelativeIndex(absolute_index, + test_data.base)); + } +} + +struct { + uint64_t post_base_index; + uint64_t base; + uint64_t expected_absolute_index; +} kPostBaseIndexTestData[] = {{0, 1, 1}, {1, 0, 1}, {2, 0, 2}, + {1, 1, 2}, {0, 2, 2}, {1, 2, 3}}; + +TEST(QpackIndexConversions, PostBaseIndex) { + for (const auto& test_data : kPostBaseIndexTestData) { + uint64_t absolute_index = 42; + EXPECT_TRUE(QpackPostBaseIndexToAbsoluteIndex( + test_data.post_base_index, test_data.base, &absolute_index)); + EXPECT_EQ(test_data.expected_absolute_index, absolute_index); + } +} + +TEST(QpackIndexConversions, EncoderStreamRelativeIndexUnderflow) { + uint64_t absolute_index; + EXPECT_FALSE(QpackEncoderStreamRelativeIndexToAbsoluteIndex( + /* relative_index = */ 10, + /* inserted_entry_count = */ 10, &absolute_index)); + EXPECT_FALSE(QpackEncoderStreamRelativeIndexToAbsoluteIndex( + /* relative_index = */ 12, + /* inserted_entry_count = */ 10, &absolute_index)); +} + +TEST(QpackIndexConversions, RequestStreamRelativeIndexUnderflow) { + uint64_t absolute_index; + EXPECT_FALSE(QpackRequestStreamRelativeIndexToAbsoluteIndex( + /* relative_index = */ 10, + /* base = */ 10, &absolute_index)); + EXPECT_FALSE(QpackRequestStreamRelativeIndexToAbsoluteIndex( + /* relative_index = */ 12, + /* base = */ 10, &absolute_index)); +} + +TEST(QpackIndexConversions, QpackPostBaseIndexToAbsoluteIndexOverflow) { + uint64_t absolute_index; + EXPECT_FALSE(QpackPostBaseIndexToAbsoluteIndex( + /* post_base_index = */ 20, + /* base = */ std::numeric_limits<uint64_t>::max() - 10, &absolute_index)); +} + +} // namespace +} // namespace test +} // namespace quic
diff --git a/quic/core/qpack/qpack_progressive_decoder.cc b/quic/core/qpack/qpack_progressive_decoder.cc index d9449e3..e5f7f09 100644 --- a/quic/core/qpack/qpack_progressive_decoder.cc +++ b/quic/core/qpack/qpack_progressive_decoder.cc
@@ -8,6 +8,7 @@ #include <limits> #include "net/third_party/quiche/src/quic/core/qpack/qpack_constants.h" +#include "net/third_party/quiche/src/quic/core/qpack/qpack_index_conversions.h" #include "net/third_party/quiche/src/quic/core/qpack/qpack_required_insert_count.h" #include "net/third_party/quiche/src/quic/platform/api/quic_logging.h" #include "net/third_party/quiche/src/quic/platform/api/quic_ptr_util.h" @@ -125,8 +126,8 @@ bool QpackProgressiveDecoder::DoIndexedHeaderFieldInstruction() { if (!instruction_decoder_.s_bit()) { uint64_t absolute_index; - if (!RequestStreamRelativeIndexToAbsoluteIndex( - instruction_decoder_.varint(), &absolute_index)) { + if (!QpackRequestStreamRelativeIndexToAbsoluteIndex( + instruction_decoder_.varint(), base_, &absolute_index)) { OnError("Invalid relative index."); return false; } @@ -164,8 +165,8 @@ bool QpackProgressiveDecoder::DoIndexedHeaderFieldPostBaseInstruction() { uint64_t absolute_index; - if (!PostBaseIndexToAbsoluteIndex(instruction_decoder_.varint(), - &absolute_index)) { + if (!QpackPostBaseIndexToAbsoluteIndex(instruction_decoder_.varint(), base_, + &absolute_index)) { OnError("Invalid post-base index."); return false; } @@ -193,8 +194,8 @@ bool QpackProgressiveDecoder::DoLiteralHeaderFieldNameReferenceInstruction() { if (!instruction_decoder_.s_bit()) { uint64_t absolute_index; - if (!RequestStreamRelativeIndexToAbsoluteIndex( - instruction_decoder_.varint(), &absolute_index)) { + if (!QpackRequestStreamRelativeIndexToAbsoluteIndex( + instruction_decoder_.varint(), base_, &absolute_index)) { OnError("Invalid relative index."); return false; } @@ -232,8 +233,8 @@ bool QpackProgressiveDecoder::DoLiteralHeaderFieldPostBaseInstruction() { uint64_t absolute_index; - if (!PostBaseIndexToAbsoluteIndex(instruction_decoder_.varint(), - &absolute_index)) { + if (!QpackPostBaseIndexToAbsoluteIndex(instruction_decoder_.varint(), base_, + &absolute_index)) { OnError("Invalid post-base index."); return false; } @@ -343,26 +344,4 @@ return true; } -bool QpackProgressiveDecoder::RequestStreamRelativeIndexToAbsoluteIndex( - uint64_t relative_index, - uint64_t* absolute_index) const { - if (relative_index >= base_) { - return false; - } - - *absolute_index = base_ - 1 - relative_index; - return true; -} - -bool QpackProgressiveDecoder::PostBaseIndexToAbsoluteIndex( - uint64_t post_base_index, - uint64_t* absolute_index) const { - if (post_base_index >= std::numeric_limits<uint64_t>::max() - base_) { - return false; - } - - *absolute_index = base_ + post_base_index; - return true; -} - } // namespace quic
diff --git a/quic/core/qpack/qpack_progressive_decoder.h b/quic/core/qpack/qpack_progressive_decoder.h index 3abf427..6d2e992 100644 --- a/quic/core/qpack/qpack_progressive_decoder.h +++ b/quic/core/qpack/qpack_progressive_decoder.h
@@ -89,18 +89,6 @@ // failure due to overflow/underflow. bool DeltaBaseToBase(bool sign, uint64_t delta_base, uint64_t* base); - // The request stream can use relative index (but different from the kind of - // relative index used on the encoder stream), and post-base index. - // These methods convert relative index and post-base index to absolute index - // (one based). They return true on success, or false if conversion fails due - // to overflow/underflow. On success, |*absolute_index| is guaranteed to be - // strictly less than std::numeric_limits<uint64_t>::max(). - bool RequestStreamRelativeIndexToAbsoluteIndex( - uint64_t relative_index, - uint64_t* absolute_index) const; - bool PostBaseIndexToAbsoluteIndex(uint64_t post_base_index, - uint64_t* absolute_index) const; - const QuicStreamId stream_id_; // |prefix_decoder_| only decodes a handful of bytes then it can be