From 539412b353da0a86184daec5e9fe9a84da458a6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Thu, 11 Jan 2024 10:52:39 +0100 Subject: [PATCH] Fix MM53643 (#25683) * Fix MM53643 * Add test * Remove unneeded part of a test --------- Co-authored-by: Mattermost Build --- server/channels/app/notification.go | 14 +++---- server/channels/app/notification_test.go | 34 +++++++++++++++++ server/channels/app/post.go | 11 +++--- server/channels/app/post_metadata.go | 48 +++++++++++------------- 4 files changed, 69 insertions(+), 38 deletions(-) diff --git a/server/channels/app/notification.go b/server/channels/app/notification.go index 9605451ffc..8e85059c36 100644 --- a/server/channels/app/notification.go +++ b/server/channels/app/notification.go @@ -485,13 +485,6 @@ func (a *App) SendNotifications(c request.CTX, post *model.Post, team *model.Tea message := model.NewWebSocketEvent(model.WebsocketEventPosted, "", post.ChannelId, "", nil, "") - // Note that PreparePostForClient should've already been called by this point - postJSON, jsonErr := post.ToJSON() - if jsonErr != nil { - return nil, errors.Wrapf(jsonErr, "failed to encode post to JSON") - } - message.Add("post", postJSON) - message.Add("channel_type", channel.Type) message.Add("channel_display_name", notification.GetChannelName(model.ShowUsername, "")) message.Add("channel_name", channel.Name) @@ -530,6 +523,13 @@ func (a *App) SendNotifications(c request.CTX, post *model.Post, team *model.Tea return nil, err } if !published { + removePermalinkMetadataFromPost(post) + postJSON, jsonErr := post.ToJSON() + if jsonErr != nil { + return nil, errors.Wrapf(jsonErr, "failed to encode post to JSON") + } + message.Add("post", postJSON) + a.Publish(message) } diff --git a/server/channels/app/notification_test.go b/server/channels/app/notification_test.go index 40ee2abd1c..ee0685bd40 100644 --- a/server/channels/app/notification_test.go +++ b/server/channels/app/notification_test.go @@ -395,6 +395,40 @@ func TestSendNotifications_MentionsFollowers(t *testing.T) { assert.Nil(t, received2.GetBroadcast().BroadcastHooks) assert.Nil(t, received2.GetBroadcast().BroadcastHookArgs) }) + + t.Run("should sanitize the post if there is an error", func(t *testing.T) { + messages, closeWS1 := connectFakeWebSocket(t, th, th.BasicUser.Id, "") + defer closeWS1() + + linkedPostId := "123456789" + postURL := fmt.Sprintf("%s/%s/pl/%s", th.App.GetSiteURL(), th.BasicTeam.Name, linkedPostId) + post := &model.Post{ + UserId: sender.Id, + ChannelId: th.BasicChannel.Id, + Message: postURL, + Metadata: &model.PostMetadata{ + Embeds: []*model.PostEmbed{ + { + Type: model.PostEmbedPermalink, + URL: postURL, + Data: &model.Post{}, + }, + }, + }, + } + post.SetProps(model.StringInterface{model.PostPropsPreviewedPost: linkedPostId}) + + _, err := th.App.SendNotifications(th.Context, post, th.BasicTeam, th.BasicChannel, sender, nil, false) + require.NoError(t, err) + + received := <-messages + require.Equal(t, model.WebsocketEventPosted, received.EventType()) + receivedPost := &model.Post{} + err = json.Unmarshal([]byte(received.GetData()["post"].(string)), &receivedPost) + require.NoError(t, err) + assert.Equal(t, postURL, receivedPost.Message) + assert.Nil(t, receivedPost.Metadata.Embeds) + }) } func assertUnmarshalsTo(t *testing.T, expected any, actual any) { diff --git a/server/channels/app/post.go b/server/channels/app/post.go index c7ad217669..1553a8f2e6 100644 --- a/server/channels/app/post.go +++ b/server/channels/app/post.go @@ -757,17 +757,18 @@ func (a *App) UpdatePost(c request.CTX, receivedUpdatedPost *model.Post, safeUpd } message := model.NewWebSocketEvent(model.WebsocketEventPostEdited, "", rpost.ChannelId, "", nil, "") - postJSON, jsonErr := rpost.ToJSON() - if jsonErr != nil { - return nil, model.NewAppError("UpdatePost", "app.post.marshal.app_error", nil, "", http.StatusInternalServerError).Wrap(jsonErr) - } - message.Add("post", postJSON) published, err := a.publishWebsocketEventForPermalinkPost(c, rpost, message) if err != nil { return nil, err } if !published { + removePermalinkMetadataFromPost(rpost) + postJSON, jsonErr := rpost.ToJSON() + if jsonErr != nil { + return nil, model.NewAppError("UpdatePost", "app.post.marshal.app_error", nil, "", http.StatusInternalServerError).Wrap(jsonErr) + } + message.Add("post", postJSON) a.Publish(message) } diff --git a/server/channels/app/post_metadata.go b/server/channels/app/post_metadata.go index 4b73688d8e..c6da152faf 100644 --- a/server/channels/app/post_metadata.go +++ b/server/channels/app/post_metadata.go @@ -190,25 +190,33 @@ func (a *App) getEmbedsAndImages(c request.CTX, post *model.Post, isNewPost bool return post } +func removePermalinkMetadataFromPost(post *model.Post) { + if post.Metadata == nil || len(post.Metadata.Embeds) == 0 { + return + } + + // Remove all permalink embeds and only keep non-permalink embeds. + // We always have only one permalink embed even if the post + // contains multiple permalinks. + var newEmbeds []*model.PostEmbed + for _, embed := range post.Metadata.Embeds { + if embed.Type != model.PostEmbedPermalink { + newEmbeds = append(newEmbeds, embed) + } + } + + post.Metadata.Embeds = newEmbeds + + post.DelProp(model.PostPropsPreviewedPost) +} + func (a *App) sanitizePostMetadataForUserAndChannel(c request.CTX, post *model.Post, previewedPost *model.PreviewPost, previewedChannel *model.Channel, userID string) *model.Post { if post.Metadata == nil || len(post.Metadata.Embeds) == 0 || previewedPost == nil { return post } if previewedChannel != nil && !a.HasPermissionToReadChannel(c, userID, previewedChannel) { - // Remove all permalink embeds and only keep non-permalink embeds. - // We always have only one permalink embed even if the post - // contains multiple permalinks. - var newEmbeds []*model.PostEmbed - for _, embed := range post.Metadata.Embeds { - if embed.Type != model.PostEmbedPermalink { - newEmbeds = append(newEmbeds, embed) - } - } - - post.Metadata.Embeds = newEmbeds - - post.DelProp(model.PostPropsPreviewedPost) + removePermalinkMetadataFromPost(post) } return post @@ -230,19 +238,7 @@ func (a *App) SanitizePostMetadataForUser(c request.CTX, post *model.Post, userI } if previewedChannel != nil && !a.HasPermissionToReadChannel(c, userID, previewedChannel) { - // Remove all permalink embeds and only keep non-permalink embeds. - // We always have only one permalink embed even if the post - // contains multiple permalinks. - var newEmbeds []*model.PostEmbed - for _, embed := range post.Metadata.Embeds { - if embed.Type != model.PostEmbedPermalink { - newEmbeds = append(newEmbeds, embed) - } - } - - post.Metadata.Embeds = newEmbeds - - post.DelProp(model.PostPropsPreviewedPost) + removePermalinkMetadataFromPost(post) } return post, nil