diff --git a/pkg/notifications/mail_render.go b/pkg/notifications/mail_render.go index 7749e1d9c..e92735dbc 100644 --- a/pkg/notifications/mail_render.go +++ b/pkg/notifications/mail_render.go @@ -17,11 +17,14 @@ package notifications import ( + "bufio" "bytes" "embed" + "html" templatehtml "html/template" "net/url" "regexp" + "strconv" "strings" templatetext "text/template" @@ -32,6 +35,9 @@ import ( "github.com/microcosm-cc/bluemonday" "github.com/yuin/goldmark" + "github.com/yuin/goldmark/ast" + goldmarkhtml "github.com/yuin/goldmark/renderer/html" + "github.com/yuin/goldmark/text" ) const mailTemplatePlain = ` @@ -290,12 +296,183 @@ func ensurePMargins(html string) string { return rePTag.ReplaceAllString(html, "

") } -// convertLinesToPlain converts mail lines to plain text, stripping HTML from lines marked as HTML. +var markdownTextWriter = goldmarkhtml.NewWriter() + +func markdownToPlainText(markdown string) string { + source := []byte(markdown) + document := goldmark.DefaultParser().Parse(text.NewReader(source)) + var plain strings.Builder + linkStarts := make(map[ast.Node]int) + listItemIndents := make([]int, 0) + listItemHasBlocks := make([]bool, 0) + + _ = ast.Walk(document, func(node ast.Node, entering bool) (ast.WalkStatus, error) { + switch n := node.(type) { + case *ast.Text: + if !entering { + return ast.WalkContinue, nil + } + writeMarkdownText(&plain, n.Value(source), n.IsRaw()) + if n.SoftLineBreak() || n.HardLineBreak() { + plain.WriteByte('\n') + if len(listItemIndents) > 0 { + plain.WriteString(strings.Repeat(" ", listItemIndents[len(listItemIndents)-1])) + } + } + case *ast.String: + if entering { + writeMarkdownText(&plain, n.Value, n.IsRaw() || n.IsCode()) + } + case *ast.AutoLink: + if entering { + plain.Write(n.Label(source)) + } + case *ast.Link: + if entering { + linkStarts[node] = plain.Len() + return ast.WalkContinue, nil + } + start := linkStarts[node] + label := plain.String()[start:] + var normalized strings.Builder + writeMarkdownText(&normalized, n.Destination, false) + destination := normalized.String() + if destination != "" && label != destination { + plain.WriteString(" (") + plain.WriteString(destination) + plain.WriteByte(')') + } + delete(linkStarts, node) + case *ast.Image: + if !entering { + if len(n.Destination) > 0 { + plain.WriteString(" (") + writeMarkdownText(&plain, n.Destination, false) + plain.WriteByte(')') + } + } + case *ast.ListItem: + if entering { + if len(listItemHasBlocks) > 0 { + listItemHasBlocks[len(listItemHasBlocks)-1] = true + } + listItemIndents = append(listItemIndents, writePlainListItem(&plain, n)) + listItemHasBlocks = append(listItemHasBlocks, false) + } else { + listItemIndents = listItemIndents[:len(listItemIndents)-1] + listItemHasBlocks = listItemHasBlocks[:len(listItemHasBlocks)-1] + writePlainNewline(&plain) + } + case *ast.Paragraph, *ast.Heading: + if entering { + writePlainListBlockStart(&plain, listItemIndents, listItemHasBlocks) + } else { + writePlainNewline(&plain) + } + case *ast.CodeBlock, *ast.FencedCodeBlock: + if entering { + writePlainListBlockStart(&plain, listItemIndents, listItemHasBlocks) + writePlainBlock(&plain, node.Lines().Value(source), listItemIndents) + writePlainNewline(&plain) + return ast.WalkSkipChildren, nil + } + case *ast.ThematicBreak: + if entering { + writePlainListBlockStart(&plain, listItemIndents, listItemHasBlocks) + plain.WriteString("---\n") + } + case *ast.RawHTML, *ast.HTMLBlock: + if entering { + return ast.WalkSkipChildren, nil + } + } + + return ast.WalkContinue, nil + }) + + return strings.TrimSpace(plain.String()) +} + +func writePlainListItem(plain *strings.Builder, item *ast.ListItem) int { + writePlainNewline(plain) + prefixStart := plain.Len() + list := item.Parent().(*ast.List) + depth := 0 + for parent := list.Parent(); parent != nil; parent = parent.Parent() { + if _, nested := parent.(*ast.List); nested { + depth++ + } + } + plain.WriteString(strings.Repeat(" ", depth)) + + if list.IsOrdered() { + position := list.Start + for sibling := item.PreviousSibling(); sibling != nil; sibling = sibling.PreviousSibling() { + position++ + } + plain.WriteString(strconv.Itoa(position)) + plain.WriteString(". ") + } else { + plain.WriteString("- ") + } + + return plain.Len() - prefixStart +} + +func writePlainListBlockStart(plain *strings.Builder, indents []int, hasBlocks []bool) { + if len(hasBlocks) == 0 { + return + } + + current := len(hasBlocks) - 1 + if hasBlocks[current] { + writePlainNewline(plain) + plain.WriteString(strings.Repeat(" ", indents[current])) + } + hasBlocks[current] = true +} + +func writePlainBlock(plain *strings.Builder, value []byte, indents []int) { + indent := 0 + if len(indents) > 0 { + indent = indents[len(indents)-1] + } + + for i, char := range value { + plain.WriteByte(char) + if char == '\n' && i < len(value)-1 { + plain.WriteString(strings.Repeat(" ", indent)) + } + } +} + +func writeMarkdownText(plain *strings.Builder, value []byte, raw bool) { + if raw { + plain.Write(value) + return + } + + var escaped bytes.Buffer + writer := bufio.NewWriter(&escaped) + markdownTextWriter.Write(writer, value) + _ = writer.Flush() + plain.WriteString(html.UnescapeString(escaped.String())) +} + +func writePlainNewline(plain *strings.Builder) { + if plain.Len() == 0 || plain.String()[plain.Len()-1] != '\n' { + plain.WriteByte('\n') + } +} + func convertLinesToPlain(lines []*mailLine) []*mailLine { plain := make([]*mailLine, 0, len(lines)) for _, line := range lines { if !line.isHTML { - plain = append(plain, line) + text := markdownToPlainText(line.Text) + if text != "" { + plain = append(plain, &mailLine{Text: text}) + } continue } @@ -352,20 +529,15 @@ func RenderMail(m *Mail, lang string) (mailOpts *mail.Opts, err error) { data := make(map[string]interface{}) data["Greeting"] = m.greeting - if m.conversational { - data["IntroLines"] = convertLinesToPlain(m.introLines) - data["OutroLines"] = convertLinesToPlain(m.outroLines) - if m.headerLine != nil { - plainHeaders := convertLinesToPlain([]*mailLine{m.headerLine}) - if len(plainHeaders) > 0 { - data["HeaderLinePlain"] = plainHeaders[0].Text - } + data["IntroLines"] = convertLinesToPlain(m.introLines) + data["OutroLines"] = convertLinesToPlain(m.outroLines) + if m.conversational && m.headerLine != nil { + plainHeaders := convertLinesToPlain([]*mailLine{m.headerLine}) + if len(plainHeaders) > 0 { + data["HeaderLinePlain"] = plainHeaders[0].Text } - } else { - data["IntroLines"] = m.introLines - data["OutroLines"] = m.outroLines } - data["FooterLines"] = m.footerLines + data["FooterLines"] = convertLinesToPlain(m.footerLines) data["ActionText"] = m.actionText data["ActionURL"] = m.actionURL data["Boundary"] = boundary diff --git a/pkg/notifications/mail_test.go b/pkg/notifications/mail_test.go index d8f5db2e4..289c3cf71 100644 --- a/pkg/notifications/mail_test.go +++ b/pkg/notifications/mail_test.go @@ -20,6 +20,7 @@ import ( "strings" "testing" + "code.vikunja.io/api/pkg/config" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -163,7 +164,7 @@ Hi there, This is a line -This **line** contains [a link](https://vikunja.io) +This line contains a link (https://vikunja.io) And another one @@ -230,6 +231,17 @@ This is a footer line // Verify no action button assert.NotContains(t, mailopts.HTMLMessage, `class="email-button"`) }) + t.Run("with link to notification settings in footer", func(t *testing.T) { + originalPublicURL := config.ServicePublicURL.GetString() + t.Cleanup(func() { config.ServicePublicURL.Set(originalPublicURL) }) + config.ServicePublicURL.Set("https://vikunja.example.com/") + + mailopts, err := RenderMail(NewMail().IncludeLinkToSettings("en"), "en") + require.NoError(t, err) + + assert.Contains(t, mailopts.Message, "here (https://vikunja.example.com/user/settings/general)") + assert.Contains(t, mailopts.HTMLMessage, `here`) + }) t.Run("with footer and action", func(t *testing.T) { mail := NewMail(). From("test@example.com"). @@ -254,7 +266,7 @@ Hi there, This is a line -This **line** contains [a link](https://vikunja.io) +This line contains a link (https://vikunja.io) And another one @@ -314,6 +326,26 @@ This is a footer line // " is the correct HTML entity for the quote character and will render as " in the browser assert.Contains(t, mailopts.HTMLMessage, `"Fix structured data Value in property "reviewCount" must be positive"`) }) + t.Run("renders markdown as plain text", func(t *testing.T) { + title := EscapeMarkdown(`Test - xyz - 123|456@789! Keep \!`) + mail := NewMail(). + From("test@example.com"). + To("test@otherdomain.com"). + Subject("Testmail"). + Greeting("Hi there,"). + Line(`A **friendly** reminder for "` + title + `". See [the task](https://vikunja.io/tasks/1).`). + Line(`* [` + title + `](https://vikunja.io/tasks/1)`) + + mailopts, err := RenderMail(mail, "en") + require.NoError(t, err) + + assert.Contains(t, mailopts.Message, `A friendly reminder for "Test - xyz - 123|456@789! Keep \!". See the task (https://vikunja.io/tasks/1).`) + assert.NotContains(t, mailopts.Message, `\-`) + assert.NotContains(t, mailopts.Message, `**`) + assert.Contains(t, mailopts.Message, `- Test - xyz - 123|456@789! Keep \! (https://vikunja.io/tasks/1)`) + assert.Contains(t, mailopts.HTMLMessage, `friendly`) + assert.Contains(t, mailopts.HTMLMessage, `Test - xyz - 123|456@789! Keep \!`) + }) t.Run("with pre-escaped HTML entities", func(t *testing.T) { // This tests the fix for issue #1664 where HTML entities were being double-escaped mail := NewMail(). @@ -326,8 +358,7 @@ This is a footer line mailopts, err := RenderMail(mail, "en") require.NoError(t, err) - // Plain text should contain the HTML entity as-is (it will be interpreted by email client) - assert.Contains(t, mailopts.Message, `"`) + assert.Contains(t, mailopts.Message, `"already escaped"`) // HTML should properly handle the pre-escaped entity without double-escaping // The entity should remain as " (not become &#34;) diff --git a/pkg/notifications/markdown_escape_test.go b/pkg/notifications/markdown_escape_test.go index 4f29b119e..5d019bc2f 100644 --- a/pkg/notifications/markdown_escape_test.go +++ b/pkg/notifications/markdown_escape_test.go @@ -99,3 +99,57 @@ func TestEscapeMarkdown_RoundTripThroughGoldmark(t *testing.T) { }) } } + +func TestMarkdownToPlainText_RoundTripsEscapedText(t *testing.T) { + inputs := []string{ + "", + "plain title", + `Test - xyz - 123|456@789!`, + `a\b`, + "`code`", + "*bold*", + "test](https://evil.com) [Click to verify", + "![](https://evil.com/track.png)", + "", + `mixed \ and - and | and !`, + "#+.{}()[]<>~_", + } + + for _, input := range inputs { + t.Run(input, func(t *testing.T) { + assert.Equal(t, input, markdownToPlainText(EscapeMarkdown(input))) + }) + } +} + +func TestMarkdownToPlainText_NormalizesDestinations(t *testing.T) { + assert.Equal(t, + `site (https://e.test/a(b)?x=1&y=2)`, + markdownToPlainText(`[site](https://e.test/a\(b\)?x=1&y=2)`), + ) + assert.Equal(t, + `chart (https://e.test/a(b)?x=1&y=2)`, + markdownToPlainText(`![chart](https://e.test/a\(b\)?x=1&y=2)`), + ) +} + +func TestMarkdownToPlainText_PreservesListNumberingAndNesting(t *testing.T) { + markdown := "3. Third\n - Nested one\n - Nested two\n4. Fourth" + want := "3. Third\n - Nested one\n - Nested two\n4. Fourth" + + assert.Equal(t, want, markdownToPlainText(markdown)) +} + +func TestMarkdownToPlainText_AlignsListContinuationLines(t *testing.T) { + markdown := "3. Parent\n - [Nested first\n nested second](https://example.com)\n4. Fourth" + want := "3. Parent\n - Nested first\n nested second (https://example.com)\n4. Fourth" + + assert.Equal(t, want, markdownToPlainText(markdown)) +} + +func TestMarkdownToPlainText_AlignsListContinuationBlocks(t *testing.T) { + markdown := "- first\n\n second\n\n ```\n code one\n code two\n ```\n- next" + want := "- first\n second\n code one\n code two\n- next" + + assert.Equal(t, want, markdownToPlainText(markdown)) +}