Fix plain mbox reader buffer aliasing
This commit is contained in:
parent
e63a513cff
commit
6c4073a01c
3 changed files with 134 additions and 2 deletions
|
|
@ -439,7 +439,7 @@ func readMboxMessagesBytes(b []byte) [][]byte {
|
||||||
for _, line := range lines {
|
for _, line := range lines {
|
||||||
if bytes.HasPrefix(line, []byte("From ")) {
|
if bytes.HasPrefix(line, []byte("From ")) {
|
||||||
if inMsg && cur.Len() > 0 {
|
if inMsg && cur.Len() > 0 {
|
||||||
msgs = append(msgs, bytes.TrimRight(cur.Bytes(), "\n"))
|
msgs = append(msgs, cloneTrimmedMessage(cur.Bytes()))
|
||||||
cur.Reset()
|
cur.Reset()
|
||||||
}
|
}
|
||||||
inMsg = true
|
inMsg = true
|
||||||
|
|
@ -455,11 +455,16 @@ func readMboxMessagesBytes(b []byte) [][]byte {
|
||||||
_ = cur.WriteByte('\n')
|
_ = cur.WriteByte('\n')
|
||||||
}
|
}
|
||||||
if inMsg && cur.Len() > 0 {
|
if inMsg && cur.Len() > 0 {
|
||||||
msgs = append(msgs, bytes.TrimRight(cur.Bytes(), "\n"))
|
msgs = append(msgs, cloneTrimmedMessage(cur.Bytes()))
|
||||||
}
|
}
|
||||||
return msgs
|
return msgs
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func cloneTrimmedMessage(b []byte) []byte {
|
||||||
|
trimmed := bytes.TrimRight(b, "\n")
|
||||||
|
return append([]byte(nil), trimmed...)
|
||||||
|
}
|
||||||
|
|
||||||
func readMboxListFromIndex(path string) ([]MboxEntry, bool) {
|
func readMboxListFromIndex(path string) ([]MboxEntry, bool) {
|
||||||
accountID, folder, ok := mboxIndexContext(path)
|
accountID, folder, ok := mboxIndexContext(path)
|
||||||
if !ok {
|
if !ok {
|
||||||
|
|
|
||||||
|
|
@ -4,6 +4,7 @@ import (
|
||||||
"bytes"
|
"bytes"
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
"time"
|
"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: <one@example.com>",
|
||||||
|
"Subject: One",
|
||||||
|
"",
|
||||||
|
"short",
|
||||||
|
"From two@example.com Tue Jul 14 10:01:00 2026",
|
||||||
|
"Message-ID: <two@example.com>",
|
||||||
|
"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: <three@example.com>",
|
||||||
|
"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 {
|
func testRawMessage(id, subject string) RawMessage {
|
||||||
return RawMessage{
|
return RawMessage{
|
||||||
MessageID: id,
|
MessageID: id,
|
||||||
|
|
|
||||||
90
bug-mbox-reader.md
Normal file
90
bug-mbox-reader.md
Normal file
|
|
@ -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.
|
||||||
Loading…
Add table
Add a link
Reference in a new issue