Signal error if Insert Count Increment causes overflow.
gfe-relnote: n/a, change to QUIC v99-only code. Protected by existing disabled gfe2_reloadable_flag_quic_enable_version_99.
PiperOrigin-RevId: 274830568
Change-Id: Ic9e7fa84e04808d2c7bade54ed3670b53b5bb0f6
diff --git a/quic/core/qpack/qpack_blocking_manager.cc b/quic/core/qpack/qpack_blocking_manager.cc
index 2243c90..ffec172 100644
--- a/quic/core/qpack/qpack_blocking_manager.cc
+++ b/quic/core/qpack/qpack_blocking_manager.cc
@@ -4,6 +4,7 @@
#include "net/third_party/quiche/src/quic/core/qpack/qpack_blocking_manager.h"
+#include <limits>
#include <utility>
namespace quic {
@@ -49,8 +50,14 @@
header_blocks_.erase(it);
}
-void QpackBlockingManager::OnInsertCountIncrement(uint64_t increment) {
+bool QpackBlockingManager::OnInsertCountIncrement(uint64_t increment) {
+ if (increment >
+ std::numeric_limits<uint64_t>::max() - known_received_count_) {
+ return false;
+ }
+
IncreaseKnownReceivedCountTo(known_received_count_ + increment);
+ return true;
}
void QpackBlockingManager::OnHeaderBlockSent(QuicStreamId stream_id,
diff --git a/quic/core/qpack/qpack_blocking_manager.h b/quic/core/qpack/qpack_blocking_manager.h
index 6bebf5f..60f7db7 100644
--- a/quic/core/qpack/qpack_blocking_manager.h
+++ b/quic/core/qpack/qpack_blocking_manager.h
@@ -40,8 +40,9 @@
void OnStreamCancellation(QuicStreamId stream_id);
// Called when an Insert Count Increment instruction is received on the
- // decoder stream.
- void OnInsertCountIncrement(uint64_t increment);
+ // decoder stream. Returns true if Known Received Count is successfully
+ // updated. Returns false on overflow.
+ bool OnInsertCountIncrement(uint64_t increment);
// Called when sending a header block containing references to dynamic table
// entries with |indices|. |indices| must not be empty.
diff --git a/quic/core/qpack/qpack_blocking_manager_test.cc b/quic/core/qpack/qpack_blocking_manager_test.cc
index 8257363..64bfb97 100644
--- a/quic/core/qpack/qpack_blocking_manager_test.cc
+++ b/quic/core/qpack/qpack_blocking_manager_test.cc
@@ -4,6 +4,8 @@
#include "net/third_party/quiche/src/quic/core/qpack/qpack_blocking_manager.h"
+#include <limits>
+
#include "net/third_party/quiche/src/quic/platform/api/quic_test.h"
namespace quic {
@@ -53,7 +55,7 @@
}
TEST_F(QpackBlockingManagerTest, NotBlockedByInsertCountIncrement) {
- manager_.OnInsertCountIncrement(2);
+ EXPECT_TRUE(manager_.OnInsertCountIncrement(2));
// Stream 0 is not blocked, because it only references entries that are
// already acknowledged by an Insert Count Increment instruction.
@@ -65,7 +67,7 @@
manager_.OnHeaderBlockSent(0, {1, 0});
EXPECT_TRUE(stream_is_blocked(0));
- manager_.OnInsertCountIncrement(2);
+ EXPECT_TRUE(manager_.OnInsertCountIncrement(2));
EXPECT_FALSE(stream_is_blocked(0));
}
@@ -114,7 +116,7 @@
EXPECT_EQ(2u, manager_.known_received_count());
// Insert Count Increment increases Known Received Count.
- manager_.OnInsertCountIncrement(2);
+ EXPECT_TRUE(manager_.OnInsertCountIncrement(2));
EXPECT_EQ(4u, manager_.known_received_count());
EXPECT_TRUE(manager_.OnHeaderAcknowledgement(2));
@@ -161,7 +163,7 @@
EXPECT_EQ(1u, manager_.smallest_blocking_index());
// Insert Count Increment does not change smallest blocking index.
- manager_.OnInsertCountIncrement(2);
+ EXPECT_TRUE(manager_.OnInsertCountIncrement(2));
EXPECT_EQ(1u, manager_.smallest_blocking_index());
manager_.OnStreamCancellation(1);
@@ -249,7 +251,7 @@
EXPECT_EQ(0u, manager_.smallest_blocking_index());
// Acknowledging entry 1 still leaves one unacknowledged reference to entry 0.
- manager_.OnInsertCountIncrement(2);
+ EXPECT_TRUE(manager_.OnInsertCountIncrement(2));
EXPECT_EQ(2u, manager_.known_received_count());
EXPECT_EQ(0u, manager_.smallest_blocking_index());
@@ -261,13 +263,13 @@
EXPECT_EQ(0u, manager_.smallest_blocking_index());
// Acknowledging entry 2 removes last reference to entry 0.
- manager_.OnInsertCountIncrement(1);
+ EXPECT_TRUE(manager_.OnInsertCountIncrement(1));
EXPECT_EQ(3u, manager_.known_received_count());
EXPECT_EQ(2u, manager_.smallest_blocking_index());
// Acknowledging entry 4 (and implicitly 3) removes reference to entry 2.
- manager_.OnInsertCountIncrement(2);
+ EXPECT_TRUE(manager_.OnInsertCountIncrement(2));
EXPECT_EQ(5u, manager_.known_received_count());
EXPECT_EQ(std::numeric_limits<uint64_t>::max(),
@@ -386,6 +388,14 @@
EXPECT_TRUE(manager_.blocking_allowed_on_stream(kStreamId2, 1));
}
+TEST_F(QpackBlockingManagerTest, InsertCountIncrementOverflow) {
+ EXPECT_TRUE(manager_.OnInsertCountIncrement(10));
+ EXPECT_EQ(10u, manager_.known_received_count());
+
+ EXPECT_FALSE(manager_.OnInsertCountIncrement(
+ std::numeric_limits<uint64_t>::max() - 5));
+}
+
} // namespace
} // namespace test
} // namespace quic
diff --git a/quic/core/qpack/qpack_encoder.cc b/quic/core/qpack/qpack_encoder.cc
index 418af8c..319af82 100644
--- a/quic/core/qpack/qpack_encoder.cc
+++ b/quic/core/qpack/qpack_encoder.cc
@@ -406,7 +406,10 @@
return;
}
- blocking_manager_.OnInsertCountIncrement(increment);
+ if (!blocking_manager_.OnInsertCountIncrement(increment)) {
+ decoder_stream_error_delegate_->OnDecoderStreamError(
+ "Insert Count Increment instruction causes overflow.");
+ }
if (blocking_manager_.known_received_count() >
header_table_.inserted_entry_count()) {
diff --git a/quic/core/qpack/qpack_encoder_test.cc b/quic/core/qpack/qpack_encoder_test.cc
index 3bc5859..6f2efe3 100644
--- a/quic/core/qpack/qpack_encoder_test.cc
+++ b/quic/core/qpack/qpack_encoder_test.cc
@@ -4,6 +4,7 @@
#include "net/third_party/quiche/src/quic/core/qpack/qpack_encoder.h"
+#include <limits>
#include <string>
#include "net/third_party/quiche/src/quic/core/qpack/qpack_encoder_test_utils.h"
@@ -176,6 +177,27 @@
encoder_.OnInsertCountIncrement(1);
}
+// Regression test for https://crbug.com/1014372.
+TEST_F(QpackEncoderTest, InsertCountIncrementOverflow) {
+ QpackHeaderTable* header_table = QpackEncoderPeer::header_table(&encoder_);
+
+ // Set dynamic table capacity large enough to hold one entry.
+ header_table->SetMaximumDynamicTableCapacity(4096);
+ header_table->SetDynamicTableCapacity(4096);
+ // Insert one entry into the header table.
+ header_table->InsertEntry("foo", "bar");
+
+ // Receive Insert Count Increment instruction with increment value 1.
+ encoder_.OnInsertCountIncrement(1);
+
+ // Receive Insert Count Increment instruction that overflows the known
+ // received count. This must result in an error instead of a crash.
+ EXPECT_CALL(decoder_stream_error_delegate_,
+ OnDecoderStreamError(
+ Eq("Insert Count Increment instruction causes overflow.")));
+ encoder_.OnInsertCountIncrement(std::numeric_limits<uint64_t>::max());
+}
+
TEST_F(QpackEncoderTest, InvalidHeaderAcknowledgement) {
// Encoder receives header acknowledgement for a stream on which no header
// block with dynamic table entries was ever sent.
