This is an automated email from the ASF dual-hosted git repository.
wwbmmm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brpc.git
The following commit(s) were added to refs/heads/master by this push:
new a5c3365a size hpack dynamic table for the 33-byte minimum entry (#3462)
a5c3365a is described below
commit a5c3365af1902caf0158975363868d99f5705bfa
Author: UB <[email protected]>
AuthorDate: Sat Aug 22 09:39:36 2026 +0530
size hpack dynamic table for the 33-byte minimum entry (#3462)
* size hpack dynamic table for the 33-byte minimum entry
* keep at least one dynamic-table slot when max_size < 33
---
src/brpc/details/hpack.cpp | 16 ++++++++++++++--
test/brpc_hpack_unittest.cpp | 34 ++++++++++++++++++++++++++++++++++
2 files changed, 48 insertions(+), 2 deletions(-)
diff --git a/src/brpc/details/hpack.cpp b/src/brpc/details/hpack.cpp
index c9c707c9..7a7bb366 100644
--- a/src/brpc/details/hpack.cpp
+++ b/src/brpc/details/hpack.cpp
@@ -223,11 +223,23 @@ int IndexTable::Init(const IndexTableOptions& options) {
num_headers = options.static_table_size;
_max_size = UINT_MAX;
} else {
- num_headers = options.max_size / (32 + 2);
+ num_headers = options.max_size / (32 + 1);
// ^
- // name and value both have at least one byte in.
+ // AddHeader() only requires a non-empty name; the value may be empty,
+ // so the smallest possible entry is name(1) + value(0) + 32 = 33 bytes
+ // (rfc7541 section 4.1). Sizing the queue for 34-byte entries under-
+ // provisions it and lets a peer sending many 33-byte entries fill the
+ // queue before eviction triggers, tripping CHECK(!full()) in
AddHeader.
_max_size = options.max_size;
}
+ // A dynamic table smaller than the 33-byte minimum entry (including the
+ // valid max_size == 0 case that disables it) yields num_headers == 0.
+ // malloc(0) may return NULL and make Init fail on some platforms, so keep
+ // at least one slot. No entry can actually be stored since entry_size >
+ // _max_size still holds in AddHeader().
+ if (num_headers == 0) {
+ num_headers = 1;
+ }
void *header_queue_storage = malloc(num_headers * sizeof(Header));
if (!header_queue_storage) {
LOG(ERROR) << "Fail to malloc space for " << num_headers << " headers";
diff --git a/test/brpc_hpack_unittest.cpp b/test/brpc_hpack_unittest.cpp
index 98d86648..191bf003 100644
--- a/test/brpc_hpack_unittest.cpp
+++ b/test/brpc_hpack_unittest.cpp
@@ -639,3 +639,37 @@ TEST_F(HPackTest, responses_with_huffman) {
}
ASSERT_TRUE(buf.buf().empty());
}
+
+TEST_F(HPackTest, many_small_indexed_headers) {
+ // Each entry below is a "literal header field with incremental indexing"
+ // (0x40) with a 1-byte name ("a") and an empty value, costing
+ // name(1) + value(0) + 32 = 33 bytes in the dynamic table (rfc7541 4.1).
+ // 121 of them stay under the default 4096-byte table (121*33 = 3993) so
+ // none are evicted and all must decode. The decode table's ring buffer was
+ // previously sized for 34-byte entries (4096/34 = 120), so the 121st
+ // AddHeader() tripped CHECK(!full()) and aborted the process.
+ brpc::HPacker p;
+ ASSERT_EQ(0, p.Init(4096));
+
+ const int num_headers = 121;
+ butil::IOBuf buf;
+ for (int i = 0; i < num_headers; ++i) {
+ const uint8_t entry[] = {0x40, 0x01, (uint8_t)'a', 0x00};
+ buf.append(entry, sizeof(entry));
+ }
+ for (int i = 0; i < num_headers; ++i) {
+ brpc::HPacker::Header h;
+ ASSERT_GT(p.Decode(&buf, &h), 0) << "failed at header " << i;
+ ASSERT_EQ("a", h.name);
+ ASSERT_TRUE(h.value.empty());
+ }
+ ASSERT_TRUE(buf.empty());
+}
+
+TEST_F(HPackTest, zero_size_dynamic_table) {
+ // max_size == 0 disables the dynamic table. num_headers then rounds down
to
+ // 0, and malloc(0) may return NULL, so Init must still keep one slot and
+ // succeed. No entry is ever stored because entry_size > max_size.
+ brpc::HPacker p;
+ ASSERT_EQ(0, p.Init(0));
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
