From 068b4810c59d86a2726177565c340c0828a8df97 Mon Sep 17 00:00:00 2001 From: wwbmmm Date: Sun, 14 Jun 2026 14:39:43 +0800 Subject: [PATCH] Fix HPACK dynamic table size update recursive decoding --- src/brpc/details/hpack.cpp | 19 +++++++--- test/brpc_hpack_unittest.cpp | 70 ++++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 5 deletions(-) diff --git a/src/brpc/details/hpack.cpp b/src/brpc/details/hpack.cpp index 687a6145d4..e4e7890e17 100644 --- a/src/brpc/details/hpack.cpp +++ b/src/brpc/details/hpack.cpp @@ -763,6 +763,8 @@ inline ssize_t HPacker::DecodeWithKnownPrefix( } ssize_t HPacker::Decode(butil::IOBufBytesIterator& iter, Header* h) { + ssize_t skipped_bytes = 0; +decode_next: if (iter == NULL) { return 0; } @@ -791,7 +793,7 @@ ssize_t HPacker::Decode(butil::IOBufBytesIterator& iter, Header* h) { return -1; } *h = *indexed_header; - return index_bytes; + return skipped_bytes + index_bytes; } break; case 7: @@ -806,7 +808,7 @@ ssize_t HPacker::Decode(butil::IOBufBytesIterator& iter, Header* h) { return -1; } _decode_table->AddHeader(*h); - return bytes_consumed; + return skipped_bytes + bytes_consumed; } break; case 3: @@ -824,17 +826,24 @@ ssize_t HPacker::Decode(butil::IOBufBytesIterator& iter, Header* h) { return -1; } _decode_table->ResetMaxSize(max_size); - return Decode(iter, h); + skipped_bytes += read_bytes; + goto decode_next; } case 1: // (0001) Literal Header Field Never Indexed // https://tools.ietf.org/html/rfc7541#section-6.2.3 - return DecodeWithKnownPrefix(iter, h, 4); + { + const ssize_t bytes_consumed = DecodeWithKnownPrefix(iter, h, 4); + return bytes_consumed > 0 ? skipped_bytes + bytes_consumed : bytes_consumed; + } // TODO: Expose NeverIndex to the caller. case 0: // (0000) Literal Header Field without Indexing // https://tools.ietf.org/html/rfc7541#section-6.2.1 - return DecodeWithKnownPrefix(iter, h, 4); + { + const ssize_t bytes_consumed = DecodeWithKnownPrefix(iter, h, 4); + return bytes_consumed > 0 ? skipped_bytes + bytes_consumed : bytes_consumed; + } // TODO: Expose NeverIndex to the caller. default: CHECK(false) << "Can't reach here"; diff --git a/test/brpc_hpack_unittest.cpp b/test/brpc_hpack_unittest.cpp index f30a712817..d6a0bd02ff 100644 --- a/test/brpc_hpack_unittest.cpp +++ b/test/brpc_hpack_unittest.cpp @@ -20,12 +20,82 @@ // Date: 2017/04/25 00:23:12 #include +#include +#include +#include +#include +#include #include "brpc/details/hpack.h" #include "butil/logging.h" class HPackTest : public testing::Test { }; +static void* DecodeManyDynamicTableSizeUpdates(void*) { + brpc::HPacker p; + if (p.Init(4096) != 0) { + return (void*)1; + } + + butil::IOBuf buf; + std::string updates(200000, '\x20'); + buf.append(updates); + + brpc::HPacker::Header h; + return (void*)(p.Decode(&buf, &h) != 0); +} + +TEST_F(HPackTest, many_dynamic_table_size_updates) { + const pid_t pid = fork(); + ASSERT_GE(pid, 0); + if (pid == 0) { + if (freopen("/dev/null", "w", stdout) == NULL || + freopen("/dev/null", "w", stderr) == NULL) { + exit(1); + } + + pthread_attr_t attr; + if (pthread_attr_init(&attr) != 0) { + exit(2); + } + if (pthread_attr_setstacksize(&attr, 64 * 1024) != 0) { + exit(3); + } + pthread_t tid; + if (pthread_create(&tid, &attr, DecodeManyDynamicTableSizeUpdates, NULL) != 0) { + exit(4); + } + pthread_attr_destroy(&attr); + void* ret = NULL; + if (pthread_join(tid, &ret) != 0) { + exit(5); + } + exit(ret == NULL ? 0 : 6); + } + int status = 0; + ASSERT_EQ(pid, waitpid(pid, &status, 0)); + ASSERT_TRUE(WIFEXITED(status)); + ASSERT_EQ(0, WEXITSTATUS(status)); +} + +TEST_F(HPackTest, dynamic_table_size_update_before_header) { + brpc::HPacker p; + ASSERT_EQ(0, p.Init(4096)); + + butil::IOBuf buf; + uint8_t encoded[] = { + 0x20, // Dynamic table size update to 0. + 0x82, // Indexed :method: GET. + }; + buf.append(encoded, sizeof(encoded)); + + brpc::HPacker::Header h; + ASSERT_EQ((ssize_t)sizeof(encoded), p.Decode(&buf, &h)); + ASSERT_TRUE(buf.empty()); + ASSERT_EQ(":method", h.name); + ASSERT_EQ("GET", h.value); +} + // Copied test cases from example of rfc7541 TEST_F(HPackTest, header_with_indexing) { brpc::HPacker p1;