fix(imap): samme mail blev en ny opgave hver gang, og uddraget var rå MIME

To fejl fra ét skærmbillede.

Markøren stod stille. Den blev sat ud fra uid'en i fetch-svaret, og den kommer
ikke fra alle servere — så next-run læste fra nul hver gang. Den sættes nu ud fra
hvad SØGNINGEN fandt, når kørslen kom hele vejen igennem: så er alt op til det
punkt betragtet, uanset hvad fetch-svaret bar med. En afbrudt kørsel flytter kun
til den sidste, den faktisk lavede en opgave af. Prøven mod hukommelsesserveren
ville aldrig have fanget det, for den svarer altid med uid.

Og uddraget hentede BODY[TEXT], som ved multipart er grænser og dele-hoveder og
det hele — derfor stod der "--=_be71cb39… Content-Transfer-Encoding: 8bit" foran
teksten i hver eneste opgave. Nu hentes hele beskeden og MIME-strukturen gås
igennem: text/plain foretrækkes, HTML er reserven med mærkerne pillet af. Groft,
for det er linjen under en overskrift og ikke en gengivelsesmotor.

Multipart-prøven er sat på prøve: med den gamle hentning fejler den med præcis
den tekst, skærmbilledet viste.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Kristian Wind
2026-08-18 22:44:00 +02:00
co-authored by Claude Opus 5
parent 30adc69cb5
commit f3fecbd0b4
3 changed files with 214 additions and 27 deletions
+9 -1
View File
@@ -147,7 +147,7 @@ func (s *Server) SyncMailbox(ctx context.Context, user *store.User, m *store.Mai
}
defer client.Close()
messages, err := client.Since(m.LastUID, 25)
messages, searched, err := client.Since(m.LastUID, 25)
if err != nil {
return 0, err
}
@@ -197,6 +197,14 @@ func (s *Server) SyncMailbox(ctx context.Context, user *store.User, m *store.Mai
s.hub.Publish(projectID, "task.created", toTaskJSON(*task))
}
// A run that got all the way through moves the marker to the whole of what the
// search considered, not merely to the last message it happened to read. That
// is what stops the same mail becoming a task again on the next run when the
// server did not put a uid in the fetch reply.
if ctx.Err() == nil && searched > highest {
highest = searched
}
// Written even when the loop was cut short, and outside the request's context
// so a cancelled request still records what it did.
if highest > m.LastUID {
+120 -22
View File
@@ -16,15 +16,18 @@
package imap
import (
"bytes"
"crypto/tls"
"crypto/x509"
"fmt"
"io"
"net"
"strings"
"time"
"github.com/emersion/go-imap/v2"
"github.com/emersion/go-imap/v2/imapclient"
"github.com/emersion/go-message"
)
// Message is one mail, reduced to what a task needs.
@@ -108,9 +111,13 @@ func (c *Client) Close() error {
// that consumed it would be taking something away. Flagging a mail is a deliberate
// act with no other meaning, which is exactly what is wanted — the same reason the
// Gmail side reads stars.
func (c *Client) Since(uid uint32, limit int) ([]Message, error) {
// The second return is the highest uid the search considered. A caller that got
// through every message can move its marker to that, and then it does not matter
// whether the server put a uid in each fetch reply — which some do not, and which
// is how the same mail became a task again on every run.
func (c *Client) Since(uid uint32, limit int) ([]Message, uint32, error) {
if _, err := c.c.Select(c.folder, &imap.SelectOptions{ReadOnly: true}).Wait(); err != nil {
return nil, fmt.Errorf("open %s: %w", c.folder, err)
return nil, 0, fmt.Errorf("open %s: %w", c.folder, err)
}
// From uid+1 to the end. A mailbox with nothing new answers with no uids
@@ -124,12 +131,12 @@ func (c *Client) Since(uid uint32, limit int) ([]Message, error) {
}
found, err := c.c.UIDSearch(criteria, nil).Wait()
if err != nil {
return nil, fmt.Errorf("search %s: %w", c.folder, err)
return nil, 0, fmt.Errorf("search %s: %w", c.folder, err)
}
uids := found.AllUIDs()
if len(uids) == 0 {
return nil, nil
return nil, uid, nil
}
if limit > 0 && len(uids) > limit {
// The newest ones. A mailbox flagged over years would otherwise make the
@@ -139,29 +146,51 @@ func (c *Client) Since(uid uint32, limit int) ([]Message, error) {
want := imap.UIDSetNum(uids...)
fetch := &imap.FetchOptions{
Envelope: true,
UID: true,
BodySection: []*imap.FetchItemBodySection{{Specifier: imap.PartSpecifierText, Peek: true}},
Envelope: true,
UID: true,
// The whole message, headers included, so the MIME structure can be walked.
// TEXT alone gives the body as bytes — boundaries and part headers and all —
// which is what put "--=_be71cb39… Content-Transfer-Encoding: 8bit" at the
// front of every task made from a multipart mail.
BodySection: []*imap.FetchItemBodySection{{Peek: true}},
}
var highest uint32
for _, u := range uids {
if uint32(u) > highest {
highest = uint32(u)
}
}
msgs, err := c.c.Fetch(want, fetch).Collect()
if err != nil {
return nil, fmt.Errorf("fetch from %s: %w", c.folder, err)
return nil, 0, fmt.Errorf("fetch from %s: %w", c.folder, err)
}
out := make([]Message, 0, len(msgs))
for _, m := range msgs {
for i, m := range msgs {
if m.Envelope == nil {
continue
}
// The uid comes from the search when the fetch reply does not carry one.
// A missing uid is not cosmetic: it is the marker the next run starts from,
// and a marker stuck at zero makes every message arrive again on every run.
// That is what happened against a real server while the tests stayed green,
// because the server they run against always answers with it.
msgUID := uint32(m.UID)
if msgUID == 0 && i < len(uids) {
msgUID = uint32(uids[i])
}
out = append(out, Message{
UID: uint32(m.UID),
UID: msgUID,
From: address(m.Envelope.From),
Subject: strings.TrimSpace(m.Envelope.Subject),
Snippet: snippet(m),
Date: m.Envelope.Date,
})
}
return out, nil
return out, highest, nil
}
// address renders the first sender as "Name <addr>", or just the address.
@@ -177,23 +206,92 @@ func address(list []imap.Address) string {
return a.Name + " <" + addr + ">"
}
// snippet is the first readable line or so of the body.
// snippet is the first readable text in the mail.
//
// No MIME walk and no HTML stripping yet: this is what shows under a task's title,
// and a wrong-but-short line is a smaller problem than a parser that has to be
// right about every mail ever sent. The full mail stays where it is.
// It walks the MIME structure rather than taking the body as bytes. text/plain is
// preferred and HTML is the fallback with its tags stripped — crudely, because
// this is the line under a task's title and not a rendering engine. The mail
// itself stays where it is.
const snippetLimit = 400
func snippet(m *imapclient.FetchMessageBuffer) string {
for _, body := range m.BodySection {
text := strings.TrimSpace(string(body.Bytes))
if text == "" {
continue
if text := readable(body.Bytes); text != "" {
return cut(text)
}
if len(text) > snippetLimit {
text = text[:snippetLimit] + "…"
}
return text
}
return ""
}
func readable(raw []byte) string {
entity, err := message.Read(bytes.NewReader(raw))
if err != nil {
// Not something this parser recognises. The raw bytes read badly but read,
// and a mail is not worth losing over its own headers.
return strings.TrimSpace(string(raw))
}
return strings.TrimSpace(fromEntity(entity, 0))
}
// fromEntity descends into a multipart mail. The depth guard is for the mail that
// nests parts inside parts inside parts — rare, and cheaper to bound than to trust.
func fromEntity(entity *message.Entity, depth int) string {
if depth > 4 {
return ""
}
if parts := entity.MultipartReader(); parts != nil {
var html string
for {
part, err := parts.NextPart()
if err != nil {
break
}
if text := fromEntity(part, depth+1); text != "" {
kind, _, _ := part.Header.ContentType()
if kind != "text/html" {
return text
}
if html == "" {
html = text
}
}
}
return html
}
body, _ := io.ReadAll(entity.Body)
kind, _, _ := entity.Header.ContentType()
if kind == "text/html" {
return stripTags(string(body))
}
return strings.TrimSpace(string(body))
}
// stripTags removes markup without pretending to understand it. Enough for one
// line of preview, and honest about being no more than that.
func stripTags(html string) string {
var b strings.Builder
depth := 0
for _, r := range html {
switch {
case r == '<':
depth++
case r == '>':
if depth > 0 {
depth--
}
case depth == 0:
b.WriteRune(r)
}
}
return strings.Join(strings.Fields(b.String()), " ")
}
func cut(text string) string {
runes := []rune(text)
if len(runes) > snippetLimit {
return string(runes[:snippetLimit]) + "…"
}
return text
}
+85 -4
View File
@@ -37,7 +37,7 @@ func TestFlaggedMailComesBackAsMessages(t *testing.T) {
}
defer client.Close()
msgs, err := client.Since(0, 25)
msgs, _, err := client.Since(0, 25)
if err != nil {
t.Fatal(err)
}
@@ -59,7 +59,7 @@ func TestFlaggedMailComesBackAsMessages(t *testing.T) {
// And reading on from a marker returns only what is newer, which is the whole
// of the deduplication.
rest, err := client.Since(msgs[0].UID, 25)
rest, _, err := client.Since(msgs[0].UID, 25)
if err != nil {
t.Fatal(err)
}
@@ -92,6 +92,9 @@ func TestAnUntrustedCertificateIsRefused(t *testing.T) {
type mail struct {
subject, from, body string
flagged bool
// raw replaces the whole message when set, which is the only way to get a
// genuinely multipart one in.
raw string
}
// serveMailbox starts an in-process IMAP server over TLS and returns its address
@@ -112,8 +115,11 @@ func serveMailbox(t *testing.T, mails []mail) (string, *x509.CertPool) {
if m.flagged {
flags = append(flags, imap.FlagFlagged)
}
raw := "From: " + m.from + "\r\nSubject: " + m.subject +
"\r\nDate: Tue, 18 Aug 2026 10:00:00 +0200\r\n\r\n" + m.body + "\r\n"
raw := m.raw
if raw == "" {
raw = "From: " + m.from + "\r\nSubject: " + m.subject +
"\r\nDate: Tue, 18 Aug 2026 10:00:00 +0200\r\n\r\n" + m.body + "\r\n"
}
if _, err := user.Append("INBOX", strings.NewReader(raw), &imap.AppendOptions{
Flags: flags,
}); err != nil {
@@ -181,3 +187,78 @@ func selfSigned(t *testing.T) (tls.Certificate, *x509.CertPool) {
pool.AppendCertsFromPEM(certPEM)
return cert, pool
}
// The bug this was written for: every press of "Fetch now" made the same mail
// into a task again. The marker the next run starts from is the uid, and if it
// comes back as zero the marker never moves.
func TestEveryMessageCarriesAUID(t *testing.T) {
addr, pool := serveMailbox(t, []mail{
{subject: "En", from: "a@example.dk", body: "en", flagged: true},
{subject: "To", from: "b@example.dk", body: "to", flagged: true},
})
client, err := Dial(Account{Host: addr, Username: "kw", Password: "hemmelig", RootCAs: pool})
if err != nil {
t.Fatal(err)
}
defer client.Close()
msgs, _, err := client.Since(0, 25)
if err != nil {
t.Fatal(err)
}
for _, m := range msgs {
if m.UID == 0 {
t.Fatalf("%q came back without a uid; the marker would never move and "+
"every run would make it into a task again", m.Subject)
}
}
if msgs[0].UID == msgs[1].UID {
t.Error("two messages share a uid")
}
}
// The other half of the same report: the snippet under a task's title read
// "--=_be71cb39… Content-Transfer-Encoding: 8bit Content-Type: text/plain".
func TestAMultipartMailReadsAsItsText(t *testing.T) {
const boundary = "=_be71cb3975bdfbc60a16327cb2f49c23"
body := "This is a multi-part message.\r\n" +
"--" + boundary + "\r\n" +
"Content-Transfer-Encoding: 8bit\r\n" +
"Content-Type: text/plain; charset=UTF-8; format=flowed\r\n\r\n" +
"Hej igen,\r\n\r\nVi har normalt 3 slags. :)\r\n" +
"--" + boundary + "\r\n" +
"Content-Type: text/html; charset=UTF-8\r\n\r\n" +
"<html><body><p>Hej igen</p></body></html>\r\n" +
"--" + boundary + "--\r\n"
addr, pool := serveMailbox(t, []mail{{
flagged: true,
raw: "Content-Type: multipart/alternative; boundary=\"" + boundary + "\"\r\n" +
"From: kontakt@mikrobagt.dk\r\nSubject: Re: Kaffebestilling\r\n" +
"Date: Tue, 18 Aug 2026 10:00:00 +0200\r\n\r\n" + body,
}})
client, err := Dial(Account{Host: addr, Username: "kw", Password: "hemmelig", RootCAs: pool})
if err != nil {
t.Fatal(err)
}
defer client.Close()
msgs, _, err := client.Since(0, 25)
if err != nil {
t.Fatal(err)
}
if len(msgs) != 1 {
t.Fatalf("got %d messages", len(msgs))
}
got := msgs[0].Snippet
for _, junk := range []string{boundary, "Content-Transfer-Encoding", "Content-Type", "<html>", "<p>"} {
if strings.Contains(got, junk) {
t.Errorf("the snippet still carries %q: %q", junk, got)
}
}
if !strings.HasPrefix(got, "Hej igen") {
t.Errorf("the snippet does not start with the text of the mail: %q", got)
}
}