email: drop the dead Gmail rule, fix nested MIME, decode windows-1251

The Gmail category rule matched X-GM-LABELS and X-Gmail-Labels against the parsed header block. Neither is a header. X-GM-LABELS is a Gmail FETCH data item and never appears in the message source, and X-Gmail-Labels only exists in a Takeout export, so the branch could not fire against a real mailbox while its doc comment promised a Promotions filter. Its test built the header by hand and therefore asserted the matcher rather than the plumbing. The rule is removed and the comment says what bringing it back would take. multipartText folded a nested multipart's answer into one string, so HTML derived text landed in the plain bucket and a real text/plain sibling later in the message was discarded by the guard on plain being set. The two buckets now stay separate through the recursion. windows-1251 returned an unsupported-charset error and the message degraded to subject only. That is the charset older Russian senders still use, so those mails could never produce a task candidate. It is decoded from a 128 entry table here rather than by vendoring x/text, for the body and for encoded words in the subject. Every other unknown charset still degrades to subject only. Found in review of #63. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TrVSBKe3RFDF4fGYKWYQnX
2026-08-01 14:01:04 +04:00
parent d69a1f8076
commit 6c81df17ec
7 changed files with 182 additions and 55 deletions
@@ -0,0 +1,48 @@
+package email
+
+// windows-1251 (and its ASCII-compatible low half) is decoded here rather than
+// pulled in from x/text.
+//
+// The alternative was returning an error for the charset, which ParseMessage
+// turns into a subject-only message. That is a live gap and not a small one:
+// cp1251 is still what older Russian senders emit, and subject-only means those
+// mails can never produce a task candidate. The whole of x/text/encoding is a
+// large dependency for the most privacy-sensitive path in the tree, and
+// windows-1251 is a 128-entry table.
+//
+// Only cp1251 is added. Guessing at an unknown charset stays forbidden: mojibake
+// is worse than nothing, because the model extracts a task from it happily.
+
+// cp1251High — the 0x80..0xFF half of windows-1251. The low half is ASCII.
+var cp1251High = [128]rune{
+	0x0402, 0x0403, 0x201A, 0x0453, 0x201E, 0x2026, 0x2020, 0x2021,
+	0x20AC, 0x2030, 0x0409, 0x2039, 0x040A, 0x040C, 0x040B, 0x040F,
+	0x0452, 0x2018, 0x2019, 0x201C, 0x201D, 0x2022, 0x2013, 0x2014,
+	0xFFFD, 0x2122, 0x0459, 0x203A, 0x045A, 0x045C, 0x045B, 0x045F,
+	0x00A0, 0x040E, 0x045E, 0x0408, 0x00A4, 0x0490, 0x00A6, 0x00A7,
+	0x0401, 0x00A9, 0x0404, 0x00AB, 0x00AC, 0x00AD, 0x00AE, 0x0407,
+	0x00B0, 0x00B1, 0x0406, 0x0456, 0x0491, 0x00B5, 0x00B6, 0x00B7,
+	0x0451, 0x2116, 0x0454, 0x00BB, 0x0458, 0x0405, 0x0455, 0x0457,
+	0x0410, 0x0411, 0x0412, 0x0413, 0x0414, 0x0415, 0x0416, 0x0417,
+	0x0418, 0x0419, 0x041A, 0x041B, 0x041C, 0x041D, 0x041E, 0x041F,
+	0x0420, 0x0421, 0x0422, 0x0423, 0x0424, 0x0425, 0x0426, 0x0427,
+	0x0428, 0x0429, 0x042A, 0x042B, 0x042C, 0x042D, 0x042E, 0x042F,
+	0x0430, 0x0431, 0x0432, 0x0433, 0x0434, 0x0435, 0x0436, 0x0437,
+	0x0438, 0x0439, 0x043A, 0x043B, 0x043C, 0x043D, 0x043E, 0x043F,
+	0x0440, 0x0441, 0x0442, 0x0443, 0x0444, 0x0445, 0x0446, 0x0447,
+	0x0448, 0x0449, 0x044A, 0x044B, 0x044C, 0x044D, 0x044E, 0x044F,
+}
+
+// decodeCP1251 maps each byte through the table. Every byte has a defined
+// meaning in this charset, so decoding cannot fail.
+func decodeCP1251(b []byte) string {
+	out := make([]rune, 0, len(b))
+	for _, c := range b {
+		if c < 0x80 {
+			out = append(out, rune(c))
+			continue
+		}
+		out = append(out, cp1251High[c-0x80])
+	}
+	return string(out)
+}
@@ -21,8 +21,18 @@ import (
 //   - Auto-Submitted other than "no" (RFC 3834) — generated by a machine.
 //   - X-Spam-Flag: YES, X-Spam-Status: Yes — the spam filter upstream already
 //     decided; we do not second-guess it in the other direction.
-//   - X-GM-LABELS / X-Gmail-Labels containing a Gmail category — Gmail's own
-//     Promotions/Social/Forums/Spam classification, when the server sends it.
+//
+// There is deliberately NO Gmail-category rule. One was written and removed:
+// it matched X-GM-LABELS and X-Gmail-Labels against the parsed header block,
+// and neither is a header. X-GM-LABELS is a Gmail FETCH data item, requested as
+// "UID FETCH n (X-GM-LABELS)" and never present in the message source;
+// X-Gmail-Labels only exists in a Takeout mbox export. This client asks for
+// BODY.PEEK[] and nothing else, so the rule could not fire against a real
+// mailbox while its doc comment promised a Promotions filter. Gmail's promotion
+// mail carries List-Unsubscribe in practice and is caught by the rule above.
+// Bringing the category rule back means adding the FETCH item and carrying the
+// labels into classifyJunk out of band, not matching a header that never
+// arrives.
 //
 // Deliberately NOT here: sender allow/deny lists and subject keyword matching.
 // Both are configuration that ages badly and both would be a place for his
@@ -35,16 +45,6 @@ import (
 // junkHeaders — headers whose mere presence marks bulk mail.
 var junkPresence = []string{"List-Unsubscribe", "List-Id", "List-Post"}

-// gmailCategories — Gmail's category labels, lowercased as they appear in
-// X-GM-LABELS. "important" and "inbox" are labels too, and are NOT categories.
-// Matching is by these exact tokens (substring is fine — they are namespaced
-// and cannot appear in a hand-made label by accident), so a user label named
-// "Social Club" is not mistaken for Gmail's Social category.
-var gmailCategories = []string{
-	"category_promotions", "category_social", "category_forums", "category_updates",
-	`\spam`, `\junk`,
-}
-
 // classifyJunk returns whether the message is bulk/automated and why. The
 // reason is a short header name, safe to log — it names the marker, never the
 // sender or the subject.
@@ -67,14 +67,5 @@ func classifyJunk(h mail.Header) (bool, string) {
 	if v := strings.ToLower(strings.TrimSpace(h.Get("X-Spam-Status"))); strings.HasPrefix(v, "yes") {
 		return true, "x-spam-status"
 	}
-	labels := strings.ToLower(h.Get("X-GM-LABELS") + " " + h.Get("X-Gmail-Labels"))
-	for _, c := range gmailCategories {
-		if c == "" {
-			continue
-		}
-		if strings.Contains(labels, c) {
-			return true, "gmail-category"
-		}
-	}
 	return false, ""
 }
@@ -31,8 +31,10 @@ func TestClassifyJunk(t *testing.T) {
 		{"spam flag", "From: a@b.c\nX-Spam-Flag: YES", true, "x-spam-flag"},
 		{"spam status", "From: a@b.c\nX-Spam-Status: Yes, score=9.1", true, "x-spam-status"},
 		{"spam status no", "From: a@b.c\nX-Spam-Status: No, score=0.1", false, ""},
-		{"gmail promo", "From: a@b.c\nX-Gmail-Labels: Inbox,CATEGORY_PROMOTIONS", true, "gmail-category"},
-		{"user label", "From: a@b.c\nX-Gmail-Labels: Social Club,Important", false, ""},
+		// X-GM-LABELS is a Gmail FETCH data item, not a header, so it never
+		// reaches classifyJunk through this client. The rule that matched it was
+		// removed rather than left claiming a Promotions filter that never ran.
+		{"gmail label header is not a rule", "From: a@b.c\nX-Gmail-Labels: Inbox,CATEGORY_PROMOTIONS", false, ""},
 	}
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
@@ -94,7 +94,14 @@ func plaintextBody(contentType, encoding string, body io.Reader) (string, error)
 		if boundary == "" {
 			return "", fmt.Errorf("email: multipart without boundary")
 		}
-		return multipartText(multipart.NewReader(body, boundary))
+		plain, html, err := multipartText(multipart.NewReader(body, boundary))
+		if err != nil {
+			return "", err
+		}
+		if strings.TrimSpace(plain) != "" {
+			return plain, nil
+		}
+		return html, nil
 	case mediaType == "text/html":
 		raw, err := decodeBody(body, encoding, params["charset"])
 		if err != nil {
@@ -109,10 +116,15 @@ func plaintextBody(contentType, encoding string, body io.Reader) (string, error)
 	}
 }

-// multipartText reads one multipart level, recursing into nested multiparts.
-// Returns the plain part if any part yielded one, else the stripped HTML.
-func multipartText(mr *multipart.Reader) (string, error) {
-	var plain, html string
+// multipartText reads one multipart level, recursing into nested multiparts,
+// and returns the two buckets separately: real text/plain, and text stripped
+// out of HTML.
+//
+// The buckets stay separate all the way up because a nested multipart can
+// contribute either kind. Folding a nested level's answer into one string put
+// HTML-derived text in the plain bucket, and a real text/plain sibling later in
+// the message was then thrown away by the "plain is already set" guard.
+func multipartText(mr *multipart.Reader) (plain, html string, err error) {
 	for {
 		part, err := mr.NextPart()
 		if err == io.EOF {
@@ -127,35 +139,47 @@ func multipartText(mr *multipart.Reader) (string, error) {
 			continue // attachment
 		}
 		ct := part.Header.Get("Content-Type")
-		mediaType, _, _ := mime.ParseMediaType(ct)
-		text, err := plaintextBody(ct, part.Header.Get("Content-Transfer-Encoding"), part)
-		part.Close()
-		if err != nil || strings.TrimSpace(text) == "" {
-			continue
-		}
-		if mediaType == "text/html" && !strings.HasPrefix(mediaType, "multipart/") {
-			if html == "" {
-				html = text
+		mediaType, params, _ := mime.ParseMediaType(ct)
+		switch {
+		case strings.HasPrefix(mediaType, "multipart/"):
+			var np, nh string
+			if b := params["boundary"]; b != "" {
+				np, nh, _ = multipartText(multipart.NewReader(part, b))
+			}
+			part.Close()
+			if plain == "" {
+				plain = np
+			}
+			if html == "" {
+				html = nh
+			}
+		default:
+			text, terr := plaintextBody(ct, part.Header.Get("Content-Transfer-Encoding"), part)
+			part.Close()
+			if terr != nil || strings.TrimSpace(text) == "" {
+				continue
+			}
+			if mediaType == "text/html" {
+				if html == "" {
+					html = text
+				}
+				continue
+			}
+			if plain == "" {
+				plain = text
 			}
-			continue
-		}
-		if plain == "" {
-			plain = text
 		}
 	}
-	if strings.TrimSpace(plain) != "" {
-		return plain, nil
-	}
-	return html, nil
+	return plain, html, nil
 }

 // decodeBody applies the transfer encoding, then the charset.
 //
-// Charset support is UTF-8 (and ASCII, its subset) only, on purpose: x/text's
-// encoding tables are not vendored here, and guessing at windows-1251 bytes
-// would feed the model mojibake it would happily extract a task from. An
-// unsupported charset returns an error, which ParseMessage turns into an empty
-// body — subject-only, which is honest.
+// Charset support is UTF-8 (and ASCII, its subset) plus windows-1251, which is
+// decoded from a table in charset.go — see the reasoning there. Everything else
+// returns an error, which ParseMessage turns into an empty body: subject-only,
+// which is honest. Guessing at unknown bytes would feed the model mojibake it
+// would happily extract a task from.
 func decodeBody(r io.Reader, encoding, charset string) (string, error) {
 	switch strings.ToLower(strings.TrimSpace(encoding)) {
 	case "quoted-printable":
@@ -170,6 +194,8 @@ func decodeBody(r io.Reader, encoding, charset string) (string, error) {
 	switch cs := strings.ToLower(strings.TrimSpace(charset)); cs {
 	case "", "utf-8", "utf8", "us-ascii", "ascii":
 		return string(b), nil
+	case "windows-1251", "cp1251", "windows1251", "x-cp1251":
+		return decodeCP1251(b), nil
 	default:
 		return "", fmt.Errorf("email: unsupported charset %q", cs)
 	}
@@ -181,6 +207,19 @@ func decodeBody(r io.Reader, encoding, charset string) (string, error) {
 // shown to him as evidence.
 func decodeHeader(v string) string {
 	dec := new(mime.WordDecoder)
+	// Same charset support as the body: an old Russian sender encodes the
+	// subject in windows-1251 too, and a subject is often the whole task.
+	dec.CharsetReader = func(charset string, r io.Reader) (io.Reader, error) {
+		switch strings.ToLower(strings.TrimSpace(charset)) {
+		case "windows-1251", "cp1251", "windows1251", "x-cp1251":
+			b, err := io.ReadAll(io.LimitReader(r, 1<<16))
+			if err != nil && len(b) == 0 {
+				return nil, err
+			}
+			return strings.NewReader(decodeCP1251(b)), nil
+		}
+		return nil, fmt.Errorf("email: unsupported charset %q", charset)
+	}
 	out, err := dec.DecodeHeader(v)
 	if err != nil {
 		return collapse(v)
@@ -70,13 +70,28 @@ func TestParsePrefersPlainAndSkipsAttachments(t *testing.T) {
 	}
 }

-// An unsupported charset must degrade to headers-only rather than to mojibake
-// the model would then extract a task from.
-func TestParseUnsupportedCharsetKeepsHeaders(t *testing.T) {
+// windows-1251 is what older Russian senders still emit. Subject-only for those
+// mails meant they could never produce a task candidate.
+func TestParseCP1251(t *testing.T) {
 	msg, err := ParseMessage(3, fixture(t, "cp1251.eml"))
 	if err != nil {
 		t.Fatalf("parse: %v", err)
 	}
+	if want := "Счёт за интернет"; msg.Subject != want {
+		t.Errorf("subject = %q, want %q", msg.Subject, want)
+	}
+	if want := "Оплати счёт до пятницы."; !strings.Contains(msg.Body, want) {
+		t.Errorf("body = %q, want it to contain %q", msg.Body, want)
+	}
+}
+
+// A charset with no table here must degrade to headers-only rather than to
+// mojibake the model would then extract a task from.
+func TestParseUnsupportedCharsetKeepsHeaders(t *testing.T) {
+	msg, err := ParseMessage(3, fixture(t, "koi8r.eml"))
+	if err != nil {
+		t.Fatalf("parse: %v", err)
+	}
 	if msg.Subject != "Legacy" {
 		t.Errorf("subject = %q, want Legacy", msg.Subject)
 	}
@@ -85,6 +100,31 @@ func TestParseUnsupportedCharsetKeepsHeaders(t *testing.T) {
 	}
 }

+// A nested multipart/alternative that only had HTML must not fill the plain
+// bucket: a real text/plain sibling later in the message is the better text and
+// used to be discarded.
+func TestParseNestedHTMLDoesNotShadowLaterPlain(t *testing.T) {
+	raw := "Subject: nested\r\n" +
+		"Content-Type: multipart/mixed; boundary=OUT\r\n\r\n" +
+		"--OUT\r\n" +
+		"Content-Type: multipart/alternative; boundary=IN\r\n\r\n" +
+		"--IN\r\n" +
+		"Content-Type: text/html; charset=utf-8\r\n\r\n" +
+		"<p>from the html part</p>\r\n" +
+		"--IN--\r\n" +
+		"--OUT\r\n" +
+		"Content-Type: text/plain; charset=utf-8\r\n\r\n" +
+		"the real plain text\r\n" +
+		"--OUT--\r\n"
+	msg, err := ParseMessage(5, []byte(raw))
+	if err != nil {
+		t.Fatalf("parse: %v", err)
+	}
+	if got := strings.TrimSpace(msg.Body); got != "the real plain text" {
+		t.Errorf("body = %q, want the text/plain part to win", got)
+	}
+}
+
 func TestParseTruncatesLongBody(t *testing.T) {
 	var b strings.Builder
 	b.WriteString("Subject: long\r\nContent-Type: text/plain; charset=utf-8\r\n\r\n")
@@ -1,7 +1,7 @@
 From: legacy@example.org
 To: kami@example.org
-Subject: Legacy
+Subject: =?windows-1251?B?0fe48iDn4CDo7fLl8O3l8g==?=
 Date: Fri, 01 Aug 2026 05:00:00 +0400
 Content-Type: text/plain; charset="windows-1251"

-Ï
+Îïëàòè ñ÷¸ò äî ïÿòíèöû.
@@ -0,0 +1,7 @@
+From: legacy@example.org
+To: kami@example.org
+Subject: Legacy
+Date: Fri, 01 Aug 2026 05:00:00 +0400
+Content-Type: text/plain; charset="koi8-r"
+
+ïÐÌÁÔÉ ÓÞ£Ô.