Fix MM53643 (#25683)

* Fix MM53643

* Add test

* Remove unneeded part of a test

---------

Co-authored-by: Mattermost Build <build@mattermost.com>

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

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) {

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

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