Cripts: reading a header returns only its first field line

[masaori335]

1. The issue

cripts::Header::operator[] (src/cripts/Headers.cc:227-246) returns the value of the first field line only. RFC 9110 §5.3 makes repeated field lines semantically one comma-joined value, and header_rewrite implements exactly that (plugins/header_rewrite/conditions.cc:286-298, walks TSMimeHdrFieldNextDup, joins on a bare ,). So the two disagree on any duplicated header — Cache-Control, Via, Accept-Encoding, Warning, and anything a client chose to split.

The mechanism: TSMimeHdrFieldValueStringGet(..., idx = -1) reaches TSMimeFieldValueGet, whose idx < 0 branch is handle->field_ptr->value_get() (src/api/InkAPI.cc:1655-1666) — that one field line, not the dup chain.

This is not a subtly wrong value, it's an inverted branch. Client sends the field twice:

X-Check: yes
X-Check: no

against if (req["X-Check"] == "yes") { req.Erase("X-Check"); }:

Read behaviour	Sees	Branch taken	Result
joined (header_rewrite)	yes,no	no	both lines reach the origin
first-field-only (Cripts)	yes	yes	every line destroyed, header absent

Why this looks like an oversight rather than a decision

The same file walks duplicates correctly in all three write paths and in neither read path:

src/cripts/Headers.cc	Walks NextDup?	Behaviour
:113-164 operator=(string_view)	yes	replaces the first line, destroys the rest
:166-199 operator=(integer)	yes	same
:118-128 empty-assign / Erase	yes	destroys every line
:227-246 operator[]	no	reads line one, ignores the rest
:248-264 AsDate	no	reads line one (defensible — comma-joining dates is nonsense)

Stronger still: operator+= (:202-225) creates a second field line rather than extending the value. Cripts can therefore write a header it cannot read back — h["X"] = "a"; h["X"] += "b"; then reads as a. An implementation that deliberately treated headers as single-valued would not do that.

Adjacent defects in the same code, found while investigating

Listing these because any fix touches the same class, and two are memory-safety issues:

Where	Problem
include/cripts/Headers.hpp:151-157	Header::String has a destructor that releases _field_loc, but no rule-of-three. The implicit copy ctor/copy-assign duplicate the TSMLoc, so h["A"] = h["B"] (which binds the implicit copy-assign, not the string_view overload) double-releases → sdk_free_field_handle → THREAD_FREE twice on a proxy-allocator handle (src/api/InkAPI.cc:863-870, :197-202). operator[] itself is safe only because NRVO fires.
src/cripts/Headers.cc:209-211	operator+= releases _field_loc without nulling it; TSMimeHdrFieldCreateNamed leaves *locp untouched on its !isWriteable early return (src/api/InkAPI.cc:1904-1906), so on a non-writeable heap ~String releases the stale handle again.
src/cripts/Headers.cc:285-328	begin()/iterate() use TSMimeHdrFieldNext, which steps the flat field list including duplicates (src/api/InkAPI.cc:2038-2055). The documented for (auto h : req) CDebug("{}: {}", h, req[h]) idiom therefore prints a duplicated header's first value once per line.
src/cripts/Headers.cc:113-199	After any assignment the proxy's _field_loc ends up nullptr and its cached value is stale, so read-after-write on the same proxy is wrong.

2. Proposed fix

Make operator[] return the joined value, and expose the individual field lines. Joining alone is not enough: Set-Cookie is explicitly exempted from list semantics by RFC 9110 §5.3 (its values contain commas and cannot be split back), so scripts need per-line access as a first-class API, not a workaround.

req["Accept-Encoding"];           // "gzip,br"   (was "gzip")
req["Accept-Encoding"].Count();   // 2           (0 when absent)
req["Accept-Encoding"].Values();  // {"gzip", "br"}

Where the joined buffer lives — the one real design question

Header::String is a StringViewMixin holding a string_view and no owned buffer, and operator[] returns it by value. A joined value needs backing store somewhere:

Option	Cost
A. Own a std::string in the proxy	StringViewMixin::operator string_view() is implicit (include/cripts/Lulu.hpp:162), so cripts::string_view v = req["Cache-Control"]; would dangle at the end of the full expression — but only when the header is duplicated. Silently fine in test, wrong in production. Trades a wrong-value bug for a use-after-free on the same trigger.
B. Arena on the owning Header ✅	Cleared in Header::Reset(), which Context::reset() (src/cripts/Context.cc:33) already calls between hooks — i.e. exactly the lifetime a header view has today. No new dangling class. Must be node-stable (std::list), since vector<std::string> reallocation moves SSO strings and recreates A's bug. Allocates only when TSMimeHdrFieldNextDup != nullptr, so the single-line path is unchanged.
C. Leave operator[], add Joined()	No behaviour change, but the default stays wrong and every caller must know to avoid [].

Proposing B.

Prerequisite

Delete Header::String's copy/move and build it as a prvalue from a private ctor (guaranteed elision), plus a real operator=(const String&) that assigns the value. Without this, adding a buffer turns the latent double-free above into an easy one. This also fixes h["A"] = h["B"], which is currently a memory bug.

Scope questions for discussion

1. The iterator. Once reads are joined, should iteration deduplicate names, or keep yielding one entry per field line? Keeping it is defensible ("duplicates stay visible"), but it makes the documented print-all-headers idiom emit the joined value N times.
2. AsDate — leave on the first field line?
3. Blast radius. Any Cript comparing a header that can legitimately repeat starts seeing a,b where it saw a. Correct, but not what those scripts were written against, so this wants a release note in doc/release-notes/upgrading.en.rst rather than a quiet patch. Is v11 the right place, or does this need a deprecation path?

I have a working implementation of the above (Option B + Count()/Values() + the rule-of-three fix), with an AuTest covering the joined read, per-line access, the inverted-branch case, and the write-path round trip. Happy to open a PR once there's agreement on 1–3.