Merge pull request #22101 from mattermost/mm-48089-fix-relative-email-urls

MM-48089 Fix broken links in emails

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),
 			})
 		}
 	}

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)

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")
 }

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)

utils/markdown.go

@@ -35,13 +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 string) (string, error) {
+func MarkdownToHTML(markdown, siteURL string) (string, error) {
+	// Turn relative links into absolute links
+	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.
-	re := regexp.MustCompile(`^|\n(>)`)
-	markdownClean := re.ReplaceAllFunc([]byte(markdown), 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(
@@ -50,7 +56,7 @@ func MarkdownToHTML(markdown string) (string, error) {
 
 	var b strings.Builder
 
-	err := md.Convert(markdownClean, &b)
+	err := md.Convert([]byte(markdownClean), &b)
 	if err != nil {
 		return "", err
 	}

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:     "<p><a href=\"https://example.com\">Link</a></p>\n",
+		},
+		{
+			name:     "relative url changed to absolute url",
+			markdown: "[Link](/foo)",
+			want:     "<p><a href=\"https://example.com/foo\">Link</a></p>\n",
+		},
+		{
+			name:     "relative url with query params changed to absolute url",
+			markdown: "[Link](/foo?bar=true)",
+			want:     "<p><a href=\"https://example.com/foo?bar=true\">Link</a></p>\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)
+		})
+	}
+}