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

@@ -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";

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));
+}