diff --git a/backend/06-mbox.go b/backend/06-mbox.go index 73558bb..e005786 100644 --- a/backend/06-mbox.go +++ b/backend/06-mbox.go @@ -439,7 +439,7 @@ func readMboxMessagesBytes(b []byte) [][]byte { for _, line := range lines { if bytes.HasPrefix(line, []byte("From ")) { if inMsg && cur.Len() > 0 { - msgs = append(msgs, bytes.TrimRight(cur.Bytes(), "\n")) + msgs = append(msgs, cloneTrimmedMessage(cur.Bytes())) cur.Reset() } inMsg = true @@ -455,11 +455,16 @@ func readMboxMessagesBytes(b []byte) [][]byte { _ = cur.WriteByte('\n') } if inMsg && cur.Len() > 0 { - msgs = append(msgs, bytes.TrimRight(cur.Bytes(), "\n")) + msgs = append(msgs, cloneTrimmedMessage(cur.Bytes())) } return msgs } +func cloneTrimmedMessage(b []byte) []byte { + trimmed := bytes.TrimRight(b, "\n") + return append([]byte(nil), trimmed...) +} + func readMboxListFromIndex(path string) ([]MboxEntry, bool) { accountID, folder, ok := mboxIndexContext(path) if !ok { diff --git a/backend/06-mbox_test.go b/backend/06-mbox_test.go index 5bf7884..ec495ff 100644 --- a/backend/06-mbox_test.go +++ b/backend/06-mbox_test.go @@ -4,6 +4,7 @@ import ( "bytes" "os" "path/filepath" + "strings" "testing" "time" @@ -203,6 +204,42 @@ func TestPlainMboxPartialIndexIsRebuiltBeforeList(t *testing.T) { } } +func TestReadMboxMessagesBytesClonesBufferRecords(t *testing.T) { + mbox := []byte(strings.Join([]string{ + "From one@example.com Tue Jul 14 10:00:00 2026", + "Message-ID: ", + "Subject: One", + "", + "short", + "From two@example.com Tue Jul 14 10:01:00 2026", + "Message-ID: ", + "Subject: Two", + "", + "this message is deliberately much longer than the first one", + "and has another line", + "From three@example.com Tue Jul 14 10:02:00 2026", + "Message-ID: ", + "Subject: Three", + "", + "tiny", + "", + }, "\n")) + + msgs := readMboxMessagesBytes(mbox) + if len(msgs) != 3 { + t.Fatalf("expected 3 messages, got %d", len(msgs)) + } + wants := []string{"Subject: One", "Subject: Two", "Subject: Three"} + for i, want := range wants { + if !bytes.Contains(msgs[i], []byte(want)) { + t.Fatalf("message %d does not contain %q: %q", i, want, msgs[i]) + } + } + if bytes.Contains(msgs[0], []byte("Subject: Two")) || bytes.Contains(msgs[1], []byte("Subject: Three")) { + t.Fatalf("messages share buffer contents: %#q", msgs) + } +} + func testRawMessage(id, subject string) RawMessage { return RawMessage{ MessageID: id, diff --git a/bug-mbox-reader.md b/bug-mbox-reader.md new file mode 100644 index 0000000..db9fd5f --- /dev/null +++ b/bug-mbox-reader.md @@ -0,0 +1,90 @@ +# 🔴 KRITISCH — mbox-Leser liefert falsche Mails (Buffer-Aliasing) + +**Schweregrad: hoch.** Der Leser gibt fuer fast jeden Index dieselbe (falsche) +Mail zurueck. Betrifft Vorschau, manuelles Verschieben, Archiv-Export und +Archiv-Dedup. + +**Entwarnung vorweg:** Die **Archiv-Dateien auf der Platte sind korrekt.** Der +Schreiber ist ein anderer Pfad und haengt die Rohbytes richtig an. Geprueft: +In `colak@dr-gold.de/INBOX.mbox` steht an Position 537 exakt das, was +`mbox_index` sagt. **Kaputt ist ausschliesslich der Leser.** Es sind keine Daten +verloren. + +## Die Ursache — eine Zeile + +[`backend/06-mbox.go:441`](backend/06-mbox.go), `readMboxMessagesBytes`: + +```go +if inMsg && cur.Len() > 0 { + msgs = append(msgs, bytes.TrimRight(cur.Bytes(), "\n")) // Slice ZEIGT IN cur's Puffer + cur.Reset() // Puffer wird wiederverwendet +} +``` + +`bytes.Buffer.Bytes()` liefert **keine Kopie**, sondern einen Slice **in den +internen Puffer**. Direkt danach `cur.Reset()` — der Puffer wird fuer die +naechste Mail ueberschrieben. **Alle zuvor angehaengten Slices zeigen auf +denselben Speicher** und tragen am Ende den zuletzt geschriebenen Inhalt. + +Der Lehrbuchfehler „`Buffer.Bytes()` + `Reset()` ohne Kopie". + +## Der Fix + +```go +if inMsg && cur.Len() > 0 { + trimmed := bytes.TrimRight(cur.Bytes(), "\n") + msg := make([]byte, len(trimmed)) + copy(msg, trimmed) // <-- ECHTE Kopie + msgs = append(msgs, msg) + cur.Reset() +} +``` +(Dasselbe fuer den Abschluss nach der Schleife, falls dort ebenfalls +`cur.Bytes()` ohne Kopie angehaengt wird — bitte pruefen.) + +**Besser noch:** `bytes.Buffer` ganz vermeiden und die Nachrichten ueber +Byte-Offsets aus dem Original-Slice schneiden (`b[start:end]`) — dann gibt es +kein Aliasing-Risiko und keine Kopie zu viel. + +## Reproduktion (gemessen an `colak@dr-gold.de` / INBOX, 678 Mails) + +``` +idx | mbox_index sagt | Vorschau liefert + 0 | Katalogseite | Angebot: CARRYMATE Transportgriffe FALSCH + 1 | Angebot: CARRYMATE Transportgriffe | Baustoff + Metall FALSCH + 2 | Schoenes Wochenende | Baustoff + Metall FALSCH + 3 | Bestellung '150129' | Baustoff + Metall FALSCH + ... + 15 | Per E-Mail senden: Bestellschein.doc| Baustoff + Metall FALSCH +``` +Ab Index 1 kommt **immer dieselbe** Mail zurueck. Die **Liste** und +`mbox_index` stimmen ueberein — nur der Leser luegt. + +## Blast Radius — bitte alle pruefen + +`ReadMboxMessage` / `readMboxMessages` haengen an: + +| Stelle | Wirkung des Bugs | +|---|---| +| `08-viewer.go:51` | **Vorschau** zeigt die falsche Mail | +| `08-viewer.go:83`, `:335` | dito (Ziel-/Transfer-Vorschau) | +| **`08-viewer.go:489`** | **manuelles Verschieben** → schreibt die **falsche Mail** in ein echtes Ziel-Postfach | +| `08-viewer.go:599` | Suche liefert falsche Treffer | +| `00-router.go:1320` | Transfer-Vorschau | +| **`12-archive-tools.go:291`, `:332`** | **Archiv-Export** (ZIP) und **Archiv-Dedup** → Export koennte falsche Mails enthalten, Dedup koennte **die falschen loeschen** | + +**Wichtig:** Falls `/archives/dedup` oder das manuelle Verschieben schon benutzt +wurden, muss geprueft werden, ob dabei Schaden entstanden ist. + +## Abnahme + +1. Unit-Test, der genau das faengt: mbox mit **3 unterschiedlich langen** Mails + schreiben, dann `readMboxMessagesBytes` aufrufen und pruefen, dass + `msgs[0] != msgs[1] != msgs[2]` und jede den **erwarteten Betreff** hat. + (Ein Test mit gleich langen Mails wuerde den Bug NICHT finden — deshalb + unterschiedliche Laengen, damit der Puffer waechst.) +2. Live: `/view?account=colak@dr-gold.de&folder=INBOX`, dann fuer die Indizes + 0, 1, 2, 300, 677 die Vorschau abrufen → Betreff und Datum muessen **exakt** + dem entsprechen, was `mbox_index` fuer diese `seq` sagt. +3. Ich (Claude) fahre den Sweep ueber alle 678 Indizes von colak gegen + `mbox_index` — **0 Abweichungen** ist die Latte.