textproto: Add Header.Count and HeaderFields.Set

[foxcpp]

No description provided.

[codecov-io]

Codecov Report

Merging #75 into master will decrease coverage by 0.14%.
The diff coverage is 62.5%.

@@            Coverage Diff             @@
##           master      #75      +/-   ##
==========================================
- Coverage    80.1%   79.96%   -0.15%     
==========================================
  Files          15       15              
  Lines         995     1003       +8     
==========================================
+ Hits          797      802       +5     
- Misses        122      125       +3     
  Partials       76       76

Impacted Files	Coverage Δ
textproto/header.go	83.44% <62.5%> (-0.6%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9c4415e...36679d8. Read the comment docs.

[emersion]

Nack. This will break DKIM signatures and other things. Header fields should always be added at the top.

To set a field, one should remove the old field and add the new one at the top.

[foxcpp]

Indeed it will. But... We already have Del, we already have Header.Set. These are signature-breaking operations too. I don't think there is a problem implementing these if it is clearly documented that it will break signatures.

You can see my use case in the referenced issue. What do I do? Also, I think milter client will also need to support such operations (and also index-based access).
In fact, I wanted to add a "safeguard" to maddy so such operations will be no-op if there is a DKIM signature on the message aka explicit drop_header DKIM-Signature is required which indicates the admin knows what they are doing.

[emersion]

I still don't know how I feel about HeaderFields.Set. In the future you'll very likely want even more operations, like inserting a header field at a specific index.

One important property ensured by Header is that changes are always applied on top. If an intermediary sets a header field, the field won't be changed in-place, instead it'll get removed and re-added on top. This makes it easier to understand who set header fields and makes it more obvious when a field has been modified or deleted.

I wonder if we should instead add a helper to create a Header from a map[string][]string and the other way around. That way library users could perform arbitrary operations on the header if they need to and don't care about breaking DKIM.

[foxcpp]

Okay, I see. I also understand that my use case is rather specific, I need ability to preserve header structure as much as possible while being able to modify separate fields freely when needed. I guess, MutableHeader can exist in go-mesaage that provides all such operations (and that includes index-based operations, as they are needed for milter). In that case, Header can be a restricted wrapper for MutableHeader. Converting back and fourth between map of slices will destroy the structure and is simply inefficient.

Or, since my case is rather specific. I might stop trying to bend your library for my needs if you want to keep it the way it is.

[foxcpp]

So what are we doing here?

FYI Milter protocol defines two (three) header operations:

1. Prepend header (SMFIR_ADDHEADER)
2. Change header (SMFIR_CHGHEADER)
   1. Change value of specific field (this is why I am adding HeaderFields.Set here)
   2. Remove specific field by setting its value to empty string (already available as HeaderFields.Del)

[foxcpp]

Okay, milter protocol version 2 includes SMFI_INSHEADER, "insert header field at a specified index".
That gets complicated.

[emersion]

Yeah. I'd suggest turning the Header into a slice, performing the milter changes, then turning the list back to a Header.

[foxcpp]

Replaced by #94.