From 0daecb74080bb89485264751ddcf488e60fe0ea0 Mon Sep 17 00:00:00 2001 From: songgaoye Date: Wed, 14 Oct 2026 18:53:00 +0800 Subject: [PATCH 1/7] Fix stale Set-Cookie pointer after HttpHeader mutations --- src/brpc/http_header.cpp | 15 +++++++++++++++ src/brpc/http_header.h | 2 ++ test/brpc_http_message_unittest.cpp | 17 +++++++++++++++++ 3 files changed, 34 insertions(+) diff --git a/src/brpc/http_header.cpp b/src/brpc/http_header.cpp index 6ef074712e..942f5253f9 100644 --- a/src/brpc/http_header.cpp +++ b/src/brpc/http_header.cpp @@ -42,6 +42,8 @@ void HttpHeader::Swap(HttpHeader &rhs) { _content_type.swap(rhs._content_type); _unresolved_path.swap(rhs._unresolved_path); std::swap(_version, rhs._version); + RefreshFirstSetCookie(); + rhs.RefreshFirstSetCookie(); } void HttpHeader::Clear() { @@ -52,6 +54,7 @@ void HttpHeader::Clear() { _content_type.clear(); _unresolved_path.clear(); _version = std::make_pair(1, 1); + _first_set_cookie = nullptr; } const std::string* HttpHeader::GetHeader(const char* key) const { @@ -136,22 +139,34 @@ std::string& HttpHeader::GetOrAddHeader(const std::string& key) { std::string* val = _headers.seek(key); if (nullptr == val) { + const size_t old_bucket_count = _headers.bucket_count(); val = _headers.insert({ key, "" }); if (is_set_cookie) { _first_set_cookie = val; } + if (_headers.bucket_count() != old_bucket_count) { + RefreshFirstSetCookie(); + } } return *val; } std::string& HttpHeader::AddHeader(const std::string& key) { + const size_t old_bucket_count = _headers.bucket_count(); std::string* val = _headers.insert({ key, "" }); if (IsSetCookie(key) && nullptr == _first_set_cookie) { _first_set_cookie = val; } + if (_headers.bucket_count() != old_bucket_count) { + RefreshFirstSetCookie(); + } return *val; } +void HttpHeader::RefreshFirstSetCookie() { + _first_set_cookie = _headers.seek(SET_COOKIE); +} + const HttpHeader& DefaultHttpHeader() { static HttpHeader h; return h; diff --git a/src/brpc/http_header.h b/src/brpc/http_header.h index 58b92ecb19..40ec82be07 100644 --- a/src/brpc/http_header.h +++ b/src/brpc/http_header.h @@ -171,6 +171,8 @@ friend void policy::ProcessHttpRequest(InputMessageBase *msg); std::string& AddHeader(const std::string& key); + void RefreshFirstSetCookie(); + bool IsSetCookie(const std::string& key) const { return _header_key_equal(key, SET_COOKIE); } diff --git a/test/brpc_http_message_unittest.cpp b/test/brpc_http_message_unittest.cpp index 078659bef5..8a8bd2e628 100644 --- a/test/brpc_http_message_unittest.cpp +++ b/test/brpc_http_message_unittest.cpp @@ -883,6 +883,23 @@ TEST(HttpMessageTest, http_header) { ASSERT_FALSE(header.GetHeader(brpc::HttpHeader::SET_COOKIE)); ASSERT_EQ(header._first_set_cookie, nullptr); + header.SetHeader(brpc::HttpHeader::SET_COOKIE, set_cookie_value1); + for (int i = 0; i < 128; ++i) { + header.SetHeader("header" + std::to_string(i), "value"); + } + value = header.GetHeader(brpc::HttpHeader::SET_COOKIE); + ASSERT_TRUE(value && *value == set_cookie_value1); + + brpc::HttpHeader other_header; + other_header.SetHeader(brpc::HttpHeader::SET_COOKIE, set_cookie_value2); + header.Swap(other_header); + value = header.GetHeader(brpc::HttpHeader::SET_COOKIE); + ASSERT_TRUE(value && *value == set_cookie_value2); + value = other_header.GetHeader(brpc::HttpHeader::SET_COOKIE); + ASSERT_TRUE(value && *value == set_cookie_value1); + header.Clear(); + ASSERT_FALSE(header.GetHeader(brpc::HttpHeader::SET_COOKIE)); + ASSERT_EQ(brpc::HTTP_METHOD_GET, header.method()); header.set_method(brpc::HTTP_METHOD_POST); ASSERT_EQ(brpc::HTTP_METHOD_POST, header.method()); From 762c824e620cfb6943fce54b5f750073d76316e2 Mon Sep 17 00:00:00 2001 From: songgaoye Date: Wed, 14 Oct 2026 22:43:30 +0800 Subject: [PATCH 2/7] Strengthen HttpHeader rehash regression test --- test/brpc_http_message_unittest.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/brpc_http_message_unittest.cpp b/test/brpc_http_message_unittest.cpp index 8a8bd2e628..d4c6d98187 100644 --- a/test/brpc_http_message_unittest.cpp +++ b/test/brpc_http_message_unittest.cpp @@ -884,9 +884,12 @@ TEST(HttpMessageTest, http_header) { ASSERT_EQ(header._first_set_cookie, nullptr); header.SetHeader(brpc::HttpHeader::SET_COOKIE, set_cookie_value1); - for (int i = 0; i < 128; ++i) { + const size_t bucket_count = header._headers.bucket_count(); + for (int i = 0; i < 4096 && + header._headers.bucket_count() == bucket_count; ++i) { header.SetHeader("header" + std::to_string(i), "value"); } + ASSERT_NE(bucket_count, header._headers.bucket_count()); value = header.GetHeader(brpc::HttpHeader::SET_COOKIE); ASSERT_TRUE(value && *value == set_cookie_value1); From 4be80aa563cc23ac2ba6ab2162733e7809f4db2a Mon Sep 17 00:00:00 2001 From: songgaoye Date: Thu, 15 Oct 2026 13:33:18 +0800 Subject: [PATCH 3/7] Fix Set-Cookie header cache invalidation --- src/brpc/details/http_message.cpp | 3 + src/brpc/http_header.cpp | 101 +++++++++++++++++----------- src/brpc/http_header.h | 18 +++-- test/brpc_http_message_unittest.cpp | 59 +++++++++++++++- 4 files changed, 135 insertions(+), 46 deletions(-) diff --git a/src/brpc/details/http_message.cpp b/src/brpc/details/http_message.cpp index 14a81e553c..e039719ce0 100644 --- a/src/brpc/details/http_message.cpp +++ b/src/brpc/details/http_message.cpp @@ -186,6 +186,9 @@ int HttpMessage::on_headers_complete(http_parser *parser) { parser->http_major = 1; } HttpHeader& headers = http_message->header(); + if (headers.HasSetCookieHeaders()) { + headers.SyncSetCookieHeaders(); + } headers.set_version(parser->http_major, parser->http_minor); // Only for response // http_parser may set status_code to 0 when the field is not needed, diff --git a/src/brpc/http_header.cpp b/src/brpc/http_header.cpp index 942f5253f9..898c537b69 100644 --- a/src/brpc/http_header.cpp +++ b/src/brpc/http_header.cpp @@ -29,9 +29,27 @@ const char* HttpHeader::CONTENT_TYPE = "content-type"; HttpHeader::HttpHeader() : _status_code(HTTP_STATUS_OK) , _method(HTTP_METHOD_GET) - , _version(1, 1) - , _first_set_cookie(nullptr) { - // NOTE: don't forget to clear the field in Clear() as well. + , _version(1, 1) {} + +HttpHeader::HttpHeader(const HttpHeader& rhs) + : _headers(rhs._headers) + , _uri(rhs._uri) + , _status_code(rhs._status_code) + , _method(rhs._method) + , _content_type(rhs._content_type) + , _unresolved_path(rhs._unresolved_path) + , _version(rhs._version) { + if (rhs._set_cookie_headers) { + _set_cookie_headers.reset(new SetCookieHeaders(*rhs._set_cookie_headers)); + } +} + +HttpHeader& HttpHeader::operator=(const HttpHeader& rhs) { + if (this != &rhs) { + HttpHeader tmp(rhs); + Swap(tmp); + } + return *this; } void HttpHeader::Swap(HttpHeader &rhs) { @@ -42,8 +60,7 @@ void HttpHeader::Swap(HttpHeader &rhs) { _content_type.swap(rhs._content_type); _unresolved_path.swap(rhs._unresolved_path); std::swap(_version, rhs._version); - RefreshFirstSetCookie(); - rhs.RefreshFirstSetCookie(); + _set_cookie_headers.swap(rhs._set_cookie_headers); } void HttpHeader::Clear() { @@ -54,7 +71,7 @@ void HttpHeader::Clear() { _content_type.clear(); _unresolved_path.clear(); _version = std::make_pair(1, 1); - _first_set_cookie = nullptr; + _set_cookie_headers.reset(); } const std::string* HttpHeader::GetHeader(const char* key) const { @@ -63,22 +80,19 @@ const std::string* HttpHeader::GetHeader(const char* key) const { const std::string* HttpHeader::GetHeader(const std::string& key) const { if (IsSetCookie(key)) { - return _first_set_cookie; + return HasSetCookieHeaders() + ? &_set_cookie_headers->front().second : nullptr; } std::string* val = _headers.seek(key); return val; } std::vector HttpHeader::GetAllSetCookieHeader() const { - return GetMultiLineHeaders(SET_COOKIE); -} - -std::vector -HttpHeader::GetMultiLineHeaders(const std::string& key) const { std::vector headers; - for (const auto& iter : _headers) { - if (_header_key_equal(iter.first, key)) { - headers.push_back(&iter.second); + if (_set_cookie_headers) { + headers.reserve(_set_cookie_headers->size()); + for (const auto& header : *_set_cookie_headers) { + headers.push_back(&header.second); } } return headers; @@ -86,6 +100,18 @@ HttpHeader::GetMultiLineHeaders(const std::string& key) const { void HttpHeader::SetHeader(const std::string& key, const std::string& value) { + if (IsSetCookie(key)) { + if (!HasSetCookieHeaders()) { + if (!_set_cookie_headers) { + _set_cookie_headers.reset(new SetCookieHeaders); + } + _set_cookie_headers->push_back(std::make_pair(key, value)); + } else { + _set_cookie_headers->front().second = value; + } + SyncSetCookieHeaders(); + return; + } GetOrAddHeader(key) = value; } @@ -95,7 +121,7 @@ void HttpHeader::RemoveHeader(const char* key) { } else { _headers.erase(key); if (IsSetCookie(key)) { - _first_set_cookie = nullptr; + _set_cookie_headers.reset(); } } } @@ -104,8 +130,12 @@ void HttpHeader::AppendHeader(const std::string& key, const butil::StringPiece& value) { if (!CanFoldedInLine(key)) { // Add a new Set-Cookie header field. - std::string& slot = AddHeader(key); - slot.assign(value.data(), value.size()); + if (!_set_cookie_headers) { + _set_cookie_headers.reset(new SetCookieHeaders); + } + _set_cookie_headers->push_back( + std::make_pair(key, std::string(value.data(), value.size()))); + SyncSetCookieHeaders(); } else { std::string& slot = GetOrAddHeader(key); if (slot.empty()) { @@ -131,40 +161,29 @@ std::string& HttpHeader::GetOrAddHeader(const std::string& key) { return _content_type; } - bool is_set_cookie = IsSetCookie(key); - // Only returns the first Set-Cookie header field for compatibility. - if (is_set_cookie && nullptr != _first_set_cookie) { - return *_first_set_cookie; - } - std::string* val = _headers.seek(key); if (nullptr == val) { - const size_t old_bucket_count = _headers.bucket_count(); val = _headers.insert({ key, "" }); - if (is_set_cookie) { - _first_set_cookie = val; - } - if (_headers.bucket_count() != old_bucket_count) { - RefreshFirstSetCookie(); - } } return *val; } std::string& HttpHeader::AddHeader(const std::string& key) { - const size_t old_bucket_count = _headers.bucket_count(); - std::string* val = _headers.insert({ key, "" }); - if (IsSetCookie(key) && nullptr == _first_set_cookie) { - _first_set_cookie = val; + if (!_set_cookie_headers) { + _set_cookie_headers.reset(new SetCookieHeaders); } - if (_headers.bucket_count() != old_bucket_count) { - RefreshFirstSetCookie(); - } - return *val; + _set_cookie_headers->push_back(std::make_pair(key, std::string())); + SyncSetCookieHeaders(); + return _set_cookie_headers->back().second; } -void HttpHeader::RefreshFirstSetCookie() { - _first_set_cookie = _headers.seek(SET_COOKIE); +void HttpHeader::SyncSetCookieHeaders() { + _headers.erase(SET_COOKIE); + if (_set_cookie_headers) { + for (const auto& header : *_set_cookie_headers) { + _headers.insert(header); + } + } } const HttpHeader& DefaultHttpHeader() { diff --git a/src/brpc/http_header.h b/src/brpc/http_header.h index 40ec82be07..e7db3702b5 100644 --- a/src/brpc/http_header.h +++ b/src/brpc/http_header.h @@ -19,6 +19,8 @@ #ifndef BRPC_HTTP_HEADER_H #define BRPC_HTTP_HEADER_H +#include +#include #include #include "butil/strings/string_piece.h" // StringPiece #include "butil/containers/case_ignored_flat_map.h" @@ -45,6 +47,8 @@ class HttpHeader { typedef HeaderMap::key_equal HeaderKeyEqual; HttpHeader(); + HttpHeader(const HttpHeader& rhs); + HttpHeader& operator=(const HttpHeader& rhs); // Exchange internal fields with another HttpHeader. void Swap(HttpHeader &rhs); @@ -165,18 +169,20 @@ friend void policy::ProcessHttpRequest(InputMessageBase *msg); static const char* COOKIE; static const char* CONTENT_TYPE; - std::vector GetMultiLineHeaders(const std::string& key) const; - std::string& GetOrAddHeader(const std::string& key); std::string& AddHeader(const std::string& key); - void RefreshFirstSetCookie(); + void SyncSetCookieHeaders(); bool IsSetCookie(const std::string& key) const { return _header_key_equal(key, SET_COOKIE); } + bool HasSetCookieHeaders() const { + return _set_cookie_headers && !_set_cookie_headers->empty(); + } + bool IsCookie(const std::string& key) const { return _header_key_equal(key, COOKIE); } @@ -204,7 +210,11 @@ friend void policy::ProcessHttpRequest(InputMessageBase *msg); std::string _content_type; std::string _unresolved_path; std::pair _version; - std::string* _first_set_cookie; + // HeaderMap rehashing invalidates value pointers and does not preserve + // duplicate-key order. deque keeps existing value pointers stable when + // another Set-Cookie header is appended. + typedef std::deque > SetCookieHeaders; + std::unique_ptr _set_cookie_headers; }; const HttpHeader& DefaultHttpHeader(); diff --git a/test/brpc_http_message_unittest.cpp b/test/brpc_http_message_unittest.cpp index d4c6d98187..6b34751a89 100644 --- a/test/brpc_http_message_unittest.cpp +++ b/test/brpc_http_message_unittest.cpp @@ -341,6 +341,25 @@ TEST(HttpMessageTest, parse_http_set_cookie) { } ASSERT_EQ(1, set_cookie_value1_count); ASSERT_EQ(1, set_cookie_value2_count); + + const char first_part[] = "HTTP/1.1 200 OK\r\nSet-Cookie: a"; + const char second_part[] = "=1\r\nSet-Cookie: b=2\r\n\r\n"; + brpc::HttpMessage fragmented_message(false, brpc::HTTP_METHOD_HEAD); + ASSERT_EQ((ssize_t)sizeof(first_part) - 1, + fragmented_message.ParseFromArray(first_part, + sizeof(first_part) - 1)); + ASSERT_FALSE(fragmented_message.Completed()); + ASSERT_EQ((ssize_t)sizeof(second_part) - 1, + fragmented_message.ParseFromArray(second_part, + sizeof(second_part) - 1)); + ASSERT_TRUE(fragmented_message.Completed()); + set_cookie = fragmented_message.header().GetHeader("set-cookie"); + ASSERT_NE(nullptr, set_cookie); + ASSERT_EQ("a=1", *set_cookie); + all_set_cookie = fragmented_message.header().GetAllSetCookieHeader(); + ASSERT_EQ(2u, all_set_cookie.size()); + ASSERT_EQ("a=1", *all_set_cookie[0]); + ASSERT_EQ("b=2", *all_set_cookie[1]); } TEST(HttpMessageTest, cl_and_te) { @@ -852,7 +871,9 @@ TEST(HttpMessageTest, http_header) { header.SetHeader(brpc::HttpHeader::SET_COOKIE, set_cookie_value1); value = header.GetHeader(brpc::HttpHeader::SET_COOKIE); ASSERT_TRUE(value && *value == set_cookie_value1); + const std::string* first_set_cookie = value; header.AppendHeader(brpc::HttpHeader::SET_COOKIE, set_cookie_value2); + ASSERT_EQ(set_cookie_value1, *first_set_cookie); value = header.GetHeader(brpc::HttpHeader::SET_COOKIE); ASSERT_TRUE(value && *value == set_cookie_value1); header.SetHeader(brpc::HttpHeader::SET_COOKIE, set_cookie_value3); @@ -879,19 +900,55 @@ TEST(HttpMessageTest, http_header) { } ASSERT_EQ(1, set_cookie_value2_count); ASSERT_EQ(1, set_cookie_value3_count); + brpc::HttpHeader copied_header(header); + copied_header.SetHeader(brpc::HttpHeader::SET_COOKIE, set_cookie_value1); + value = header.GetHeader(brpc::HttpHeader::SET_COOKIE); + ASSERT_TRUE(value && *value == set_cookie_value3); + value = copied_header.GetHeader(brpc::HttpHeader::SET_COOKIE); + ASSERT_TRUE(value && *value == set_cookie_value1); + brpc::HttpHeader assigned_header; + assigned_header = header; + assigned_header.SetHeader(brpc::HttpHeader::SET_COOKIE, set_cookie_value2); + value = header.GetHeader(brpc::HttpHeader::SET_COOKIE); + ASSERT_TRUE(value && *value == set_cookie_value3); + value = assigned_header.GetHeader(brpc::HttpHeader::SET_COOKIE); + ASSERT_TRUE(value && *value == set_cookie_value2); header.RemoveHeader(brpc::HttpHeader::SET_COOKIE); ASSERT_FALSE(header.GetHeader(brpc::HttpHeader::SET_COOKIE)); - ASSERT_EQ(header._first_set_cookie, nullptr); + ASSERT_TRUE(header.GetAllSetCookieHeader().empty()); header.SetHeader(brpc::HttpHeader::SET_COOKIE, set_cookie_value1); + const std::string* first_set_cookie_after_reset = + header.GetHeader(brpc::HttpHeader::SET_COOKIE); + ASSERT_NE(nullptr, first_set_cookie_after_reset); + header.AppendHeader(brpc::HttpHeader::SET_COOKIE, set_cookie_value2); const size_t bucket_count = header._headers.bucket_count(); for (int i = 0; i < 4096 && header._headers.bucket_count() == bucket_count; ++i) { header.SetHeader("header" + std::to_string(i), "value"); } ASSERT_NE(bucket_count, header._headers.bucket_count()); + ASSERT_EQ(set_cookie_value1, *first_set_cookie_after_reset); value = header.GetHeader(brpc::HttpHeader::SET_COOKIE); ASSERT_TRUE(value && *value == set_cookie_value1); + all_set_cookie = header.GetAllSetCookieHeader(); + ASSERT_EQ(2u, all_set_cookie.size()); + ASSERT_EQ(set_cookie_value1, *all_set_cookie[0]); + ASSERT_EQ(set_cookie_value2, *all_set_cookie[1]); + int set_cookie_value1_count = 0; + set_cookie_value2_count = 0; + for (auto iter = header.HeaderBegin(); iter != header.HeaderEnd(); ++iter) { + if (!header.IsSetCookie(iter->first)) { + continue; + } + if (iter->second == set_cookie_value1) { + ++set_cookie_value1_count; + } else if (iter->second == set_cookie_value2) { + ++set_cookie_value2_count; + } + } + ASSERT_EQ(1, set_cookie_value1_count); + ASSERT_EQ(1, set_cookie_value2_count); brpc::HttpHeader other_header; other_header.SetHeader(brpc::HttpHeader::SET_COOKIE, set_cookie_value2); From b6001dc7745c850b20318c41dc0b78ab362ecc32 Mon Sep 17 00:00:00 2001 From: songgaoye Date: Fri, 16 Oct 2026 13:54:03 +0800 Subject: [PATCH 4/7] Preserve AddHeader behavior for non-cookie headers --- src/brpc/http_header.cpp | 3 +++ test/brpc_http_message_unittest.cpp | 17 +++++++---------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/brpc/http_header.cpp b/src/brpc/http_header.cpp index 898c537b69..49ff99848e 100644 --- a/src/brpc/http_header.cpp +++ b/src/brpc/http_header.cpp @@ -169,6 +169,9 @@ std::string& HttpHeader::GetOrAddHeader(const std::string& key) { } std::string& HttpHeader::AddHeader(const std::string& key) { + if (!IsSetCookie(key)) { + return *_headers.insert({ key, "" }); + } if (!_set_cookie_headers) { _set_cookie_headers.reset(new SetCookieHeaders); } diff --git a/test/brpc_http_message_unittest.cpp b/test/brpc_http_message_unittest.cpp index 6b34751a89..254db0ff1e 100644 --- a/test/brpc_http_message_unittest.cpp +++ b/test/brpc_http_message_unittest.cpp @@ -923,10 +923,7 @@ TEST(HttpMessageTest, http_header) { ASSERT_NE(nullptr, first_set_cookie_after_reset); header.AppendHeader(brpc::HttpHeader::SET_COOKIE, set_cookie_value2); const size_t bucket_count = header._headers.bucket_count(); - for (int i = 0; i < 4096 && - header._headers.bucket_count() == bucket_count; ++i) { - header.SetHeader("header" + std::to_string(i), "value"); - } + ASSERT_TRUE(header._headers.resize(bucket_count * 2)); ASSERT_NE(bucket_count, header._headers.bucket_count()); ASSERT_EQ(set_cookie_value1, *first_set_cookie_after_reset); value = header.GetHeader(brpc::HttpHeader::SET_COOKIE); @@ -935,20 +932,20 @@ TEST(HttpMessageTest, http_header) { ASSERT_EQ(2u, all_set_cookie.size()); ASSERT_EQ(set_cookie_value1, *all_set_cookie[0]); ASSERT_EQ(set_cookie_value2, *all_set_cookie[1]); - int set_cookie_value1_count = 0; - set_cookie_value2_count = 0; + int set_cookie_value1_after_rehash_count = 0; + int set_cookie_value2_after_rehash_count = 0; for (auto iter = header.HeaderBegin(); iter != header.HeaderEnd(); ++iter) { if (!header.IsSetCookie(iter->first)) { continue; } if (iter->second == set_cookie_value1) { - ++set_cookie_value1_count; + ++set_cookie_value1_after_rehash_count; } else if (iter->second == set_cookie_value2) { - ++set_cookie_value2_count; + ++set_cookie_value2_after_rehash_count; } } - ASSERT_EQ(1, set_cookie_value1_count); - ASSERT_EQ(1, set_cookie_value2_count); + ASSERT_EQ(1, set_cookie_value1_after_rehash_count); + ASSERT_EQ(1, set_cookie_value2_after_rehash_count); brpc::HttpHeader other_header; other_header.SetHeader(brpc::HttpHeader::SET_COOKIE, set_cookie_value2); From e9a11288e880b593340079a5cfc81e676faef76c Mon Sep 17 00:00:00 2001 From: songgaoye Date: Sun, 18 Oct 2026 08:56:27 +0800 Subject: [PATCH 5/7] Avoid repeated Set-Cookie header synchronization --- src/brpc/http_header.cpp | 24 ++++++++++++++++++++---- src/brpc/http_header.h | 9 ++++++++- test/brpc_http_message_unittest.cpp | 2 +- 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/src/brpc/http_header.cpp b/src/brpc/http_header.cpp index 49ff99848e..0fa8aa04e3 100644 --- a/src/brpc/http_header.cpp +++ b/src/brpc/http_header.cpp @@ -29,7 +29,8 @@ const char* HttpHeader::CONTENT_TYPE = "content-type"; HttpHeader::HttpHeader() : _status_code(HTTP_STATUS_OK) , _method(HTTP_METHOD_GET) - , _version(1, 1) {} + , _version(1, 1) + , _set_cookie_headers_dirty(false) {} HttpHeader::HttpHeader(const HttpHeader& rhs) : _headers(rhs._headers) @@ -38,7 +39,8 @@ HttpHeader::HttpHeader(const HttpHeader& rhs) , _method(rhs._method) , _content_type(rhs._content_type) , _unresolved_path(rhs._unresolved_path) - , _version(rhs._version) { + , _version(rhs._version) + , _set_cookie_headers_dirty(rhs._set_cookie_headers_dirty) { if (rhs._set_cookie_headers) { _set_cookie_headers.reset(new SetCookieHeaders(*rhs._set_cookie_headers)); } @@ -61,6 +63,7 @@ void HttpHeader::Swap(HttpHeader &rhs) { _unresolved_path.swap(rhs._unresolved_path); std::swap(_version, rhs._version); _set_cookie_headers.swap(rhs._set_cookie_headers); + std::swap(_set_cookie_headers_dirty, rhs._set_cookie_headers_dirty); } void HttpHeader::Clear() { @@ -72,6 +75,7 @@ void HttpHeader::Clear() { _unresolved_path.clear(); _version = std::make_pair(1, 1); _set_cookie_headers.reset(); + _set_cookie_headers_dirty = false; } const std::string* HttpHeader::GetHeader(const char* key) const { @@ -109,6 +113,7 @@ void HttpHeader::SetHeader(const std::string& key, } else { _set_cookie_headers->front().second = value; } + _set_cookie_headers_dirty = true; SyncSetCookieHeaders(); return; } @@ -122,6 +127,7 @@ void HttpHeader::RemoveHeader(const char* key) { _headers.erase(key); if (IsSetCookie(key)) { _set_cookie_headers.reset(); + _set_cookie_headers_dirty = false; } } } @@ -133,9 +139,12 @@ void HttpHeader::AppendHeader(const std::string& key, if (!_set_cookie_headers) { _set_cookie_headers.reset(new SetCookieHeaders); } + if (_set_cookie_headers_dirty) { + SyncSetCookieHeaders(); + } _set_cookie_headers->push_back( std::make_pair(key, std::string(value.data(), value.size()))); - SyncSetCookieHeaders(); + _headers.insert(_set_cookie_headers->back()); } else { std::string& slot = GetOrAddHeader(key); if (slot.empty()) { @@ -175,18 +184,25 @@ std::string& HttpHeader::AddHeader(const std::string& key) { if (!_set_cookie_headers) { _set_cookie_headers.reset(new SetCookieHeaders); } + if (!_set_cookie_headers_dirty) { + _headers.erase(SET_COOKIE); + _set_cookie_headers_dirty = true; + } _set_cookie_headers->push_back(std::make_pair(key, std::string())); - SyncSetCookieHeaders(); return _set_cookie_headers->back().second; } void HttpHeader::SyncSetCookieHeaders() { + if (!_set_cookie_headers_dirty) { + return; + } _headers.erase(SET_COOKIE); if (_set_cookie_headers) { for (const auto& header : *_set_cookie_headers) { _headers.insert(header); } } + _set_cookie_headers_dirty = false; } const HttpHeader& DefaultHttpHeader() { diff --git a/src/brpc/http_header.h b/src/brpc/http_header.h index e7db3702b5..ea85a47e52 100644 --- a/src/brpc/http_header.h +++ b/src/brpc/http_header.h @@ -124,7 +124,12 @@ class HttpHeader { HeaderIterator HeaderBegin() const { return _headers.begin(); } HeaderIterator HeaderEnd() const { return _headers.end(); } // #headers - size_t HeaderCount() const { return _headers.size(); } + size_t HeaderCount() const { + if (_set_cookie_headers_dirty) { + return _headers.size() + _set_cookie_headers->size(); + } + return _headers.size(); + } // Get the URI object, check src/brpc/uri.h for details. const URI& uri() const { return _uri; } @@ -215,6 +220,8 @@ friend void policy::ProcessHttpRequest(InputMessageBase *msg); // another Set-Cookie header is appended. typedef std::deque > SetCookieHeaders; std::unique_ptr _set_cookie_headers; + // When true, Set-Cookie headers are only stored in _set_cookie_headers. + bool _set_cookie_headers_dirty; }; const HttpHeader& DefaultHttpHeader(); diff --git a/test/brpc_http_message_unittest.cpp b/test/brpc_http_message_unittest.cpp index 254db0ff1e..24e60ad023 100644 --- a/test/brpc_http_message_unittest.cpp +++ b/test/brpc_http_message_unittest.cpp @@ -889,7 +889,7 @@ TEST(HttpMessageTest, http_header) { int set_cookie_value2_count = 0; int set_cookie_value3_count = 0; for (auto iter = header.HeaderBegin(); iter != header.HeaderEnd(); ++iter) { - if (!header.IsSetCookie(brpc::HttpHeader::SET_COOKIE)) { + if (!header.IsSetCookie(iter->first)) { continue; } if (iter->second == set_cookie_value2) { From c2df88f352e7ad797329cd7186b1927357445904 Mon Sep 17 00:00:00 2001 From: songgaoye Date: Thu, 22 Oct 2026 23:10:17 +0800 Subject: [PATCH 6/7] Add Set-Cookie collision regression test --- test/brpc_http_message_unittest.cpp | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/brpc_http_message_unittest.cpp b/test/brpc_http_message_unittest.cpp index 24e60ad023..916a6e482e 100644 --- a/test/brpc_http_message_unittest.cpp +++ b/test/brpc_http_message_unittest.cpp @@ -879,6 +879,20 @@ TEST(HttpMessageTest, http_header) { header.SetHeader(brpc::HttpHeader::SET_COOKIE, set_cookie_value3); value = header.GetHeader(brpc::HttpHeader::SET_COOKIE); ASSERT_TRUE(value && *value == set_cookie_value3); + + // set-cookie and user-agent collide in the default HeaderMap bucket + // layout. Updating Set-Cookie must not corrupt the surviving header. + brpc::HttpHeader collision_header; + collision_header.SetHeader(brpc::HttpHeader::SET_COOKIE, + set_cookie_value1); + collision_header.SetHeader("user-agent", "brpc"); + collision_header.SetHeader(brpc::HttpHeader::SET_COOKIE, + set_cookie_value2); + value = collision_header.GetHeader(brpc::HttpHeader::SET_COOKIE); + ASSERT_TRUE(value && *value == set_cookie_value2); + value = collision_header.GetHeader("user-agent"); + ASSERT_TRUE(value && *value == "brpc"); + std::vector all_set_cookie = header.GetAllSetCookieHeader(); ASSERT_EQ(2u, all_set_cookie.size()); From adf42b1effd16d3e68ce1f842e560b9a1e9588e4 Mon Sep 17 00:00:00 2001 From: songgaoye Date: Sun, 25 Oct 2026 19:48:34 +0800 Subject: [PATCH 7/7] Simplify Set-Cookie header storage --- src/brpc/http_header.cpp | 19 +++++++++---------- src/brpc/http_header.h | 2 +- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/brpc/http_header.cpp b/src/brpc/http_header.cpp index 0fa8aa04e3..1404940ec5 100644 --- a/src/brpc/http_header.cpp +++ b/src/brpc/http_header.cpp @@ -85,7 +85,7 @@ const std::string* HttpHeader::GetHeader(const char* key) const { const std::string* HttpHeader::GetHeader(const std::string& key) const { if (IsSetCookie(key)) { return HasSetCookieHeaders() - ? &_set_cookie_headers->front().second : nullptr; + ? &_set_cookie_headers->front() : nullptr; } std::string* val = _headers.seek(key); return val; @@ -96,7 +96,7 @@ std::vector HttpHeader::GetAllSetCookieHeader() const { if (_set_cookie_headers) { headers.reserve(_set_cookie_headers->size()); for (const auto& header : *_set_cookie_headers) { - headers.push_back(&header.second); + headers.push_back(&header); } } return headers; @@ -109,9 +109,9 @@ void HttpHeader::SetHeader(const std::string& key, if (!_set_cookie_headers) { _set_cookie_headers.reset(new SetCookieHeaders); } - _set_cookie_headers->push_back(std::make_pair(key, value)); + _set_cookie_headers->push_back(value); } else { - _set_cookie_headers->front().second = value; + _set_cookie_headers->front() = value; } _set_cookie_headers_dirty = true; SyncSetCookieHeaders(); @@ -142,9 +142,8 @@ void HttpHeader::AppendHeader(const std::string& key, if (_set_cookie_headers_dirty) { SyncSetCookieHeaders(); } - _set_cookie_headers->push_back( - std::make_pair(key, std::string(value.data(), value.size()))); - _headers.insert(_set_cookie_headers->back()); + _set_cookie_headers->push_back(std::string(value.data(), value.size())); + _headers.insert({ SET_COOKIE, _set_cookie_headers->back() }); } else { std::string& slot = GetOrAddHeader(key); if (slot.empty()) { @@ -188,8 +187,8 @@ std::string& HttpHeader::AddHeader(const std::string& key) { _headers.erase(SET_COOKIE); _set_cookie_headers_dirty = true; } - _set_cookie_headers->push_back(std::make_pair(key, std::string())); - return _set_cookie_headers->back().second; + _set_cookie_headers->push_back(std::string()); + return _set_cookie_headers->back(); } void HttpHeader::SyncSetCookieHeaders() { @@ -199,7 +198,7 @@ void HttpHeader::SyncSetCookieHeaders() { _headers.erase(SET_COOKIE); if (_set_cookie_headers) { for (const auto& header : *_set_cookie_headers) { - _headers.insert(header); + _headers.insert({ SET_COOKIE, header }); } } _set_cookie_headers_dirty = false; diff --git a/src/brpc/http_header.h b/src/brpc/http_header.h index ea85a47e52..cbaaf64157 100644 --- a/src/brpc/http_header.h +++ b/src/brpc/http_header.h @@ -218,7 +218,7 @@ friend void policy::ProcessHttpRequest(InputMessageBase *msg); // HeaderMap rehashing invalidates value pointers and does not preserve // duplicate-key order. deque keeps existing value pointers stable when // another Set-Cookie header is appended. - typedef std::deque > SetCookieHeaders; + typedef std::deque SetCookieHeaders; std::unique_ptr _set_cookie_headers; // When true, Set-Cookie headers are only stored in _set_cookie_headers. bool _set_cookie_headers_dirty;