From 6020499a42af6837dc5f5afdf93a531e46094f96 Mon Sep 17 00:00:00 2001 From: Maximilian Ripper Date: Tue, 17 Jan 2023 18:21:32 +0100 Subject: [PATCH 1/4] MM-48089 Fix relative email urls --- app/email/email_batching.go | 2 +- app/email/notification_email.go | 12 +++++----- app/email/notification_email_test.go | 12 +++++----- app/notification_email.go | 4 ++-- utils/markdown.go | 11 +++++++-- utils/markdown_test.go | 34 ++++++++++++++++++++++++++++ 6 files changed, 58 insertions(+), 17 deletions(-) diff --git a/app/email/email_batching.go b/app/email/email_batching.go index c4b3b01b60..a5d47ceeb6 100644 --- a/app/email/email_batching.go +++ b/app/email/email_batching.go @@ -315,7 +315,7 @@ func (es *Service) sendBatchedEmailNotification(userID string, notifications []* MessageURL: MessageURL, ShowChannelIcon: showChannelIcon, OtherChannelMembersCount: otherChannelMembersCount, - MessageAttachments: ProcessMessageAttachments(notification.post), + MessageAttachments: ProcessMessageAttachments(notification.post, siteURL), }) } } diff --git a/app/email/notification_email.go b/app/email/notification_email.go index 06c0364b38..d8b0327220 100644 --- a/app/email/notification_email.go +++ b/app/email/notification_email.go @@ -60,14 +60,14 @@ func (es *Service) GetMessageForNotification(post *model.Post, translateFunc i18 return translateFunc("api.post.get_message_for_notification.files_sent", len(filenames), props) } -func ProcessMessageAttachments(post *model.Post) []*EmailMessageAttachment { +func ProcessMessageAttachments(post *model.Post, siteURL string) []*EmailMessageAttachment { emailMessageAttachments := []*EmailMessageAttachment{} for _, messageAttachment := range post.Attachments() { emailMessageAttachment := &EmailMessageAttachment{ SlackAttachment: *messageAttachment, - Pretext: prepareTextForEmail(messageAttachment.Pretext), - Text: prepareTextForEmail(messageAttachment.Text), + Pretext: prepareTextForEmail(messageAttachment.Pretext, siteURL), + Text: prepareTextForEmail(messageAttachment.Text, siteURL), } stripedTitle, err := utils.StripMarkdown(emailMessageAttachment.Title) @@ -92,7 +92,7 @@ func ProcessMessageAttachments(post *model.Post) []*EmailMessageAttachment { } if stringValue, ok := field.Value.(string); ok { - field.Value = prepareTextForEmail(stringValue) + field.Value = prepareTextForEmail(stringValue, siteURL) } if !field.Short { @@ -124,9 +124,9 @@ func ProcessMessageAttachments(post *model.Post) []*EmailMessageAttachment { return emailMessageAttachments } -func prepareTextForEmail(text string) template.HTML { +func prepareTextForEmail(text, siteURL string) template.HTML { escapedText := html.EscapeString(text) - markdownText, err := utils.MarkdownToHTML(escapedText) + markdownText, err := utils.MarkdownToHTML(escapedText, siteURL) if err != nil { mlog.Warn("Encountered error while converting markdown to HTML", mlog.Err(err)) return template.HTML(text) diff --git a/app/email/notification_email_test.go b/app/email/notification_email_test.go index 4f8e176c5b..74a67d9289 100644 --- a/app/email/notification_email_test.go +++ b/app/email/notification_email_test.go @@ -63,10 +63,10 @@ func TestProcessMessageAttachments(t *testing.T) { model.ParseSlackAttachment(post, messageAttachments) - processedAttachcmentsPost := ProcessMessageAttachments(post) - require.NotNil(t, processedAttachcmentsPost) - require.Len(t, processedAttachcmentsPost, 2) - require.Equal(t, processedAttachcmentsPost[0].Color, "#FF0000") - require.Equal(t, processedAttachcmentsPost[0].FieldRows[0].Cells[0].Title, "message attachment 1 field 1 title") - require.Equal(t, processedAttachcmentsPost[1].Color, "#FF0000") + processedAttachmentsPost := ProcessMessageAttachments(post, "https://example.com") + require.NotNil(t, processedAttachmentsPost) + require.Len(t, processedAttachmentsPost, 2) + require.Equal(t, processedAttachmentsPost[0].Color, "#FF0000") + require.Equal(t, processedAttachmentsPost[0].FieldRows[0].Cells[0].Title, "message attachment 1 field 1 title") + require.Equal(t, processedAttachmentsPost[1].Color, "#FF0000") } diff --git a/app/notification_email.go b/app/notification_email.go index a56180ea2f..0d2490db65 100644 --- a/app/notification_email.go +++ b/app/notification_email.go @@ -234,7 +234,7 @@ func (a *App) getNotificationEmailBody(c request.CTX, recipient *model.User, pos if emailNotificationContentsType == model.EmailNotificationContentsFull { postMessage := a.GetMessageForNotification(post, translateFunc) postMessage = html.EscapeString(postMessage) - mdPostMessage, mdErr := utils.MarkdownToHTML(postMessage) + mdPostMessage, mdErr := utils.MarkdownToHTML(postMessage, a.GetSiteURL()) if mdErr != nil { mlog.Warn("Encountered error while converting markdown to HTML", mlog.Err(mdErr)) mdPostMessage = postMessage @@ -247,7 +247,7 @@ func (a *App) getNotificationEmailBody(c request.CTX, recipient *model.User, pos } pData.Message = template.HTML(normalizedPostMessage) pData.Time = translateFunc("app.notification.body.dm.time", messageTime) - pData.MessageAttachments = email.ProcessMessageAttachments(post) + pData.MessageAttachments = email.ProcessMessageAttachments(post, a.GetSiteURL()) } data := a.Srv().EmailService.NewEmailTemplateData(recipient.Locale) diff --git a/utils/markdown.go b/utils/markdown.go index 7783edd277..1e103efa5c 100644 --- a/utils/markdown.go +++ b/utils/markdown.go @@ -36,10 +36,17 @@ func StripMarkdown(markdown string) (string, error) { } // MarkdownToHTML takes a string containing Markdown and returns a string with HTML tagged version -func MarkdownToHTML(markdown string) (string, error) { +func MarkdownToHTML(markdown, siteURL string) (string, error) { + // Turn relative links into absolute links + relLinkRe := regexp.MustCompile(`\[(.*)]\((/.*)\)`) + absLinkMarkdown := relLinkRe.ReplaceAllFunc([]byte(markdown), func(s []byte) []byte { + out := relLinkRe.ReplaceAllString(string(s), "[$1]("+siteURL+"$2)") + return []byte(out) + }) + // Unescape any blockquote text to be parsed by the markdown parser. re := regexp.MustCompile(`^|\n(>)`) - markdownClean := re.ReplaceAllFunc([]byte(markdown), func(s []byte) []byte { + markdownClean := re.ReplaceAllFunc([]byte(absLinkMarkdown), func(s []byte) []byte { out := html.UnescapeString(string(s)) return []byte(out) }) diff --git a/utils/markdown_test.go b/utils/markdown_test.go index 85e7212d21..8f6bbdb0f2 100644 --- a/utils/markdown_test.go +++ b/utils/markdown_test.go @@ -280,3 +280,37 @@ func TestStripMarkdown(t *testing.T) { }) } } + +func TestMarkdownToHTML(t *testing.T) { + siteURL := "https://example.com" + tests := []struct { + name string + markdown string + want string + }{ + { + name: "absolute url not changed", + markdown: "[Link](https://example.com)", + want: "

Link

\n", + }, + { + name: "relative url changed to absolute url", + markdown: "[Link](/foo)", + want: "

Link

\n", + }, + { + name: "relative url with query params changed to absolute url", + markdown: "[Link](/foo?bar=true)", + want: "

Link

\n", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := MarkdownToHTML(tt.markdown, siteURL) + if err != nil { + t.Fatalf("error: %v", err) + } + assert.Equal(t, tt.want, got) + }) + } +} From 7e1e1ea5e85e4f6b911e94182694f61b9a34b0a7 Mon Sep 17 00:00:00 2001 From: Maximilian Ripper Date: Tue, 17 Jan 2023 18:26:56 +0100 Subject: [PATCH 2/4] MM-48089 Fix linter issue --- utils/markdown.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/markdown.go b/utils/markdown.go index 1e103efa5c..d5c0d7b196 100644 --- a/utils/markdown.go +++ b/utils/markdown.go @@ -46,7 +46,7 @@ func MarkdownToHTML(markdown, siteURL string) (string, error) { // Unescape any blockquote text to be parsed by the markdown parser. re := regexp.MustCompile(`^|\n(>)`) - markdownClean := re.ReplaceAllFunc([]byte(absLinkMarkdown), func(s []byte) []byte { + markdownClean := re.ReplaceAllFunc(absLinkMarkdown, func(s []byte) []byte { out := html.UnescapeString(string(s)) return []byte(out) }) From dd0eac29d04f06d8272e0af70d1826a45fefb66c Mon Sep 17 00:00:00 2001 From: Maximilian Ripper Date: Wed, 18 Jan 2023 14:28:55 +0100 Subject: [PATCH 3/4] MM-48089 Compile regular expressions once instead of doing it with every call to `MarkdownToHTML` --- utils/markdown.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/utils/markdown.go b/utils/markdown.go index d5c0d7b196..6d4d05bab6 100644 --- a/utils/markdown.go +++ b/utils/markdown.go @@ -35,18 +35,19 @@ func StripMarkdown(markdown string) (string, error) { return strings.TrimSpace(buf.String()), nil } +var relLinkReg = regexp.MustCompile(`\[(.*)]\((/.*)\)`) +var blockquoteReg = regexp.MustCompile(`^|\n(>)`) + // MarkdownToHTML takes a string containing Markdown and returns a string with HTML tagged version func MarkdownToHTML(markdown, siteURL string) (string, error) { // Turn relative links into absolute links - relLinkRe := regexp.MustCompile(`\[(.*)]\((/.*)\)`) - absLinkMarkdown := relLinkRe.ReplaceAllFunc([]byte(markdown), func(s []byte) []byte { - out := relLinkRe.ReplaceAllString(string(s), "[$1]("+siteURL+"$2)") + absLinkMarkdown := relLinkReg.ReplaceAllFunc([]byte(markdown), func(s []byte) []byte { + out := relLinkReg.ReplaceAllString(string(s), "[$1]("+siteURL+"$2)") return []byte(out) }) // Unescape any blockquote text to be parsed by the markdown parser. - re := regexp.MustCompile(`^|\n(>)`) - markdownClean := re.ReplaceAllFunc(absLinkMarkdown, func(s []byte) []byte { + markdownClean := blockquoteReg.ReplaceAllFunc(absLinkMarkdown, func(s []byte) []byte { out := html.UnescapeString(string(s)) return []byte(out) }) From 5b80211e66c1ff4377583364dd87e2728badc63d Mon Sep 17 00:00:00 2001 From: Maximilian Ripper Date: Fri, 20 Jan 2023 13:36:02 +0100 Subject: [PATCH 4/4] MM-48089 Use `ReplaceAllStringFunc` to avoid double conversion --- utils/markdown.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/utils/markdown.go b/utils/markdown.go index 6d4d05bab6..92759b8b69 100644 --- a/utils/markdown.go +++ b/utils/markdown.go @@ -41,15 +41,13 @@ var blockquoteReg = regexp.MustCompile(`^|\n(>)`) // MarkdownToHTML takes a string containing Markdown and returns a string with HTML tagged version func MarkdownToHTML(markdown, siteURL string) (string, error) { // Turn relative links into absolute links - absLinkMarkdown := relLinkReg.ReplaceAllFunc([]byte(markdown), func(s []byte) []byte { - out := relLinkReg.ReplaceAllString(string(s), "[$1]("+siteURL+"$2)") - return []byte(out) + absLinkMarkdown := relLinkReg.ReplaceAllStringFunc(markdown, func(s string) string { + return relLinkReg.ReplaceAllString(s, "[$1]("+siteURL+"$2)") }) // Unescape any blockquote text to be parsed by the markdown parser. - markdownClean := blockquoteReg.ReplaceAllFunc(absLinkMarkdown, func(s []byte) []byte { - out := html.UnescapeString(string(s)) - return []byte(out) + markdownClean := blockquoteReg.ReplaceAllStringFunc(absLinkMarkdown, func(s string) string { + return html.UnescapeString(s) }) md := goldmark.New( @@ -58,7 +56,7 @@ func MarkdownToHTML(markdown, siteURL string) (string, error) { var b strings.Builder - err := md.Convert(markdownClean, &b) + err := md.Convert([]byte(markdownClean), &b) if err != nil { return "", err }