Copilot commented on code in PR #13376:
URL: https://github.com/apache/trafficserver/pull/13376#discussion_r4078420738
##########
src/proxy/hdrs/unit_tests/test_mime.cc:
##########
@@ -90,6 +96,235 @@ TEST_CASE("Mime", "[proxy][mime]")
hdr.destroy();
}
+TEST_CASE("MimeParserReuseAcrossHeaders", "[proxy][mimeparser]")
+{
+ // A parser reused on a different header without an intervening clear must
+ // still detect duplicates of fields already present in that header. The tail
+ // append derives its candidate from the header under parse, so a change of
+ // header cannot carry state over. If it instead trusted parser-held state, a
+ // wire duplicate of a pre-existing custom field would attach as an
+ // independent head rather than joining the dup chain.
+ MIMEParser parser;
+ mime_parser_init(&parser);
+
+ // First header: parse a non-WKS field so the parser seeds its dup state.
+ MIMEHdr hdrA;
+ hdrA.create(nullptr);
+ {
+ std::string_view text = "X-Foo: 1\r\n\r\n"sv;
+ const char *start = text.data();
+ REQUIRE(hdrA.parse(&parser, &start, text.data() + text.size(), true,
false, false) == ParseResult::DONE);
+ }
+
+ // Second header (different mh) already carries a live non-WKS field; reuse
the
+ // same parser WITHOUT clearing it and parse a duplicate of that field.
+ MIMEHdr hdrB;
+ hdrB.create(nullptr);
+ MIMEField *pre = hdrB.field_create("X-Baz"sv);
+ pre->value_set(hdrB.m_heap, hdrB.m_mime, "a"sv);
+ hdrB.field_attach(pre);
+ {
+ std::string_view text = "X-Baz: b\r\n\r\n"sv;
+ const char *start = text.data();
+ REQUIRE(hdrB.parse(&parser, &start, text.data() + text.size(), true,
false, false) == ParseResult::DONE);
+ }
+
+ // The wire field must have joined the pre-existing field's dup chain:
exactly
+ // two X-Baz values reachable from the head.
+ MIMEField *head = hdrB.field_find("X-Baz"sv);
+ REQUIRE(head != nullptr);
+ int count = 0;
+ for (MIMEField *f = head; f != nullptr; f = f->m_next_dup) {
+ ++count;
+ }
+ CHECK(count == 2);
+
+ mime_parser_clear(&parser);
+ hdrA.destroy();
+ hdrB.destroy();
+}
+
+TEST_CASE("MimeParserTailAppendEquivalence", "[proxy][mimeparser]")
+{
+ // The O(1) adjacent-duplicate tail append must produce the same field/dup
+ // structure as attach's full duplicate search. Build the same field sequence
+ // two ways -- via the parser (which takes the tail-append path) and via
+ // explicit create+attach (the reference full-attach path) -- and compare the
+ // dup chain of every name.
+ struct Field {
+ const char *name;
+ const char *value;
+ };
+ auto scenario = GENERATE(from_range(std::vector<std::vector<Field>>{
+ {{"X-A", "1"}, {"X-A", "2"}, {"X-A", "3"}}, // consecutive custom dups
+ {{"X-A", "1"}, {"X-B", "2"}, {"X-A", "3"}}, // interleaved
+ {{"X-A", "1"}, {"X-A", "2"}, {"X-B", "3"}, {"X-B", "4"}}, // two adjacent
runs
+ {{"Set-Cookie", "a"}, {"Set-Cookie", "b"}, {"Set-Cookie", "c"},
{"Set-Cookie", "d"}}, // well-known dups
+ {{"X-A", "1"}, {"X-B", "2"}, {"X-C", "3"}}, // no dups
+ }));
+
+ // Parser-built header (tail-append path).
+ std::string raw;
+ for (auto const &f : scenario) {
+ raw += f.name;
+ raw += ": ";
+ raw += f.value;
+ raw += "\r\n";
+ }
+ raw += "\r\n";
+ MIMEParser parser;
+ mime_parser_init(&parser);
+ MIMEHdr hdrA;
+ hdrA.create(nullptr);
+ {
+ const char *start = raw.data();
+ REQUIRE(hdrA.parse(&parser, &start, raw.data() + raw.size(), true, false,
false) == ParseResult::DONE);
+ }
+ mime_parser_clear(&parser);
+
+ // Reference header via explicit create+attach (attach's full path, no tail
append).
+ MIMEHdr hdrB;
+ hdrB.create(nullptr);
+ for (auto const &f : scenario) {
+ MIMEField *fld = hdrB.field_create(std::string_view{f.name});
+ fld->value_set(hdrB.m_heap, hdrB.m_mime, std::string_view{f.value});
+ hdrB.field_attach(fld);
+ }
+
+ // Compare structure, not just values. The tail append sets the dup-head flag
+ // and the well-known index itself instead of letting attach do it, and it
+ // must still raise the presence bit for a well-known name.
+ struct Slot {
+ std::string value;
+ bool dup_head;
+ int16_t wks_idx;
+
+ bool
+ operator==(Slot const &o) const
+ {
+ return value == o.value && dup_head == o.dup_head && wks_idx ==
o.wks_idx;
+ }
+ };
+
+ auto collect = [](MIMEHdr &h, std::string_view n) {
+ std::vector<Slot> slots;
+ for (MIMEField *fld = h.field_find(n); fld != nullptr; fld =
fld->m_next_dup) {
+ auto v = fld->value_get();
+ slots.push_back(Slot{std::string{v}, fld->is_dup_head() != 0,
fld->m_wks_idx});
+ }
+ return slots;
+ };
+
+ std::set<std::string> names;
+ for (auto const &f : scenario) {
+ names.insert(f.name);
+ }
+ for (auto const &name : names) {
+ std::vector<Slot> va = collect(hdrA, name);
+ std::vector<Slot> vb = collect(hdrB, name);
+ CAPTURE(name, va.size(), vb.size());
+ REQUIRE(va.size() == vb.size());
+ for (size_t i = 0; i < va.size(); i++) {
+ CAPTURE(i, va[i].value, vb[i].value, va[i].dup_head, vb[i].dup_head,
va[i].wks_idx, vb[i].wks_idx);
+ CHECK(va[i] == vb[i]);
+ }
+ }
+
+ CHECK(hdrA.fields_count() == hdrB.fields_count());
+ CHECK(hdrA.m_mime->m_presence_bits == hdrB.m_mime->m_presence_bits);
+
+ hdrA.destroy();
+ hdrB.destroy();
+}
+
+TEST_CASE("HdrTokenFusedNameScanParity", "[proxy][hdrtoken]")
+{
+ // hdrtoken_field_name_scan and hdrtoken_tokenize_prehashed fuse the colon
+ // scan, the FNV hash, and field-name validation into one pass. Verify the
+ // fused path agrees with references: the colon position, per-byte validity,
+ // and -- via the prehashed lookup fed the fused hash -- the well-known index
+ // the standalone tokenizer returns (which is a proxy for hash parity).
+ struct Case {
+ const char *name;
+ const char *tail;
+ };
+ static const std::vector<Case> cases = {
+ {"Content-Length", ": 5" },
+ {"content-length", ":5" },
+ {"CONTENT-LENGTH", ":5" },
+ {"Host", ": x" },
+ {"hOsT", ":x" },
+ {"Set-Cookie", ": a=b" },
+ {"Cache-Control", ":no" },
+ {"Transfer-Encoding", ":chunk" },
+ {"@Ats-Internal", ":z" },
+ {"X-Custom-Header", ": v" },
+ {"sec-ch-ua", ": \"x\""},
+ {"sec-fetch-mode", ":cors" },
+ {"priority", ":u=1" },
+ {"X-My-Header", ":v" },
+ {"a", ":b" },
+ };
+
+ for (auto const &c : cases) {
+ std::string const buf = std::string(c.name) + c.tail;
+ int const name_len = static_cast<int>(strlen(c.name));
+ uint32_t hash = 0;
+ bool valid = false;
+ int const colon = hdrtoken_field_name_scan(buf.data(),
static_cast<int>(buf.size()), &hash, &valid);
+ CAPTURE(c.name);
+ CHECK(colon == name_len);
+
+ bool ref_valid = true;
+ for (int i = 0; i < name_len; ++i) {
+ if (!ParseRules::is_http_field_name(c.name[i])) {
+ ref_valid = false;
+ break;
+ }
+ }
+ CHECK(valid == ref_valid);
+ CHECK(hdrtoken_tokenize_prehashed(c.name, name_len, hash) ==
hdrtoken_tokenize(c.name, name_len));
+ }
+
+ // The cases above only pin hash parity where the name is in the token table;
+ // on a miss both tokenizers return -1 whatever hash they were handed. Sweep
+ // the whole table, in three case forms, so every entry is a live comparison.
+ for (int idx = 0; idx < hdrtoken_num_wks; ++idx) {
+ std::string const wks{hdrtoken_strs[idx],
static_cast<size_t>(hdrtoken_str_lengths[idx])};
+ std::string upper{wks}, lower{wks};
+
+ for (auto &ch : upper) {
+ ch = static_cast<char>(toupper(static_cast<unsigned char>(ch)));
+ }
+ for (auto &ch : lower) {
+ ch = static_cast<char>(tolower(static_cast<unsigned char>(ch)));
Review Comment:
The new parity test calls `toupper` and `tolower`, but this file does not
include `<cctype>`; `<cstring>` and the project headers do not guarantee those
declarations. Add the standard header so the test does not depend on transitive
includes and fail on a conforming toolchain.
--
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]
