From 4be139767afacb1f50f876c34d462d1872f0fac3 Mon Sep 17 00:00:00 2001 From: Mark Mennell Date: Tue, 25 Aug 2026 16:13:40 +0800 Subject: [PATCH 1/2] Allow to/add-to overlap, treat recipients as a set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follows the revised ruling in markmnl/fmsg#29: an address MAY appear in both _to_ and _add to_, re-serving an original recipient who lost the message. What made the overlap ambiguous was not the overlap itself but the per-recipient iteration, which walked occurrences while Terms defines _recipients_ as a set. The spec now iterates distinct addresses; this brings fmsgd in line on both sides of the wire. - readAddToRecipients no longer rejects an add-to address present in _to_ with code 1. Duplicates *within* _add to_ are still rejected. - localRecipients returns one entry per distinct address, in the order it first appears scanning _to_ then _add to_. This is what sizes the per-recipient code stream on the receive side. - The sender's domainRecips is deduplicated identically. Without this it would read one byte too many for an overlapping recipient and desync the stream — the exact failure the prohibition existed to prevent. - An accepted overlapping address is now recorded against both _to_ and the batch, instead of being classified exclusively as add-to. The msg_to insert's ON CONFLICT DO NOTHING preserves its original response code, as SPEC §3's closing NOTE requires. Note this makes fmsgd accept such a message rather than reject it; for a recipient that still holds the message fmsgd answers 103 (user duplicate), which is conformant. Actually re-serving a recipient whose copy is gone is a host policy decision, unchanged here. --- cmd/fmsgd/host.go | 50 ++++++++++++++++++++++++++++++--------------- cmd/fmsgd/sender.go | 17 ++++++++++++--- 2 files changed, 48 insertions(+), 19 deletions(-) diff --git a/cmd/fmsgd/host.go b/cmd/fmsgd/host.go index 63825fc..f3fd7b0 100644 --- a/cmd/fmsgd/host.go +++ b/cmd/fmsgd/host.go @@ -799,12 +799,9 @@ func readAddToRecipients(c net.Conn, r *bufio.Reader, h *FMsgHeader, seen map[st return fmt.Errorf("duplicate recipient address in add to: %s", addr.ToString()) } addToSeen[key] = true - if seen[key] { - if err := sendCode(c, RejectCodeInvalid); err != nil { - return err - } - return fmt.Errorf("add-to address already in to: %s", addr.ToString()) - } + // An address in both _to_ and _add to_ is permitted (SPEC §1.4.6.3 + // NOTE II): it re-serves an original recipient. Recipients are a set, + // so localRecipients collapses it to a single per-recipient code. h.AddTo = append(h.AddTo, *addr) addToCount-- } @@ -1244,17 +1241,30 @@ func uniqueFilepath(dir string, timestamp uint32, ext string) string { } } +// localRecipients returns this host's recipients as a SET (SPEC Terms): one +// entry per distinct address, in the order the address first appears scanning +// _to_ then _add to_. An address in both lists is one recipient and so gets +// exactly one per-recipient response code, keeping the response stream in +// step with what the sender expects. func localRecipients(h *FMsgHeader) []FMsgAddress { addrs := make([]FMsgAddress, 0, len(h.To)+len(h.AddTo)) - for _, addr := range h.To { - if strings.EqualFold(addr.Domain, Domain) { - addrs = append(addrs, addr) + seen := make(map[string]bool, len(h.To)+len(h.AddTo)) + appendUnique := func(addr FMsgAddress) { + if !strings.EqualFold(addr.Domain, Domain) { + return + } + key := strings.ToLower(addr.ToString()) + if seen[key] { + return } + seen[key] = true + addrs = append(addrs, addr) + } + for _, addr := range h.To { + appendUnique(addr) } for _, addr := range h.AddTo { - if strings.EqualFold(addr.Domain, Domain) { - addrs = append(addrs, addr) - } + appendUnique(addr) } return addrs } @@ -1528,7 +1538,13 @@ func downloadMessage(c net.Conn, r io.Reader, h *FMsgHeader, skipData bool) erro defer src.Close() // validate each recipient and copy message for accepted ones - // Build a set of add-to addresses for later classification + // Build sets of to / add-to addresses for later classification. The two + // may overlap (SPEC §1.4.6.3 NOTE II), in which case the recipient belongs + // to both the original's _to_ list and this batch and is recorded in both. + toSet := make(map[string]bool) + for _, addr := range h.To { + toSet[strings.ToLower(addr.ToString())] = true + } addToSet := make(map[string]bool) for _, addr := range h.AddTo { addToSet[strings.ToLower(addr.ToString())] = true @@ -1561,11 +1577,13 @@ func downloadMessage(c net.Conn, r io.Reader, h *FMsgHeader, skipData bool) erro } codes[i] = RejectCodeAccept - if addToSet[strings.ToLower(addr.ToString())] { - acceptedAddTo = append(acceptedAddTo, addr) - } else { + key := strings.ToLower(addr.ToString()) + if toSet[key] { acceptedTo = append(acceptedTo, addr) } + if addToSet[key] { + acceptedAddTo = append(acceptedAddTo, addr) + } if primaryFilepath == "" { primaryFilepath = fp if err := persistAttachmentPayloads(h, filepath.Dir(primaryFilepath)); err != nil { diff --git a/cmd/fmsgd/sender.go b/cmd/fmsgd/sender.go index 83cb111..0e02485 100644 --- a/cmd/fmsgd/sender.go +++ b/cmd/fmsgd/sender.go @@ -577,8 +577,9 @@ func deliverUnit(db *sql.DB, target pendingTarget, h *FMsgHeader, table string, } }() - // Per-recipient response codes arrive in to-field order then add-to order - // (SPEC §10.2 step 6). Only this unit's locked recipients are recorded; the + // Per-recipient response codes arrive one per DISTINCT recipient, in the + // order the address first appears scanning _to_ then _add to_ (SPEC §10.2 + // step 6). Only this unit's locked recipients are recorded; the // recipients carried only to reconstruct that ordering stay unlocked. lockedSet := make(map[string]bool, len(locked)) for _, a := range locked { @@ -589,13 +590,23 @@ func deliverUnit(db *sql.DB, target pendingTarget, h *FMsgHeader, table string, isLocked bool } var domainRecips []domainRecip + // Recipients are a SET (SPEC Terms): an address in both _to_ and _add to_ + // is one recipient and the receiving host sends exactly one code for it, + // in its _to_ position. Counting the occurrence twice here would read one + // byte too many and desync the stream. + seenRecip := make(map[string]bool) appendDomain := func(addrs []FMsgAddress) { for _, addr := range addrs { if !strings.EqualFold(addr.Domain, target.Domain) { continue } s := addr.ToString() - domainRecips = append(domainRecips, domainRecip{addr: s, isLocked: lockedSet[strings.ToLower(s)]}) + key := strings.ToLower(s) + if seenRecip[key] { + continue + } + seenRecip[key] = true + domainRecips = append(domainRecips, domainRecip{addr: s, isLocked: lockedSet[key]}) } } appendDomain(h.To) From c83dd95e4382f99365dfd6b7f1d0b844c88fa7c9 Mon Sep 17 00:00:00 2001 From: Mark Mennell Date: Tue, 25 Aug 2026 17:14:17 +0800 Subject: [PATCH 2/2] One response code per recipient entry, not per distinct address MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follows the revised ruling in markmnl/fmsg#29. Recipients are no longer treated as a set: addresses need only be distinct within _to_ and within _add to_, and an address in both lists is a recipient of each, answered once for its _to_ entry and once for its _add to_ entry. That is what fmsgd already did, so localRecipients and the sender's domainRecips go back to plain wire order and the earlier deduplication here is dropped. What remains is accepting the overlap at all: - readAddToRecipients no longer rejects an add-to address present in _to_ with code 1. Duplicates within _add to_ are still rejected. - The accepted-recipient classification keys off wire position rather than set membership: entries before numLocalTo came from _to_, the rest from _add to_. Previously an overlapping address was classified exclusively as add-to, losing its _to_ record; now each entry is recorded against the list it came from, and the msg_to insert's ON CONFLICT DO NOTHING preserves its original response code as SPEC §3's closing NOTE requires. - With the overlap check gone, readAddToRecipients no longer needs the _to_ key set, so readToRecipients stops returning it. Tests updated. --- cmd/fmsgd/host.go | 87 +++++++++++++++++------------------------- cmd/fmsgd/host_test.go | 17 +++------ cmd/fmsgd/sender.go | 21 +++------- 3 files changed, 47 insertions(+), 78 deletions(-) diff --git a/cmd/fmsgd/host.go b/cmd/fmsgd/host.go index f3fd7b0..5d04b45 100644 --- a/cmd/fmsgd/host.go +++ b/cmd/fmsgd/host.go @@ -708,35 +708,35 @@ func readVersionOrChallenge(c net.Conn, r *bufio.Reader, h *FMsgHeader) (bool, e return false, nil } -func readToRecipients(c net.Conn, r *bufio.Reader, h *FMsgHeader) (map[string]bool, error) { +func readToRecipients(c net.Conn, r *bufio.Reader, h *FMsgHeader) error { num, err := r.ReadByte() if err != nil { - return nil, err + return err } if num == 0 { if err := sendCode(c, RejectCodeInvalid); err != nil { - return nil, err + return err } - return nil, fmt.Errorf("to count must be >= 1") + return fmt.Errorf("to count must be >= 1") } seen := make(map[string]bool) for num > 0 { addr, err := readAddress(r) if err != nil { - return nil, err + return err } key := strings.ToLower(addr.ToString()) if seen[key] { - return nil, fmt.Errorf("duplicate recipient address: %s", addr.ToString()) + return fmt.Errorf("duplicate recipient address: %s", addr.ToString()) } seen[key] = true h.To = append(h.To, *addr) num-- } - return seen, nil + return nil } -func readAddToRecipients(c net.Conn, r *bufio.Reader, h *FMsgHeader, seen map[string]bool) error { +func readAddToRecipients(c net.Conn, r *bufio.Reader, h *FMsgHeader) error { if h.Flags&FlagHasAddTo == 0 { return nil } @@ -800,8 +800,8 @@ func readAddToRecipients(c net.Conn, r *bufio.Reader, h *FMsgHeader, seen map[st } addToSeen[key] = true // An address in both _to_ and _add to_ is permitted (SPEC §1.4.6.3 - // NOTE II): it re-serves an original recipient. Recipients are a set, - // so localRecipients collapses it to a single per-recipient code. + // NOTE II): it re-serves an original recipient. It is a recipient of + // each list and gets one response code per entry. h.AddTo = append(h.AddTo, *addr) addToCount-- } @@ -1032,12 +1032,11 @@ func readHeader(c net.Conn) (*FMsgHeader, *bufio.Reader, error) { h.From = *from - seen, err := readToRecipients(c, r, h) - if err != nil { + if err := readToRecipients(c, r, h); err != nil { return h, r, err } - if err := readAddToRecipients(c, r, h, seen); err != nil { + if err := readAddToRecipients(c, r, h); err != nil { return h, r, err } @@ -1241,32 +1240,26 @@ func uniqueFilepath(dir string, timestamp uint32, ext string) string { } } -// localRecipients returns this host's recipients as a SET (SPEC Terms): one -// entry per distinct address, in the order the address first appears scanning -// _to_ then _add to_. An address in both lists is one recipient and so gets -// exactly one per-recipient response code, keeping the response stream in -// step with what the sender expects. -func localRecipients(h *FMsgHeader) []FMsgAddress { - addrs := make([]FMsgAddress, 0, len(h.To)+len(h.AddTo)) - seen := make(map[string]bool, len(h.To)+len(h.AddTo)) - appendUnique := func(addr FMsgAddress) { - if !strings.EqualFold(addr.Domain, Domain) { - return - } - key := strings.ToLower(addr.ToString()) - if seen[key] { - return - } - seen[key] = true - addrs = append(addrs, addr) - } +// localRecipients returns this host's recipients in wire order: every _to_ +// entry for this domain, then every _add to_ entry for this domain. Addresses +// are distinct within each list but MAY appear in both (SPEC Terms), in which +// case the address is a recipient of each and receives one response code per +// entry — so the count here is exactly the number of code bytes exchanged. +// numLocalTo reports how many of the returned entries came from _to_. +func localRecipients(h *FMsgHeader) (addrs []FMsgAddress, numLocalTo int) { + addrs = make([]FMsgAddress, 0, len(h.To)+len(h.AddTo)) for _, addr := range h.To { - appendUnique(addr) + if strings.EqualFold(addr.Domain, Domain) { + addrs = append(addrs, addr) + } } + numLocalTo = len(addrs) for _, addr := range h.AddTo { - appendUnique(addr) + if strings.EqualFold(addr.Domain, Domain) { + addrs = append(addrs, addr) + } } - return addrs + return addrs, numLocalTo } func allLocalRecipientsHaveMessageHash(msgHash []byte, addrs []FMsgAddress) (bool, error) { @@ -1484,7 +1477,7 @@ func storeAcceptedMessage(h *FMsgHeader, codes []byte, acceptedTo []FMsgAddress, } func downloadMessage(c net.Conn, r io.Reader, h *FMsgHeader, skipData bool) error { - addrs := localRecipients(h) + addrs, numLocalTo := localRecipients(h) if len(addrs) == 0 { return fmt.Errorf("%w our domain: %s, not in recipient list", ErrProtocolViolation, Domain) } @@ -1538,17 +1531,6 @@ func downloadMessage(c net.Conn, r io.Reader, h *FMsgHeader, skipData bool) erro defer src.Close() // validate each recipient and copy message for accepted ones - // Build sets of to / add-to addresses for later classification. The two - // may overlap (SPEC §1.4.6.3 NOTE II), in which case the recipient belongs - // to both the original's _to_ list and this batch and is recorded in both. - toSet := make(map[string]bool) - for _, addr := range h.To { - toSet[strings.ToLower(addr.ToString())] = true - } - addToSet := make(map[string]bool) - for _, addr := range h.AddTo { - addToSet[strings.ToLower(addr.ToString())] = true - } acceptedTo := []FMsgAddress{} acceptedAddTo := []FMsgAddress{} var primaryFilepath string @@ -1577,11 +1559,12 @@ func downloadMessage(c net.Conn, r io.Reader, h *FMsgHeader, skipData bool) erro } codes[i] = RejectCodeAccept - key := strings.ToLower(addr.ToString()) - if toSet[key] { + // Entries before numLocalTo came from _to_, the rest from _add to_. An + // address in both lists appears once in each range and is recorded in + // both, rather than being classified exclusively as add-to. + if i < numLocalTo { acceptedTo = append(acceptedTo, addr) - } - if addToSet[key] { + } else { acceptedAddTo = append(acceptedAddTo, addr) } if primaryFilepath == "" { @@ -1700,7 +1683,7 @@ func handleConn(c net.Conn) { // Codes 65 and 64 both require a dup check when challenge was completed. allLocalDup := false if header.ChallengeCompleted && header.InitialResponseCode != AcceptCodeAddTo { - addrs := localRecipients(header) + addrs, _ := localRecipients(header) var err error // Duplicate detection keys on msg.sha256 (the canonical original-form // hash). For an add-to delivery that is header.Pid; the challenge hash diff --git a/cmd/fmsgd/host_test.go b/cmd/fmsgd/host_test.go index c38db03..1af4c4f 100644 --- a/cmd/fmsgd/host_test.go +++ b/cmd/fmsgd/host_test.go @@ -263,15 +263,14 @@ func TestReadToRecipients(t *testing.T) { b = append(b, encodeUInt8String(t, "@bob@example.com")...) h := &FMsgHeader{} - seen, err := readToRecipients(nil, bufio.NewReader(bytes.NewReader(b)), h) - if err != nil { + if err := readToRecipients(nil, bufio.NewReader(bytes.NewReader(b)), h); err != nil { t.Fatalf("readToRecipients returned error: %v", err) } if len(h.To) != 2 { t.Fatalf("len(h.To) = %d, want 2", len(h.To)) } - if !seen["@alice@example.com"] || !seen["@bob@example.com"] { - t.Fatalf("seen map missing expected recipients: %#v", seen) + if h.To[0].ToString() != "@alice@example.com" || h.To[1].ToString() != "@bob@example.com" { + t.Fatalf("unexpected To: %+v", h.To) } } @@ -281,14 +280,12 @@ func TestReadAddToRecipients(t *testing.T) { From: FMsgAddress{User: "alice", Domain: "example.com"}, To: []FMsgAddress{{User: "bob", Domain: "example.com"}}, } - seen := map[string]bool{"@bob@example.com": true} - b := []byte{} b = append(b, encodeUInt8String(t, "@alice@example.com")...) // add-to-from b = append(b, 1) // add-to count b = append(b, encodeUInt8String(t, "@carol@example.com")...) - err := readAddToRecipients(nil, bufio.NewReader(bytes.NewReader(b)), h, seen) + err := readAddToRecipients(nil, bufio.NewReader(bytes.NewReader(b)), h) if err != nil { t.Fatalf("readAddToRecipients returned error: %v", err) } @@ -359,7 +356,7 @@ func TestReadAddToRecipientsRejectsWhenPidMissing(t *testing.T) { h := &FMsgHeader{Flags: FlagHasAddTo} c := &testConn{} - err := readAddToRecipients(c, bufio.NewReader(bytes.NewReader(nil)), h, map[string]bool{}) + err := readAddToRecipients(c, bufio.NewReader(bytes.NewReader(nil)), h) if err == nil { t.Fatalf("expected error when add-to flag is set without pid") } @@ -375,15 +372,13 @@ func TestReadAddToRecipientsRejectsDuplicateAddTo(t *testing.T) { To: []FMsgAddress{{User: "bob", Domain: "example.com"}}, } c := &testConn{} - seen := map[string]bool{"@bob@example.com": true} - b := []byte{} b = append(b, encodeUInt8String(t, "@alice@example.com")...) // add-to-from b = append(b, 2) // add-to count b = append(b, encodeUInt8String(t, "@carol@example.com")...) b = append(b, encodeUInt8String(t, "@carol@example.com")...) - err := readAddToRecipients(c, bufio.NewReader(bytes.NewReader(b)), h, seen) + err := readAddToRecipients(c, bufio.NewReader(bytes.NewReader(b)), h) if err == nil { t.Fatalf("expected duplicate add-to error") } diff --git a/cmd/fmsgd/sender.go b/cmd/fmsgd/sender.go index 0e02485..969ac8e 100644 --- a/cmd/fmsgd/sender.go +++ b/cmd/fmsgd/sender.go @@ -577,10 +577,11 @@ func deliverUnit(db *sql.DB, target pendingTarget, h *FMsgHeader, table string, } }() - // Per-recipient response codes arrive one per DISTINCT recipient, in the - // order the address first appears scanning _to_ then _add to_ (SPEC §10.2 - // step 6). Only this unit's locked recipients are recorded; the - // recipients carried only to reconstruct that ordering stay unlocked. + // Per-recipient response codes arrive one per recipient entry, in to-field + // order then add-to order (SPEC §10.2 step 6). An address in both lists is a + // recipient of each and so is counted, and answered, twice. Only this + // unit's locked recipients are recorded; the recipients carried only to + // reconstruct that ordering stay unlocked. lockedSet := make(map[string]bool, len(locked)) for _, a := range locked { lockedSet[strings.ToLower(a)] = true @@ -590,23 +591,13 @@ func deliverUnit(db *sql.DB, target pendingTarget, h *FMsgHeader, table string, isLocked bool } var domainRecips []domainRecip - // Recipients are a SET (SPEC Terms): an address in both _to_ and _add to_ - // is one recipient and the receiving host sends exactly one code for it, - // in its _to_ position. Counting the occurrence twice here would read one - // byte too many and desync the stream. - seenRecip := make(map[string]bool) appendDomain := func(addrs []FMsgAddress) { for _, addr := range addrs { if !strings.EqualFold(addr.Domain, target.Domain) { continue } s := addr.ToString() - key := strings.ToLower(s) - if seenRecip[key] { - continue - } - seenRecip[key] = true - domainRecips = append(domainRecips, domainRecip{addr: s, isLocked: lockedSet[key]}) + domainRecips = append(domainRecips, domainRecip{addr: s, isLocked: lockedSet[strings.ToLower(s)]}) } } appendDomain(h.To)