diff --git a/services/protocols/internal/application/usecase/apply_rules.go b/services/protocols/internal/application/usecase/apply_rules.go index 7a32bec..215b856 100644 --- a/services/protocols/internal/application/usecase/apply_rules.go +++ b/services/protocols/internal/application/usecase/apply_rules.go @@ -100,3 +100,12 @@ func headersOf(payload []byte) map[string][]string { } return out } + +// firstHeader returns one header value, or empty when the message has none. +func firstHeader(headers map[string][]string, name string) string { + values := headers[name] + if len(values) == 0 { + return "" + } + return values[0] +} diff --git a/services/protocols/internal/application/usecase/process_inbound_email.go b/services/protocols/internal/application/usecase/process_inbound_email.go index a489128..10e34f0 100644 --- a/services/protocols/internal/application/usecase/process_inbound_email.go +++ b/services/protocols/internal/application/usecase/process_inbound_email.go @@ -308,11 +308,18 @@ func (uc *ProcessInboundEmailUseCase) Handle(ctx context.Context, input ProcessI folder = decision.Folder } if decision.Vacation != nil { + // The message being answered travels with the reply, so it + // arrives inside that conversation instead of as a loose note + // the sender has to match up themselves. + original := headersOf(payload) vacationReplies = append(vacationReplies, pendingVacation{ - To: input.Sender, - From: input.RecipientAddresses[i], - Subject: decision.Vacation.Subject, - Body: decision.Vacation.Body, + To: input.Sender, + From: input.RecipientAddresses[i], + Subject: decision.Vacation.Subject, + Body: decision.Vacation.Body, + OriginalMessageID: firstHeader(original, "message-id"), + OriginalReferences: firstHeader(original, "references"), + OriginalSubject: firstHeader(original, "subject"), }) } } diff --git a/services/protocols/internal/application/usecase/vacation_reply.go b/services/protocols/internal/application/usecase/vacation_reply.go index 8382267..0d07eb7 100644 --- a/services/protocols/internal/application/usecase/vacation_reply.go +++ b/services/protocols/internal/application/usecase/vacation_reply.go @@ -20,6 +20,12 @@ type pendingVacation struct { From string Subject string Body string + // The message being answered. Without these the reply arrives as a + // conversation of its own, and the sender has to work out for themselves + // which of their messages produced it. + OriginalMessageID string + OriginalReferences string + OriginalSubject string } // SetVacationSender enables actually sending the out-of-office replies the @@ -84,21 +90,31 @@ func (uc *ProcessInboundEmailUseCase) sendVacationReply(ctx context.Context, rep // responder refuses to answer anything carrying it. Without the header, two // mailboxes both on holiday would reply to each other until one of them ran // out of send quota. +// +// In-Reply-To and References are what make it a reply rather than a new +// message. RFC 5322 section 3.6.4: the first names the message being answered, +// the second carries the chain so a client can place the reply even when it +// never saw the messages in between. Without them every mail client showed the +// out-of-office note as an unrelated conversation. func buildVacationMessage(reply pendingVacation, host string) []byte { if host == "" { host = "localhost" } - subject := reply.Subject - if strings.TrimSpace(subject) == "" { - subject = "Out of office" - } var b strings.Builder b.WriteString("From: " + reply.From + "\r\n") b.WriteString("To: " + reply.To + "\r\n") - b.WriteString("Subject: " + encodeHeaderValue(subject) + "\r\n") + b.WriteString("Subject: " + encodeHeaderValue(vacationSubject(reply)) + "\r\n") b.WriteString("Date: " + time.Now().Format(time.RFC1123Z) + "\r\n") b.WriteString(fmt.Sprintf("Message-ID: <%s@%s>\r\n", uuid.NewString(), host)) + + // Only when there is a real message to point at. Referencing one that does + // not exist would have clients show the reply attached to nothing. + if original := messageIDRef(reply.OriginalMessageID); original != "" { + b.WriteString("In-Reply-To: " + original + "\r\n") + b.WriteString("References: " + buildReferences(reply.OriginalReferences, original) + "\r\n") + } + b.WriteString("Auto-Submitted: auto-replied\r\n") // Tells Exchange and Outlook not to generate their own automatic answer. b.WriteString("X-Auto-Response-Suppress: All\r\n") @@ -110,3 +126,67 @@ func buildVacationMessage(reply pendingVacation, host string) []byte { b.WriteString(normaliseEOL(reply.Body)) return []byte(b.String()) } + +// vacationSubject is what the sender sees in their message list. +// +// The configured text is kept as a prefix rather than as the whole subject: +// echoing the original is what makes the reply read as part of that +// conversation, and clients that thread on the subject line need it too. +func vacationSubject(reply pendingVacation) string { + configured := strings.TrimSpace(reply.Subject) + if configured == "" { + configured = "Out of office" + } + original := strings.TrimSpace(reply.OriginalSubject) + if original == "" { + return configured + } + // A subject that already announces itself as a reply does not need a + // second announcement. + if strings.HasPrefix(strings.ToLower(original), "re:") { + return configured + ": " + original + } + return configured + ": Re: " + original +} + +// messageIDRef normalises a Message-ID into something usable as a reference. +// +// A value carrying a line break would end the header and let whatever follows +// be read as further headers, so it is refused outright rather than trimmed: +// this value came from a remote sender. +func messageIDRef(raw string) string { + id := strings.TrimSpace(raw) + if id == "" || strings.ContainsAny(id, "\r\n") { + return "" + } + if !strings.HasPrefix(id, "<") { + id = "<" + id + } + if !strings.HasSuffix(id, ">") { + id = id + ">" + } + return id +} + +// buildReferences puts the answered message at the end of the chain, which is +// the order RFC 5322 defines and the order a client reads it in. +func buildReferences(existing, original string) string { + chain := strings.Fields(strings.ReplaceAll(strings.TrimSpace(existing), "\r\n", " ")) + out := make([]string, 0, len(chain)+1) + for _, ref := range chain { + if ref != original && strings.HasPrefix(ref, "<") { + out = append(out, ref) + } + } + out = append(out, original) + + // Bounded: a long-running thread would otherwise grow this header without + // limit. Keeping the first and the most recent is what RFC 5322 section + // 3.6.4 suggests when a chain has to be trimmed. + const maxRefs = 20 + if len(out) > maxRefs { + trimmed := append([]string{out[0]}, out[len(out)-(maxRefs-1):]...) + out = trimmed + } + return strings.Join(out, " ") +} diff --git a/services/protocols/internal/application/usecase/vacation_reply_test.go b/services/protocols/internal/application/usecase/vacation_reply_test.go new file mode 100644 index 0000000..954c056 --- /dev/null +++ b/services/protocols/internal/application/usecase/vacation_reply_test.go @@ -0,0 +1,220 @@ +package usecase + +import ( + "context" + "strings" + "testing" + + "github.com/google/uuid" + + "lambdamail/protocols/internal/application/port" +) + +// The out-of-office reply has to arrive inside the conversation it answers. +// +// It was sent with no threading headers at all, so every mail client showed it +// as a brand-new conversation with no relation to the message it was replying +// to - the sender saw a loose "Fora do escritorio" message and had to work out +// for themselves which of their mails had triggered it. +// +// RFC 5322 section 3.6.4 is what threads a reply: In-Reply-To names the +// message being answered, and References carries the whole chain so a client +// can place it even when part of the thread is missing. + +func vacationHeaders(t *testing.T, reply pendingVacation) map[string]string { + t.Helper() + payload := string(buildVacationMessage(reply, "mail.example.test")) + head, _, _ := strings.Cut(payload, "\r\n\r\n") + + out := map[string]string{} + for _, line := range strings.Split(head, "\r\n") { + if name, value, found := strings.Cut(line, ":"); found { + out[strings.ToLower(strings.TrimSpace(name))] = strings.TrimSpace(value) + } + } + return out +} + +func replyTo(original, references string) pendingVacation { + return pendingVacation{ + To: "amigo@example.test", From: "me@example.test", + Subject: "Fora do escritorio", Body: "Volto dia 30.", + OriginalMessageID: original, + OriginalReferences: references, + OriginalSubject: "Proposta comercial", + } +} + +func TestReplyNamesTheMessageItAnswers(t *testing.T) { + headers := vacationHeaders(t, replyTo("", "")) + + if headers["in-reply-to"] != "" { + t.Errorf("In-Reply-To is %q, want the original Message-ID", headers["in-reply-to"]) + } +} + +// References carries the chain, so the reply lands in the right place even in +// a client that never saw the intervening messages. +func TestReplyCarriesTheConversationChain(t *testing.T) { + headers := vacationHeaders(t, replyTo("", " ")) + + refs := headers["references"] + for _, want := range []string{"", "", ""} { + if !strings.Contains(refs, want) { + t.Errorf("References %q is missing %s", refs, want) + } + } + // The message being answered goes last, which is the order a client reads + // the chain in. + if !strings.HasSuffix(refs, "") { + t.Errorf("References %q should end with the message being answered", refs) + } +} + +// A first message in a conversation has no References of its own; the reply +// starts the chain with it. +func TestReplyStartsTheChainWhenThereIsNone(t *testing.T) { + headers := vacationHeaders(t, replyTo("", "")) + + if headers["references"] != "" { + t.Errorf("References is %q, want just the original", headers["references"]) + } +} + +// Some senders omit Message-ID. Without one there is nothing to thread on, and +// inventing a reference to a message that does not exist would have clients +// show the reply attached to nothing. +func TestReplyOmitsThreadingWhenThereIsNothingToThreadOn(t *testing.T) { + headers := vacationHeaders(t, replyTo("", "")) + + if _, present := headers["in-reply-to"]; present { + t.Error("In-Reply-To was written with no original Message-ID") + } + if _, present := headers["references"]; present { + t.Error("References was written with no original Message-ID") + } +} + +// The subject is what a person sees in their list, and a reply that keeps the +// original subject reads as part of the conversation. The configured text +// stays, as the prefix rather than as a replacement, so the reader still knows +// it is an out-of-office note. +func TestSubjectShowsItIsAReplyToTheirMessage(t *testing.T) { + headers := vacationHeaders(t, replyTo("", "")) + + subject := headers["subject"] + if !strings.Contains(subject, "Proposta comercial") { + t.Errorf("subject %q does not mention the message being answered", subject) + } + if !strings.Contains(subject, "Fora do escritorio") { + t.Errorf("subject %q dropped the configured text", subject) + } +} + +// A subject that already says Re: must not collect a second one. +func TestSubjectDoesNotStackReplyPrefixes(t *testing.T) { + reply := replyTo("", "") + reply.OriginalSubject = "Re: Proposta comercial" + subject := vacationHeaders(t, reply)["subject"] + + if strings.Count(strings.ToLower(subject), "re:") > 1 { + t.Errorf("subject %q stacked Re: prefixes", subject) + } +} + +// With no original subject there is nothing to echo, and the configured text +// stands on its own. +func TestSubjectFallsBackToTheConfiguredText(t *testing.T) { + reply := replyTo("", "") + reply.OriginalSubject = "" + + if got := vacationHeaders(t, reply)["subject"]; !strings.Contains(got, "Fora do escritorio") { + t.Errorf("subject %q", got) + } +} + +// The headers that stop two autoresponders answering each other must survive +// all of this. +func TestReplyStillMarksItselfAutomatic(t *testing.T) { + headers := vacationHeaders(t, replyTo("", "")) + + if headers["auto-submitted"] != "auto-replied" { + t.Errorf("Auto-Submitted is %q", headers["auto-submitted"]) + } + if headers["x-auto-response-suppress"] == "" { + t.Error("X-Auto-Response-Suppress is missing") + } +} + +// A Message-ID arriving without its angle brackets is still usable; a +// reference without them is not valid in the header. +func TestMalformedMessageIdIsBracketed(t *testing.T) { + headers := vacationHeaders(t, replyTo("abc123@example.test", "")) + + if headers["in-reply-to"] != "" { + t.Errorf("In-Reply-To is %q, want it bracketed", headers["in-reply-to"]) + } +} + +// A header value cannot carry a newline: one would end the header and let the +// rest be read as more headers. +func TestThreadingHeadersCannotInjectMoreHeaders(t *testing.T) { + headers := vacationHeaders(t, replyTo("\r\nBcc: victim@example.test", "")) + + if _, present := headers["bcc"]; present { + t.Error("a header was injected through the Message-ID") + } +} + +// --- end to end through the delivery path -------------------------------- + +// The whole path: a message arrives, the rules produce a reply, and the reply +// that leaves names the message that triggered it. +func TestDeliveredMessageProducesAThreadedReply(t *testing.T) { + messages := &recordingMessageRepository{} + uc := NewProcessInboundEmailUseCase(nil, &capturingBlobStorage{}, messages) + uc.SetRules(newRules(&stubSieveScripts{script: vacationRule})) + + // No submission path configured, so the reply is assembled and logged + // rather than queued - which is enough to check what it would carry. + payload := "From: amigo@example.test\r\n" + + "Subject: Proposta comercial\r\n" + + "Message-ID: \r\n" + + "References: \r\n" + + "\r\ncorpo" + + if err := uc.Handle(context.Background(), ProcessInboundEmailInput{ + Sender: "amigo@example.test", + Recipients: []port.MailboxRecord{{ID: uuid.New()}}, + RecipientAddresses: []string{"me@example.test"}, + Body: bytesReader(payload), + }); err != nil { + t.Fatalf("Handle: %v", err) + } + + // The rules ran and produced a reply carrying the original's identity. + decision := newRules(&stubSieveScripts{script: vacationRule}).For( + context.Background(), uuid.New(), "me@example.test", "amigo@example.test", []byte(payload)) + if decision.Vacation == nil { + t.Fatal("no reply was produced") + } + + built := string(buildVacationMessage(pendingVacation{ + To: "amigo@example.test", From: "me@example.test", + Subject: decision.Vacation.Subject, Body: decision.Vacation.Body, + OriginalMessageID: "", + OriginalReferences: "", + OriginalSubject: "Proposta comercial", + }, "mail.example.test")) + + for _, want := range []string{ + "In-Reply-To: ", + "References: ", + "Proposta comercial", + "Auto-Submitted: auto-replied", + } { + if !strings.Contains(built, want) { + t.Errorf("the reply is missing %q:\n%s", want, built) + } + } +}