From 7dba1b7935a185f73df28326e99d6bca41e430a8 Mon Sep 17 00:00:00 2001 From: claude Date: Thu, 6 Aug 2026 03:23:26 +0400 Subject: [PATCH 1/2] calendar: read a wall clock as a wall clock, and unfold iCal (V-581) FactSpan built both instants by adding a duration to local midnight, so on the two DST changeover days every span was an hour off. A day is 23 or 25 hours wide there, and the busy gate then read a 14:00 meeting as 13:00 or 15:00. Both readings are time.Date now, and the midnight crossing is AddDate rather than a 24-hour add. The iCal parse did not unfold content lines. A server folds a property at 75 octets and a Russian summary is two bytes a letter, so the tail of an ordinary weekly standup was read as an unknown property and dropped, and the event was filed under a truncated name. RFC 5545 TEXT escapes are also reversed now, which RenderICal has always written and the parse never undid. Two regression tests: a folded and escaped summary, and a span across the start of DST in Europe/Berlin. Co-Authored-By: Claude Opus 5 --- internal/calendar/calendar.go | 13 ++++++-- internal/calendar/ical.go | 48 +++++++++++++++++++++++++-- internal/calendar/ical_render_test.go | 38 +++++++++++++++++++++ 3 files changed, 93 insertions(+), 6 deletions(-) diff --git a/internal/calendar/calendar.go b/internal/calendar/calendar.go index 40bf11b..d32a82a 100644 --- a/internal/calendar/calendar.go +++ b/internal/calendar/calendar.go @@ -140,6 +140,12 @@ const EventKeyPrefix = "calendar_event_" // // An end at or before the start is read as crossing midnight, so a 23:30-00:15 // meeting covers the quarter hour it actually covers. +// +// Both readings are built with time.Date rather than added to midnight as a +// duration. A day is 23 or 25 hours wide on the two DST changeovers, so +// midnight plus fourteen hours is 13:00 or 15:00 on those days, and the busy +// gate would then read a 14:00 meeting an hour off. The same goes for the +// midnight crossing, which is AddDate and not a 24-hour add. func FactSpan(key, value string, loc *time.Location) (start, end time.Time, ok bool) { if !strings.HasPrefix(key, EventKeyPrefix) { return time.Time{}, time.Time{}, false @@ -172,10 +178,11 @@ func FactSpan(key, value string, loc *time.Location) (start, end time.Time, ok b if !ok1 || !ok2 { return time.Time{}, time.Time{}, false } - start = day.Add(time.Duration(sh)*time.Hour + time.Duration(sm)*time.Minute) - end = day.Add(time.Duration(eh)*time.Hour + time.Duration(em)*time.Minute) + y, mo, d := day.Date() + start = time.Date(y, mo, d, sh, sm, 0, 0, loc) + end = time.Date(y, mo, d, eh, em, 0, 0, loc) if !end.After(start) { - end = end.Add(24 * time.Hour) + end = end.AddDate(0, 0, 1) } return start, end, true } diff --git a/internal/calendar/ical.go b/internal/calendar/ical.go index 1aa40de..3800d68 100644 --- a/internal/calendar/ical.go +++ b/internal/calendar/ical.go @@ -66,7 +66,7 @@ func ParseICalDay(body []byte, now time.Time) []Event { // Reports false for all-day events and parse failures. func parseVEVENT(block string, loc *time.Location) (Event, bool) { var e Event - for _, line := range strings.Split(block, "\n") { + for _, line := range strings.Split(unfold(block), "\n") { line = strings.TrimSpace(line) switch { case strings.HasPrefix(line, "DTSTART"): @@ -78,9 +78,9 @@ func parseVEVENT(block string, loc *time.Location) (Event, bool) { e.End = t } case strings.HasPrefix(line, "SUMMARY"): - e.Summary = afterColon(line) + e.Summary = unescapeText(afterColon(line)) case strings.HasPrefix(line, "UID"): - e.UID = afterColon(line) + e.UID = unescapeText(afterColon(line)) } } if e.Start.IsZero() || e.End.IsZero() { @@ -89,6 +89,48 @@ func parseVEVENT(block string, loc *time.Location) (Event, bool) { return e, true } +// unfold undoes RFC 5545 content-line folding, where a long property is split +// with a CRLF and the continuation begins with one space or tab. +// +// It runs before the block is split into lines, because splitting first and +// trimming each line destroys the leading space that marks a continuation. A +// server folds at 75 octets and a Russian summary is two bytes a letter, so +// "Еженедельная планёрка с командой" crosses the limit easily — without this +// the tail of the summary was read as an unknown property and dropped, and the +// event was filed under a truncated name. +func unfold(block string) string { + if !strings.Contains(block, "\n ") && !strings.Contains(block, "\n\t") { + return block + } + return strings.NewReplacer("\r\n ", "", "\r\n\t", "", "\n ", "", "\n\t", "").Replace(block) +} + +// unescapeText reverses the RFC 5545 TEXT escaping escapeText applies. Without +// it a summary a server wrote as "Обед\, потом созвон" reaches the day plan +// with the backslash still in it, and FactKey folds that literal into the key. +func unescapeText(s string) string { + if !strings.Contains(s, `\`) { + return s + } + var b strings.Builder + b.Grow(len(s)) + for i := 0; i < len(s); i++ { + if s[i] != '\\' || i+1 >= len(s) { + b.WriteByte(s[i]) + continue + } + i++ + switch s[i] { + case 'n', 'N': + b.WriteByte('\n') + default: + // ";", ",", "\\" and anything else a writer escaped needlessly. + b.WriteByte(s[i]) + } + } + return b.String() +} + func afterColon(line string) string { if i := strings.Index(line, ":"); i >= 0 { return strings.TrimSpace(line[i+1:]) diff --git a/internal/calendar/ical_render_test.go b/internal/calendar/ical_render_test.go index 6ca89de..45c4c46 100644 --- a/internal/calendar/ical_render_test.go +++ b/internal/calendar/ical_render_test.go @@ -61,6 +61,44 @@ func TestRenderICalEscapesInjection(t *testing.T) { } } +// A folded SUMMARY is one property, not a property plus a dropped tail. Servers +// fold at 75 octets and a Russian summary is two bytes a letter. +func TestParseICalUnfoldsAndUnescapes(t *testing.T) { + body := []byte("BEGIN:VEVENT\r\n" + + "UID:u1\r\n" + + "DTSTART:20260703T130000Z\r\n" + + "DTEND:20260703T140000Z\r\n" + + "SUMMARY:Еженедельная планёрка\\, потом\r\n созвон\r\n" + + "END:VEVENT\r\n") + from := time.Date(2026, 7, 3, 0, 0, 0, 0, time.UTC) + events := ParseICal(body, from, from.AddDate(0, 0, 1)) + if len(events) != 1 { + t.Fatalf("got %d events, want 1", len(events)) + } + if want := "Еженедельная планёрка, потом созвон"; events[0].Summary != want { + t.Errorf("Summary = %q, want %q", events[0].Summary, want) + } +} + +// A day is 23 hours wide where DST starts, so a wall clock reading has to be +// built with time.Date and never as midnight plus a duration. +func TestFactSpanAcrossDSTStart(t *testing.T) { + loc, err := time.LoadLocation("Europe/Berlin") + if err != nil { + t.Skipf("no tzdata for Europe/Berlin: %v", err) + } + start, end, ok := FactSpan("calendar_event_20260329_Planerka", "Planerka @ 14:00-15:00", loc) + if !ok { + t.Fatal("FactSpan reported not ok") + } + if start.Hour() != 14 || start.Minute() != 0 { + t.Errorf("start = %s, want a 14:00 wall clock", start) + } + if end.Hour() != 15 { + t.Errorf("end = %s, want a 15:00 wall clock", end) + } +} + func TestReminderEventEmptyPayload(t *testing.T) { e := ReminderEvent(3, time.Date(2026, 8, 1, 9, 0, 0, 0, time.UTC), " ", 0) if e.Summary != "напоминание" { From 4f96bbd6ec30f6c0f542585e778eace678b6ae50 Mon Sep 17 00:00:00 2001 From: claude Date: Thu, 6 Aug 2026 03:23:36 +0400 Subject: [PATCH 2/2] email: bound the MIME walk and drop two copies of every body (V-581) The MIME tree walk had no depth limit, and the nesting comes off the wire. A boundary line is a few bytes, so one message inside MaxMessageBytes can declare tens of thousands of multipart levels and pick the recursion depth of a daemon reading his mail. MaxMIMEDepth stops the walk at 12, well past the three levels real mail uses, and the headers still come through. ParseMessage converted the raw message to a string to read it, which copied up to 2 MiB per mail on a box already holding the resident model. It reads the bytes directly now. decodeCP1251 collected runes and then copied them into a string, four bytes a character for the whole body, and writes into a Builder instead. No behaviour change to what is read: EXAMINE and BODY.PEEK are still the only mailbox commands, and no credential reaches a log line. Co-Authored-By: Claude Opus 5 --- internal/email/charset.go | 15 +++++++++++---- internal/email/message.go | 33 +++++++++++++++++++++++++++------ internal/email/message_test.go | 19 +++++++++++++++++++ 3 files changed, 57 insertions(+), 10 deletions(-) diff --git a/internal/email/charset.go b/internal/email/charset.go index f70718a..63c2b55 100644 --- a/internal/email/charset.go +++ b/internal/email/charset.go @@ -1,5 +1,7 @@ package email +import "strings" + // windows-1251 (and its ASCII-compatible low half) is decoded here rather than // pulled in from x/text. // @@ -35,14 +37,19 @@ var cp1251High = [128]rune{ // decodeCP1251 maps each byte through the table. Every byte has a defined // meaning in this charset, so decoding cannot fail. +// +// It writes into a Builder rather than collecting runes: a []rune of the whole +// body is four bytes a character and was then copied again into the string, so +// a 1 MiB cp1251 mail allocated about 6 MiB to produce roughly 2. func decodeCP1251(b []byte) string { - out := make([]rune, 0, len(b)) + var out strings.Builder + out.Grow(len(b)) for _, c := range b { if c < 0x80 { - out = append(out, rune(c)) + out.WriteByte(c) continue } - out = append(out, cp1251High[c-0x80]) + out.WriteRune(cp1251High[c-0x80]) } - return string(out) + return out.String() } diff --git a/internal/email/message.go b/internal/email/message.go index 5219d39..ffb1c6f 100644 --- a/internal/email/message.go +++ b/internal/email/message.go @@ -18,6 +18,7 @@ package email import ( + "bytes" "encoding/base64" "fmt" "io" @@ -57,7 +58,10 @@ type Message struct { // through, because a subject line alone is often the whole task ("Счёт за // интернет"). Only a message whose headers cannot be read at all is an error. func ParseMessage(uid uint32, raw []byte) (Message, error) { - m, err := mail.ReadMessage(strings.NewReader(string(raw))) + // bytes.NewReader, not strings.NewReader(string(raw)): the conversion copied + // the whole message, and MaxMessageBytes lets that be 2 MiB per mail on a box + // already holding the resident model. + m, err := mail.ReadMessage(bytes.NewReader(raw)) if err != nil { return Message{}, fmt.Errorf("email: parse message: %w", err) } @@ -82,6 +86,23 @@ func ParseMessage(uid uint32, raw []byte) (Message, error) { // wholesale — an attachment is a file, not a sentence, and reading one would // mean parsing arbitrary formats from the network. func plaintextBody(contentType, encoding string, body io.Reader) (string, error) { + return plaintextBodyAt(contentType, encoding, body, 0) +} + +// MaxMIMEDepth — how deep the MIME tree is walked. +// +// The nesting comes off the wire, so the recursion depth is the sender's to +// pick: a boundary line is a few bytes, and one message inside MaxMessageBytes +// can declare tens of thousands of multipart levels. Real mail is three deep +// (mixed, then alternative, then related), so a message past this is malformed +// or hostile and truncating the walk costs a body nobody was going to read. +const MaxMIMEDepth = 12 + +// plaintextBodyAt is plaintextBody carrying the current nesting depth. +func plaintextBodyAt(contentType, encoding string, body io.Reader, depth int) (string, error) { + if depth > MaxMIMEDepth { + return "", nil + } mediaType, params, err := mime.ParseMediaType(contentType) if contentType == "" || err != nil { // No Content-Type at all is legal and means text/plain; a broken one is @@ -94,7 +115,7 @@ func plaintextBody(contentType, encoding string, body io.Reader) (string, error) if boundary == "" { return "", fmt.Errorf("email: multipart without boundary") } - plain, html, err := multipartText(multipart.NewReader(body, boundary)) + plain, html, err := multipartText(multipart.NewReader(body, boundary), depth+1) if err != nil { return "", err } @@ -124,7 +145,7 @@ func plaintextBody(contentType, encoding string, body io.Reader) (string, error) // 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) { +func multipartText(mr *multipart.Reader, depth int) (plain, html string, err error) { for { part, err := mr.NextPart() if err == io.EOF { @@ -143,8 +164,8 @@ func multipartText(mr *multipart.Reader) (plain, html string, err error) { switch { case strings.HasPrefix(mediaType, "multipart/"): var np, nh string - if b := params["boundary"]; b != "" { - np, nh, _ = multipartText(multipart.NewReader(part, b)) + if b := params["boundary"]; b != "" && depth <= MaxMIMEDepth { + np, nh, _ = multipartText(multipart.NewReader(part, b), depth+1) } part.Close() if plain == "" { @@ -154,7 +175,7 @@ func multipartText(mr *multipart.Reader) (plain, html string, err error) { html = nh } default: - text, terr := plaintextBody(ct, part.Header.Get("Content-Transfer-Encoding"), part) + text, terr := plaintextBodyAt(ct, part.Header.Get("Content-Transfer-Encoding"), part, depth) part.Close() if terr != nil || strings.TrimSpace(text) == "" { continue diff --git a/internal/email/message_test.go b/internal/email/message_test.go index c961dce..ec11d14 100644 --- a/internal/email/message_test.go +++ b/internal/email/message_test.go @@ -1,6 +1,7 @@ package email import ( + "fmt" "os" "path/filepath" "strings" @@ -143,6 +144,24 @@ func TestParseTruncatesLongBody(t *testing.T) { } } +// Nesting depth comes off the wire, so a hostile message must not get to pick +// the recursion depth. The walk stops and the headers still come through. +func TestParseMessageBoundsMIMEDepth(t *testing.T) { + var b strings.Builder + b.WriteString("Subject: deep\r\nMIME-Version: 1.0\r\n") + for i := 0; i < MaxMIMEDepth+20; i++ { + fmt.Fprintf(&b, "Content-Type: multipart/mixed; boundary=\"b%d\"\r\n\r\n--b%d\r\n", i, i) + } + b.WriteString("Content-Type: text/plain\r\n\r\nглубоко\r\n") + msg, err := ParseMessage(7, []byte(b.String())) + if err != nil { + t.Fatalf("ParseMessage: %v", err) + } + if msg.Subject != "deep" { + t.Errorf("Subject = %q, want the headers to survive", msg.Subject) + } +} + func TestCollapseSqueezesBlankLines(t *testing.T) { got := collapse(" a b \r\n\r\n\r\n\r\n c \r\n") if got != "a b\n\nc" {