You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by bn...@apache.org on 2026/09/14 22:28:39 UTC
(trafficserver) branch master updated: Reuse deleted MIME field slots (#13455)
This is an automated email from the ASF dual-hosted git repository.
bneradt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new c6ea03da66 Reuse deleted MIME field slots (#13455)
c6ea03da66 is described below
commit c6ea03da669ce6a4b527f5768f581eb391b05872
Author: Brian Neradt <br...@gmail.com>
AuthorDate: Mon Sep 14 17:28:32 2026 -0500
Reuse deleted MIME field slots (#13455)
Long-lived MIME headers can grow field-block chains as fields are added
and removed. Maintain a deleted-slot free list and consume it before
allocating another block, while preserving duplicate insertion order.
Skip duplicate lookup when the full tail has no reusable slots. Add
churn, ordering, copy, marshal, and HPACK decoder coverage.
Fixes: #8466
---
include/proxy/hdrs/MIME.h | 32 ++--
include/proxy/http2/HPACK.h | 16 +-
src/api/InkAPI.cc | 2 +-
src/proxy/hdrs/MIME.cc | 163 +++++++++++++-----
src/proxy/hdrs/unit_tests/test_Hdrs.cc | 190 +++++++++++++++++++++
src/proxy/http2/HPACK.cc | 13 +-
.../http2/unit_tests/test_HpackIndexingTable.cc | 6 +-
7 files changed, 358 insertions(+), 64 deletions(-)
diff --git a/include/proxy/hdrs/MIME.h b/include/proxy/hdrs/MIME.h
index 210b77489a..5eb53517d9 100644
--- a/include/proxy/hdrs/MIME.h
+++ b/include/proxy/hdrs/MIME.h
@@ -90,6 +90,9 @@ enum class MimeParseState {
#define MIME_FIELD_SLOTNUM_MAX (MIME_FIELD_SLOTNUM_MASK - 1)
#define MIME_FIELD_SLOTNUM_UNKNOWN MIME_FIELD_SLOTNUM_MAX
+#define MIME_FIELD_FREE_SLOT_NONE -1
+#define MIME_FIELD_FREE_SLOT_UNINITIALIZED -2
+
/***********************************************************************
* *
* MIMEField & MIMEFieldBlockImpl *
@@ -99,16 +102,19 @@ enum class MimeParseState {
struct MIMEHdrImpl;
struct MIMEField {
- const char *m_ptr_name; // 4
- const char *m_ptr_value; // 4
- MIMEField *m_next_dup; // 4
- int16_t m_wks_idx; // 2
- uint16_t m_len_name; // 2
- uint32_t m_len_value : 24; // 3
- uint8_t m_n_v_raw_printable : 1; // 1/8
- uint8_t m_n_v_raw_printable_pad : 3; // 3/8
- uint8_t m_readiness : 2; // 2/8
- uint8_t m_flags : 2; // 2/8
+ const char *m_ptr_name; // 4
+ const char *m_ptr_value; // 4
+ union {
+ MIMEField *m_next_dup; // 4
+ int32_t m_free_next; ///< Slot number of the next deleted field.
+ };
+ int16_t m_wks_idx; // 2
+ uint16_t m_len_name; // 2
+ uint32_t m_len_value : 24; // 3
+ uint8_t m_n_v_raw_printable : 1; // 1/8
+ uint8_t m_n_v_raw_printable_pad : 3; // 3/8
+ uint8_t m_readiness : 2; // 2/8
+ uint8_t m_flags : 2; // 2/8
bool
is_dup_head() const
@@ -304,7 +310,8 @@ struct MIMEHdrImpl : public HdrHeapObjImpl {
friend struct MIMEHdrImpl;
};
- // HdrHeapObjImpl is 4 bytes, so this will result in 4 bytes padding
+ // HdrHeapObjImpl is 4 bytes, so this uses the 4 bytes that would otherwise be padding.
+ int32_t m_free_slot; ///< Slot number at the head of the deleted field free list.
uint64_t m_presence_bits;
uint32_t m_slot_accelerators[4];
@@ -748,6 +755,7 @@ int mime_hdr_fields_count(MIMEHdrImpl *mh);
void mime_field_init(MIMEField *field);
MIMEField *mime_field_create(HdrHeap *heap, MIMEHdrImpl *mh);
+MIMEField *mime_field_create_for_name(HdrHeap *heap, MIMEHdrImpl *mh, std::string_view name);
MIMEField *mime_field_create_named(HdrHeap *heap, MIMEHdrImpl *mh, std::string_view name);
void mime_hdr_field_attach(MIMEHdrImpl *mh, MIMEField *field, int check_for_dups, MIMEField *prev_dup);
@@ -1183,7 +1191,7 @@ MIMEHdr::fields_count() const
inline MIMEField *
MIMEHdr::field_create(std::string_view name)
{
- MIMEField *field = mime_field_create(m_heap, m_mime);
+ MIMEField *field = name.empty() ? mime_field_create(m_heap, m_mime) : mime_field_create_for_name(m_heap, m_mime, name);
if (!name.empty()) {
auto length{static_cast<int>(name.length())};
diff --git a/include/proxy/http2/HPACK.h b/include/proxy/http2/HPACK.h
index f9fcc0a9dc..446fcbb8aa 100644
--- a/include/proxy/http2/HPACK.h
+++ b/include/proxy/http2/HPACK.h
@@ -71,10 +71,16 @@ class MIMEFieldWrapper
{
public:
MIMEFieldWrapper(MIMEField *f, HdrHeap *hh, MIMEHdrImpl *impl) : _field(f), _heap(hh), _mh(impl) {}
+ MIMEFieldWrapper(HdrHeap *hh, MIMEHdrImpl *impl) : _heap(hh), _mh(impl) {}
bool
name_set(const char *name, int name_len)
{
- return _field->name_set(_heap, _mh, std::string_view{name, static_cast<std::string_view::size_type>(name_len)});
+ std::string_view name_view{name, static_cast<std::string_view::size_type>(name_len)};
+
+ if (_field == nullptr) {
+ _field = mime_field_create_for_name(_heap, _mh, name_view);
+ }
+ return _field->name_set(_heap, _mh, name_view);
}
bool
@@ -101,8 +107,14 @@ public:
return _field;
}
+ MIMEField *
+ field_get()
+ {
+ return _field;
+ }
+
private:
- MIMEField *_field;
+ MIMEField *_field = nullptr;
HdrHeap *_heap;
MIMEHdrImpl *_mh;
};
diff --git a/src/api/InkAPI.cc b/src/api/InkAPI.cc
index aef48debd8..e07923119d 100644
--- a/src/api/InkAPI.cc
+++ b/src/api/InkAPI.cc
@@ -1787,7 +1787,7 @@ TSMimeHdrFieldAppend(TSMBuffer bufp, TSMLoc mh_mloc, TSMLoc field_mloc)
HdrHeap *heap = ((reinterpret_cast<HdrHeapSDKHandle *>(bufp))->m_heap);
// allocate a new hdr field and copy any pre-set info
- mh_field = mime_field_create(heap, mh);
+ mh_field = mime_field_create_for_name(heap, mh, field_handle->field_ptr->name_get());
// FIX: is it safe to copy everything over?
memcpy(mh_field, field_handle->field_ptr, sizeof(MIMEField));
diff --git a/src/proxy/hdrs/MIME.cc b/src/proxy/hdrs/MIME.cc
index a83bd35067..26aaeaa410 100644
--- a/src/proxy/hdrs/MIME.cc
+++ b/src/proxy/hdrs/MIME.cc
@@ -542,6 +542,7 @@ mime_hdr_sanity_check(MIMEHdrImpl *mh)
MIMEField *field, *next_dup;
uint32_t slot_index, index;
uint64_t masksum;
+ size_t deleted_count = 0;
ink_assert(mh != nullptr);
@@ -617,13 +618,15 @@ mime_hdr_sanity_check(MIMEHdrImpl *mh)
ink_release_assert(found);
}
// re-find the field --- should always find the head dup
- MIMEField *mf = mime_hdr_field_find(mh, field->m_ptr_name, field->m_len_name);
+ MIMEField *mf = mime_hdr_field_find(mh, {field->m_ptr_name, field->m_len_name});
ink_release_assert(mf != nullptr);
if (mf == field) {
ink_release_assert((field->m_flags & MIME_FIELD_SLOT_FLAGS_DUP_HEAD) != 0);
} else {
ink_release_assert((field->m_flags & MIME_FIELD_SLOT_FLAGS_DUP_HEAD) == 0);
}
+ } else if (field->m_readiness == MIME_FIELD_SLOT_READINESS_DELETED) {
+ ++deleted_count;
}
++slot_index;
@@ -633,6 +636,20 @@ mime_hdr_sanity_check(MIMEHdrImpl *mh)
ink_release_assert(last_fblock == mh->m_fblock_list_tail);
ink_release_assert(masksum == mh->m_presence_bits);
+
+ if (mh->m_free_slot != MIME_FIELD_FREE_SLOT_UNINITIALIZED) {
+ size_t free_count = 0;
+ int32_t free_slot = mh->m_free_slot;
+
+ while (free_slot != MIME_FIELD_FREE_SLOT_NONE) {
+ field = mime_hdr_field_get_slotnum(mh, free_slot);
+ ink_release_assert(field != nullptr);
+ ink_release_assert(field->m_readiness == MIME_FIELD_SLOT_READINESS_DELETED);
+ ink_release_assert(++free_count <= deleted_count);
+ free_slot = field->m_free_next;
+ }
+ ink_release_assert(free_count == deleted_count);
+ }
}
#endif
@@ -918,6 +935,8 @@ mime_hdr_cooked_stuff_init(MIMEHdrImpl *mh, MIMEField *changing_field_or_null)
void
mime_hdr_init(MIMEHdrImpl *mh)
{
+ mh->m_free_slot = MIME_FIELD_FREE_SLOT_NONE;
+
mime_hdr_init_accelerators_and_presence_bits(mh);
mime_hdr_cooked_stuff_init(mh, nullptr);
@@ -971,6 +990,26 @@ mime_hdr_destroy(HdrHeap *heap, MIMEHdrImpl *mh)
// heap->deallocate_obj(mh);
}
+static void
+mime_hdr_rebuild_field_free_list(MIMEHdrImpl *mh)
+{
+ int32_t slotnum = 0;
+
+ mh->m_free_slot = MIME_FIELD_FREE_SLOT_NONE;
+ for (MIMEFieldBlockImpl *fblock = &mh->m_first_fblock; fblock != nullptr; fblock = fblock->m_next) {
+ for (uint32_t index = 0; index < fblock->m_freetop; ++index) {
+ MIMEField *field = &fblock->m_field_slots[index];
+
+ if (field->m_readiness == MIME_FIELD_SLOT_READINESS_DELETED || field->m_readiness == MIME_FIELD_SLOT_READINESS_EMPTY) {
+ field->m_readiness = MIME_FIELD_SLOT_READINESS_DELETED;
+ field->m_free_next = mh->m_free_slot;
+ mh->m_free_slot = slotnum + static_cast<int32_t>(index);
+ }
+ }
+ slotnum += MIME_FIELD_BLOCK_SLOTS;
+ }
+}
+
void
mime_hdr_copy_onto(MIMEHdrImpl *s_mh, HdrHeap *s_heap, MIMEHdrImpl *d_mh, HdrHeap *d_heap, bool inherit_strs)
{
@@ -1016,6 +1055,7 @@ mime_hdr_copy_onto(MIMEHdrImpl *s_mh, HdrHeap *s_heap, MIMEHdrImpl *d_mh, HdrHea
}
mime_hdr_field_block_list_adjust(block_count, &(s_mh->m_first_fblock), &(d_mh->m_first_fblock));
+ mime_hdr_rebuild_field_free_list(d_mh);
MIME_HDR_SANITY_CHECK(s_mh);
MIME_HDR_SANITY_CHECK(d_mh);
@@ -1314,14 +1354,20 @@ mime_field_create(HdrHeap *heap, MIMEHdrImpl *mh)
MIMEFieldBlockImpl *tail_fblock, *new_fblock;
tail_fblock = mh->m_fblock_list_tail;
- if (tail_fblock->m_freetop >= MIME_FIELD_BLOCK_SLOTS) {
- new_fblock = (MIMEFieldBlockImpl *)heap->allocate_obj(sizeof(MIMEFieldBlockImpl), HdrHeapObjType::FIELD_BLOCK);
- _mime_hdr_field_block_init(new_fblock);
- tail_fblock->m_next = new_fblock;
- tail_fblock = new_fblock;
- mh->m_fblock_list_tail = new_fblock;
+ if (tail_fblock->m_freetop < MIME_FIELD_BLOCK_SLOTS) {
+ field = &(tail_fblock->m_field_slots[tail_fblock->m_freetop]);
+ ++tail_fblock->m_freetop;
+
+ mime_field_init(field);
+ return field;
}
+ new_fblock = (MIMEFieldBlockImpl *)heap->allocate_obj(sizeof(MIMEFieldBlockImpl), HdrHeapObjType::FIELD_BLOCK);
+ _mime_hdr_field_block_init(new_fblock);
+ tail_fblock->m_next = new_fblock;
+ tail_fblock = new_fblock;
+ mh->m_fblock_list_tail = new_fblock;
+
field = &(tail_fblock->m_field_slots[tail_fblock->m_freetop]);
++tail_fblock->m_freetop;
@@ -1330,10 +1376,63 @@ mime_field_create(HdrHeap *heap, MIMEHdrImpl *mh)
return field;
}
+MIMEField *
+mime_field_create_for_name(HdrHeap *heap, MIMEHdrImpl *mh, std::string_view name)
+{
+ if (mh->m_fblock_list_tail->m_freetop < MIME_FIELD_BLOCK_SLOTS) {
+ return mime_field_create(heap, mh);
+ }
+
+ if (mh->m_free_slot == MIME_FIELD_FREE_SLOT_UNINITIALIZED) {
+ mime_hdr_rebuild_field_free_list(mh);
+ }
+
+ if (mh->m_free_slot == MIME_FIELD_FREE_SLOT_NONE) {
+ return mime_field_create(heap, mh);
+ }
+
+ int last_dup_slot = -1;
+
+ if (MIMEField *last_dup = name.empty() ? nullptr : mime_hdr_field_find(mh, name); last_dup != nullptr) {
+ while (last_dup->m_next_dup != nullptr) {
+ last_dup = last_dup->m_next_dup;
+ }
+ last_dup_slot = mime_hdr_field_slotnum(mh, last_dup);
+ ink_release_assert(last_dup_slot >= 0);
+ }
+
+ int32_t previous_free_slot = MIME_FIELD_FREE_SLOT_NONE;
+ int32_t free_slot = mh->m_free_slot;
+
+ while (free_slot != MIME_FIELD_FREE_SLOT_NONE) {
+ MIMEField *field = mime_hdr_field_get_slotnum(mh, free_slot);
+ ink_release_assert(field != nullptr);
+ ink_release_assert(field->m_readiness == MIME_FIELD_SLOT_READINESS_DELETED);
+
+ if (free_slot > last_dup_slot) {
+ if (previous_free_slot == MIME_FIELD_FREE_SLOT_NONE) {
+ mh->m_free_slot = field->m_free_next;
+ } else {
+ MIMEField *previous_free = mime_hdr_field_get_slotnum(mh, previous_free_slot);
+
+ ink_release_assert(previous_free != nullptr);
+ previous_free->m_free_next = field->m_free_next;
+ }
+ mime_field_init(field);
+ return field;
+ }
+
+ previous_free_slot = free_slot;
+ free_slot = field->m_free_next;
+ }
+
+ return mime_field_create(heap, mh);
+}
+
MIMEField *
mime_field_create_named(HdrHeap *heap, MIMEHdrImpl *mh, std::string_view name)
{
- MIMEField *field = mime_field_create(heap, mh);
+ MIMEField *field = mime_field_create_for_name(heap, mh, name);
int field_name_wks_idx = hdrtoken_tokenize(name.data(), static_cast<int>(name.length()));
if (!mime_field_name_set(heap, mh, field, field_name_wks_idx, name, true)) {
// The name exceeds the uint16_t field-length limit and was rejected. Tear
@@ -1532,32 +1631,6 @@ mime_hdr_field_delete(HdrHeap *heap, MIMEHdrImpl *mh, MIMEField *field, bool del
MIME_HDR_SANITY_CHECK(mh);
mime_field_destroy(mh, field);
-
- MIMEFieldBlockImpl *prev_block = nullptr;
- bool can_destroy_block = true;
- for (auto fblock = &(mh->m_first_fblock); fblock != nullptr; fblock = fblock->m_next) {
- if (prev_block != nullptr) {
- if (fblock->m_freetop == MIME_FIELD_BLOCK_SLOTS && fblock->contains(field)) {
- // Check if fields in all slots are deleted
- for (auto &m_field_slot : fblock->m_field_slots) {
- if (m_field_slot.m_readiness != MIME_FIELD_SLOT_READINESS_DELETED) {
- can_destroy_block = false;
- break;
- }
- }
- // Destroy a block and maintain the chain
- if (can_destroy_block) {
- prev_block->m_next = fblock->m_next;
- _mime_field_block_destroy(heap, fblock);
- if (prev_block->m_next == nullptr) {
- mh->m_fblock_list_tail = prev_block;
- }
- }
- break;
- }
- }
- prev_block = fblock;
- }
}
MIME_HDR_SANITY_CHECK(mh);
@@ -1615,7 +1688,7 @@ mime_hdr_prepare_for_value_set(HdrHeap *heap, MIMEHdrImpl *mh, std::string_view
if (field == nullptr) // no fields of this name
{
wks_idx = hdrtoken_tokenize(name.data(), static_cast<int>(name.length()));
- field = mime_field_create(heap, mh);
+ field = mime_field_create_for_name(heap, mh, name);
mime_field_name_set(heap, mh, field, wks_idx, name, true);
mime_hdr_field_attach(mh, field, 0, nullptr);
@@ -1623,7 +1696,7 @@ mime_hdr_prepare_for_value_set(HdrHeap *heap, MIMEHdrImpl *mh, std::string_view
{
wks_idx = field->m_wks_idx;
mime_hdr_field_delete(heap, mh, field, true);
- field = mime_field_create(heap, mh);
+ field = mime_field_create_for_name(heap, mh, name);
mime_field_name_set(heap, mh, field, wks_idx, name, true);
mime_hdr_field_attach(mh, field, 0, nullptr);
}
@@ -1631,10 +1704,18 @@ mime_hdr_prepare_for_value_set(HdrHeap *heap, MIMEHdrImpl *mh, std::string_view
}
void
-mime_field_destroy(MIMEHdrImpl * /* mh ATS_UNUSED */, MIMEField *field)
+mime_field_destroy(MIMEHdrImpl *mh, MIMEField *field)
{
ink_assert(field->m_readiness == MIME_FIELD_SLOT_READINESS_DETACHED);
field->m_readiness = MIME_FIELD_SLOT_READINESS_DELETED;
+
+ if (mh->m_free_slot == MIME_FIELD_FREE_SLOT_UNINITIALIZED) {
+ mime_hdr_rebuild_field_free_list(mh);
+ } else {
+ field->m_free_next = mh->m_free_slot;
+ mh->m_free_slot = mime_hdr_field_slotnum(mh, field);
+ ink_release_assert(mh->m_free_slot >= 0);
+ }
}
std::string_view
@@ -2538,7 +2619,7 @@ mime_parser_parse(MIMEParser *parser, HdrHeap *heap, MIMEHdrImpl *mh, const char
// build and insert the new field object //
///////////////////////////////////////////
- MIMEField *field = mime_field_create(heap, mh);
+ MIMEField *field = mime_field_create_for_name(heap, mh, field_name);
mime_field_name_value_set(heap, mh, field, field_name_wks_idx, field_name, field_value, raw_print_field, parsed.size(), false);
mime_hdr_field_attach(mh, field, 1, nullptr);
}
@@ -3514,6 +3595,8 @@ MIMEFieldBlockImpl::marshal(MarshalXlate *ptr_xlate, int num_ptr, MarshalXlate *
if (field->m_next_dup) {
HDR_MARSHAL_PTR_1(field->m_next_dup, MIMEField, ptr_xlate);
}
+ } else {
+ field->m_next_dup = nullptr;
}
}
} else {
@@ -3526,6 +3609,8 @@ MIMEFieldBlockImpl::marshal(MarshalXlate *ptr_xlate, int num_ptr, MarshalXlate *
if (field->m_next_dup) {
HDR_MARSHAL_PTR(field->m_next_dup, MIMEField, ptr_xlate, num_ptr);
}
+ } else {
+ field->m_next_dup = nullptr;
}
}
}
@@ -3612,6 +3697,7 @@ int
MIMEHdrImpl::marshal(MarshalXlate *ptr_xlate, int num_ptr, MarshalXlate *str_xlate, int num_str)
{
// printf("MIMEHdrImpl:marshal num_ptr = %d num_str = %d\n", num_ptr, num_str);
+ m_free_slot = MIME_FIELD_FREE_SLOT_UNINITIALIZED;
HDR_MARSHAL_PTR(m_fblock_list_tail, MIMEFieldBlockImpl, ptr_xlate, num_ptr);
return m_first_fblock.marshal(ptr_xlate, num_ptr, str_xlate, num_str);
}
@@ -3621,6 +3707,7 @@ MIMEHdrImpl::unmarshal(intptr_t offset)
{
HDR_UNMARSHAL_PTR(m_fblock_list_tail, MIMEFieldBlockImpl, offset);
m_first_fblock.unmarshal(offset);
+ m_free_slot = MIME_FIELD_FREE_SLOT_UNINITIALIZED;
}
void
diff --git a/src/proxy/hdrs/unit_tests/test_Hdrs.cc b/src/proxy/hdrs/unit_tests/test_Hdrs.cc
index 193b7072fb..f0a432fcb0 100644
--- a/src/proxy/hdrs/unit_tests/test_Hdrs.cc
+++ b/src/proxy/hdrs/unit_tests/test_Hdrs.cc
@@ -657,6 +657,196 @@ test_arena_aux(Arena &arena, int len)
} // end anonymous namespace
+TEST_CASE("MIME fields reuse deleted slots", "[proxy][hdrtest][mime]")
+{
+ mime_init();
+ http_init();
+
+ auto add_field = [](HTTPHdr &hdr, std::string_view name) {
+ MIMEField *field = hdr.field_create(name);
+
+ hdr.field_attach(field);
+ return field;
+ };
+
+ SECTION("A fully deleted field block is retained and reused")
+ {
+ HTTPHdr hdr;
+ hdr.create(HTTPType::RESPONSE);
+ ts::PostScript cleanup([&]() -> void { hdr.destroy(); });
+
+ std::array<MIMEField *, MIME_FIELD_BLOCK_SLOTS * 2> fields;
+ for (unsigned index = 0; index < fields.size(); ++index) {
+ fields[index] = add_field(hdr, "X-Field-" + std::to_string(index));
+ }
+
+ MIMEFieldBlockImpl *second_block = hdr.m_mime->m_first_fblock.m_next;
+ REQUIRE(second_block != nullptr);
+ REQUIRE(second_block == hdr.m_mime->m_fblock_list_tail);
+
+ for (unsigned index = MIME_FIELD_BLOCK_SLOTS; index < fields.size(); ++index) {
+ hdr.field_delete(fields[index], false);
+ }
+
+ CHECK(hdr.m_mime->m_first_fblock.m_next == second_block);
+ CHECK(hdr.m_mime->m_fblock_list_tail == second_block);
+
+ MIMEField *reused = hdr.field_create("X-Reused");
+ CHECK(reused == fields.back());
+ CHECK(hdr.m_mime->m_fblock_list_tail == second_block);
+ }
+
+ SECTION("Duplicate fields skip earlier free slots")
+ {
+ HTTPHdr hdr;
+ hdr.create(HTTPType::RESPONSE);
+ ts::PostScript cleanup([&]() -> void { hdr.destroy(); });
+
+ MIMEField *first = add_field(hdr, "X-Duplicate");
+ MIMEField *filler = add_field(hdr, "X-Filler");
+ MIMEField *last = add_field(hdr, "X-Duplicate");
+ for (unsigned index = 3; index < MIME_FIELD_BLOCK_SLOTS; ++index) {
+ add_field(hdr, "X-Filler-" + std::to_string(index));
+ }
+
+ REQUIRE(first->m_next_dup == last);
+ hdr.field_delete(first, false);
+ REQUIRE(hdr.field_find("X-Duplicate") == last);
+
+ MIMEField *reused = add_field(hdr, "X-Duplicate");
+ CHECK(reused != first);
+ CHECK(hdr.field_find("X-Duplicate") == last);
+ CHECK(last->m_next_dup == reused);
+ CHECK(reused->m_next_dup == nullptr);
+ CHECK(mime_hdr_field_slotnum(hdr.m_mime, last) < mime_hdr_field_slotnum(hdr.m_mime, reused));
+ CHECK(filler->is_live());
+ }
+
+ SECTION("A reused earlier slot preserves same-name field order")
+ {
+ HTTPHdr hdr;
+ hdr.create(HTTPType::RESPONSE);
+ ts::PostScript cleanup([&]() -> void { hdr.destroy(); });
+
+ MIMEField *victim = add_field(hdr, "X-Victim");
+ for (unsigned index = 1; index < MIME_FIELD_BLOCK_SLOTS - 1; ++index) {
+ add_field(hdr, "X-Filler-" + std::to_string(index));
+ }
+ MIMEField *first = add_field(hdr, "Set-Cookie");
+ REQUIRE(hdr.m_mime->m_first_fblock.m_freetop == MIME_FIELD_BLOCK_SLOTS);
+
+ hdr.field_delete(victim, false);
+ MIMEField *second = add_field(hdr, "Set-Cookie");
+
+ CHECK(hdr.field_find("Set-Cookie") == first);
+ CHECK(mime_hdr_field_slotnum(hdr.m_mime, first) < mime_hdr_field_slotnum(hdr.m_mime, second));
+ }
+
+ SECTION("Unused tail slots preserve field insertion order")
+ {
+ HTTPHdr hdr;
+ hdr.create(HTTPType::RESPONSE);
+ ts::PostScript cleanup([&]() -> void { hdr.destroy(); });
+
+ MIMEField *deleted = add_field(hdr, "X-Deleted");
+ MIMEField *kept = add_field(hdr, "X-Kept");
+
+ hdr.field_delete(deleted, false);
+ MIMEField *appended = add_field(hdr, "X-Appended");
+
+ CHECK(appended != deleted);
+ CHECK(mime_hdr_field_slotnum(hdr.m_mime, kept) < mime_hdr_field_slotnum(hdr.m_mime, appended));
+ }
+
+ SECTION("Repeated field churn stays at the high-water block count")
+ {
+ HTTPHdr hdr;
+ hdr.create(HTTPType::RESPONSE);
+ ts::PostScript cleanup([&]() -> void { hdr.destroy(); });
+
+ MIMEField *field = nullptr;
+ for (unsigned index = 0; index < MIME_FIELD_BLOCK_SLOTS * 2; ++index) {
+ field = add_field(hdr, "X-Field-" + std::to_string(index));
+ }
+
+ MIMEField *slot = field;
+ MIMEFieldBlockImpl *second_block = hdr.m_mime->m_first_fblock.m_next;
+ bool reused_slot = true;
+ constexpr unsigned churn_cycles = 1024;
+
+ for (unsigned index = 0; index < churn_cycles; ++index) {
+ hdr.field_delete(field, false);
+ field = add_field(hdr, "X-Churn");
+ reused_slot &= field == slot;
+ }
+
+ CHECK(reused_slot);
+ CHECK(hdr.m_mime->m_first_fblock.m_next == second_block);
+ CHECK(hdr.m_mime->m_fblock_list_tail == second_block);
+ CHECK(second_block->m_freetop == MIME_FIELD_BLOCK_SLOTS);
+ }
+
+ SECTION("Copied headers rebuild the free list")
+ {
+ HTTPHdr source;
+ HTTPHdr copy;
+ source.create(HTTPType::RESPONSE);
+ copy.create(HTTPType::RESPONSE);
+ ts::PostScript cleanup([&]() -> void {
+ source.destroy();
+ copy.destroy();
+ });
+
+ std::array<MIMEField *, MIME_FIELD_BLOCK_SLOTS * 2> fields;
+ for (unsigned index = 0; index < fields.size(); ++index) {
+ fields[index] = add_field(source, "X-Field-" + std::to_string(index));
+ }
+ source.field_delete(fields[4], false);
+ source.field_delete(fields[MIME_FIELD_BLOCK_SLOTS + 4], false);
+
+ copy.copy(&source);
+ MIMEFieldBlockImpl *tail = copy.m_mime->m_fblock_list_tail;
+ MIMEField *expected_slot = mime_hdr_field_get_slotnum(copy.m_mime, MIME_FIELD_BLOCK_SLOTS + 4);
+
+ CHECK(copy.field_create("X-Reused") == expected_slot);
+ CHECK(copy.m_mime->m_fblock_list_tail == tail);
+ }
+
+ SECTION("Headers copied from an unmarshaled heap rebuild the free list")
+ {
+ HTTPHdr source;
+ source.create(HTTPType::RESPONSE);
+ ts::PostScript cleanup([&]() -> void { source.destroy(); });
+
+ std::array<MIMEField *, MIME_FIELD_BLOCK_SLOTS * 2> fields;
+ for (unsigned index = 0; index < fields.size(); ++index) {
+ fields[index] = add_field(source, "X-Field-" + std::to_string(index));
+ }
+ source.field_delete(fields[MIME_FIELD_BLOCK_SLOTS + 4], false);
+
+ std::vector<char> marshal_buffer(source.m_heap->marshal_length());
+ int marshal_length = source.m_heap->marshal(marshal_buffer.data(), marshal_buffer.size());
+ REQUIRE(marshal_length > 0);
+
+ TestRefCountObj ref;
+ ref.refcount_inc();
+
+ HTTPHdr unmarshaled;
+ REQUIRE(unmarshaled.unmarshal(marshal_buffer.data(), marshal_length, &ref) > 0);
+
+ HTTPHdr writable;
+ writable.create(HTTPType::RESPONSE);
+ ts::PostScript writable_cleanup([&]() -> void { writable.destroy(); });
+ writable.copy(&unmarshaled);
+
+ MIMEFieldBlockImpl *tail = writable.m_mime->m_fblock_list_tail;
+ MIMEField *expected_slot = mime_hdr_field_get_slotnum(writable.m_mime, MIME_FIELD_BLOCK_SLOTS + 4);
+
+ CHECK(writable.field_create("X-Reused") == expected_slot);
+ CHECK(writable.m_mime->m_fblock_list_tail == tail);
+ }
+}
+
TEST_CASE("HdrTest", "[proxy][hdrtest]")
{
hdrtoken_init();
diff --git a/src/proxy/http2/HPACK.cc b/src/proxy/http2/HPACK.cc
index f2ab1b9c36..e9b7c06376 100644
--- a/src/proxy/http2/HPACK.cc
+++ b/src/proxy/http2/HPACK.cc
@@ -723,9 +723,9 @@ hpack_decode_header_block(HpackIndexingTable &indexing_table, HTTPHdr *hdr, cons
while (cursor < in_buf_end) {
int64_t read_bytes = 0;
- // decode a header field encoded by HPACK
- MIMEField *field = mime_field_create(heap, hh->m_fields_impl);
- MIMEFieldWrapper header(field, heap, hh->m_fields_impl);
+ // Decode a header field encoded by HPACK. The wrapper allocates the field
+ // after its name is known so the selected slot preserves duplicate order.
+ MIMEFieldWrapper header(heap, hh->m_fields_impl);
HpackField ftype = hpack_parse_field_type(*cursor);
switch (ftype) {
@@ -763,8 +763,8 @@ hpack_decode_header_block(HpackIndexingTable &indexing_table, HTTPHdr *hdr, cons
continue;
}
- auto name{field->name_get()};
- auto value{field->value_get()};
+ auto name{header.name_get()};
+ auto value{header.value_get()};
// [RFC 7540] 6.5.2. SETTINGS_MAX_HEADER_LIST_SIZE:
// The value is based on the uncompressed size of header fields, including the length of the name and value in octets plus an
@@ -775,8 +775,7 @@ hpack_decode_header_block(HpackIndexingTable &indexing_table, HTTPHdr *hdr, cons
return HPACK_ERROR_SIZE_EXCEEDED_ERROR;
}
- // Store to HdrHeap
- mime_hdr_field_attach(hh->m_fields_impl, field, 1, nullptr);
+ mime_hdr_field_attach(hh->m_fields_impl, header.field_get(), 1, nullptr);
}
// Parsing all headers is done
if (has_http2_violation) {
diff --git a/src/proxy/http2/unit_tests/test_HpackIndexingTable.cc b/src/proxy/http2/unit_tests/test_HpackIndexingTable.cc
index 4963b0b393..291549135b 100644
--- a/src/proxy/http2/unit_tests/test_HpackIndexingTable.cc
+++ b/src/proxy/http2/unit_tests/test_HpackIndexingTable.cc
@@ -105,8 +105,7 @@ TEST_CASE("HPACK low level APIs", "[hpack]")
for (const auto &i : indexed_test_case) {
std::unique_ptr<HTTPHdr, void (*)(HTTPHdr *)> headers(new HTTPHdr, destroy_http_hdr);
headers->create(HTTPType::REQUEST);
- MIMEField *field = mime_field_create(headers->m_heap, headers->m_http->m_fields_impl);
- MIMEFieldWrapper header(field, headers->m_heap, headers->m_http->m_fields_impl);
+ MIMEFieldWrapper header(headers->m_heap, headers->m_http->m_fields_impl);
int len = decode_indexed_header_field(header, i.encoded_field, i.encoded_field + i.encoded_field_len, indexing_table);
REQUIRE(len == i.encoded_field_len);
@@ -255,8 +254,7 @@ TEST_CASE("HPACK low level APIs", "[hpack]")
for (const auto &i : literal_test_case) {
std::unique_ptr<HTTPHdr, void (*)(HTTPHdr *)> headers(new HTTPHdr, destroy_http_hdr);
headers->create(HTTPType::REQUEST);
- MIMEField *field = mime_field_create(headers->m_heap, headers->m_http->m_fields_impl);
- MIMEFieldWrapper header(field, headers->m_heap, headers->m_http->m_fields_impl);
+ MIMEFieldWrapper header(headers->m_heap, headers->m_http->m_fields_impl);
int len = decode_literal_header_field(header, i.encoded_field, i.encoded_field + i.encoded_field_len, indexing_table,
MAX_FIELD_SIZE);