From 22b3d9e4cb70a4b7e510a2e8149a8578f78f3e56 Mon Sep 17 00:00:00 2001
From: Harshil Sharma <18575143+harshilsharma63@users.noreply.github.com>
Date: Thu, 30 Mar 2023 07:38:53 +0530
Subject: [PATCH] Fixed MM-51060 (#22532)

* Fixed MM-51060

* Minor improvements

* Breaking bigger loops

* CI

* Removed an unintended line change

---------

Co-authored-by: Mattermost Build <build@mattermost.com>
---
 server/channels/app/post_metadata.go      |  9 +++++
 server/channels/app/post_metadata_test.go | 47 +++++++++++++++++++++++
 2 files changed, 56 insertions(+)

diff --git a/server/channels/app/post_metadata.go b/server/channels/app/post_metadata.go
index fc87374fe3..460354eaa5 100644
--- a/server/channels/app/post_metadata.go
+++ b/server/channels/app/post_metadata.go
@@ -363,6 +363,12 @@ func (a *App) getImagesForPost(c request.CTX, post *model.Post, imageURLs []stri
 	}
 
 	for _, imageURL := range imageURLs {
+		// prevent infinite loop if a OG image URL is the same post's permalink
+		resolvedURL := resolveMetadataURL(imageURL, a.GetSiteURL())
+		if looksLikeAPermalink(resolvedURL, a.GetSiteURL()) {
+			continue
+		}
+
 		if _, image, _, err := a.getLinkMetadata(c, imageURL, post.CreateAt, isNewPost, post.GetPreviewedPostProp()); err != nil {
 			appErr, ok := err.(*model.AppError)
 			isNotFound := ok && appErr.StatusCode == http.StatusNotFound
@@ -651,6 +657,9 @@ func (a *App) getLinkMetadata(c request.CTX, requestURL string, timestamp int64,
 
 			var res *http.Response
 			res, err = client.Do(request)
+			if err != nil {
+				mlog.Warn("error fetching OG image data", mlog.Err(err))
+			}
 
 			if res != nil {
 				body = res.Body
diff --git a/server/channels/app/post_metadata_test.go b/server/channels/app/post_metadata_test.go
index 9171b9c657..8ee5f71d3b 100644
--- a/server/channels/app/post_metadata_test.go
+++ b/server/channels/app/post_metadata_test.go
@@ -18,6 +18,10 @@ import (
 	"testing"
 	"time"
 
+	"github.com/mattermost/mattermost-server/v6/server/channels/store"
+	"github.com/mattermost/mattermost-server/v6/server/channels/store/storetest/mocks"
+	"github.com/stretchr/testify/mock"
+
 	"github.com/dyatlov/go-opengraph/opengraph"
 	ogimage "github.com/dyatlov/go-opengraph/opengraph/types/image"
 	"github.com/stretchr/testify/assert"
@@ -1311,6 +1315,49 @@ func TestGetImagesForPost(t *testing.T) {
 		images := th.App.getImagesForPost(th.Context, post, []string{}, false)
 		assert.Equal(t, images, map[string]*model.PostImage{})
 	})
+
+	t.Run("should not process OpenGraph image that's a Mattermost permalink", func(t *testing.T) {
+		th := SetupWithStoreMock(t)
+		defer th.TearDown()
+
+		ogURL := "https://example.com/index.html"
+		imageURL := th.App.GetSiteURL() + "/pl/qwertyuiopasdfghjklzxcvbnm"
+
+		post := &model.Post{
+			Id: "qwertyuiopasdfghjklzxcvbnm",
+			Metadata: &model.PostMetadata{
+				Embeds: []*model.PostEmbed{
+					{
+						Type: model.PostEmbedOpengraph,
+						URL:  ogURL,
+						Data: &opengraph.OpenGraph{
+							Images: []*ogimage.Image{
+								{
+									URL: imageURL,
+								},
+							},
+						},
+					},
+				},
+			},
+		}
+
+		mockPostStore := mocks.PostStore{}
+		mockPostStore.On("GetSingle", "qwertyuiopasdfghjklzxcvbnm", false).RunFn = func(args mock.Arguments) {
+			assert.Fail(t, "should not have tried to process Mattermost permalink in OG image URL")
+		}
+
+		mockLinkMetadataStore := mocks.LinkMetadataStore{}
+		mockLinkMetadataStore.On("Get", mock.Anything, mock.Anything).Return(nil, store.NewErrNotFound("mock resource", "mock ID"))
+
+		mockStore := th.App.Srv().Store().(*mocks.Store)
+		mockStore.On("Post").Return(&mockPostStore)
+		mockStore.On("LinkMetadata").Return(&mockLinkMetadataStore)
+
+		images := th.App.getImagesForPost(th.Context, post, []string{}, false)
+		assert.Equal(t, 0, len(images))
+		assert.Equal(t, images, map[string]*model.PostImage{})
+	})
 }
 
 func TestGetEmojiNamesForString(t *testing.T) {