bneradt commented on code in PR #13455:
URL: https://github.com/apache/trafficserver/pull/13455#discussion_r3754772601
##########
src/proxy/hdrs/unit_tests/test_Hdrs.cc:
##########
@@ -657,6 +657,175 @@ 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("Reused duplicate fields remain ordered by slot")
+ {
+ 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") == reused);
+ CHECK(reused->m_next_dup == last);
+ CHECK(last->m_next_dup == nullptr);
+ CHECK(filler->is_live());
+ }
+
+ 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));
+ }
Review Comment:
Thanks, this exposed that arbitrary slot reuse could reorder same-name
fields. I added this regression test and made allocation name-aware so
duplicate fields skip earlier free slots.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
