From dc366bc1e23bd6c27ade6b225b345dd7b510a258 Mon Sep 17 00:00:00 2001 From: Lev <1187448+levb@users.noreply.github.com> Date: Thu, 21 Jul 2022 07:11:18 -0700 Subject: [PATCH] MM-45208: strip post meta for plugins (#20686) Try two... * Revert "Revert "Removed the opengraph type dependency for plugins by stripping post Metadata (#20612)" (#20684)" This reverts commit 32dee6d4499b40fded7749a7f9b651ddafffb4b6. * Fixed race condition * PR feedback * lint --- app/plugin_api.go | 60 ++++++++++++++++++++++++++++-------- app/post.go | 27 ++++++++-------- model/post.go | 16 +++------- model/post_list.go | 8 +++++ model/post_search_results.go | 6 ++++ plugin/client_rpc.go | 2 -- 6 files changed, 79 insertions(+), 40 deletions(-) diff --git a/app/plugin_api.go b/app/plugin_api.go index 6e0edeab21..0b1187d250 100644 --- a/app/plugin_api.go +++ b/app/plugin_api.go @@ -513,7 +513,7 @@ func (api *PluginAPI) SearchPostsInTeam(teamID string, paramsList []*model.Searc if err != nil { return nil, err } - return postList.ToSlice(), nil + return postList.ForPlugin().ToSlice(), nil } func (api *PluginAPI) SearchPostsInTeamForUser(teamID string, userID string, searchParams model.SearchParameter) (*model.PostSearchResults, *model.AppError) { @@ -547,7 +547,11 @@ func (api *PluginAPI) SearchPostsInTeamForUser(teamID string, userID string, sea includeDeletedChannels = *searchParams.IncludeDeletedChannels } - return api.app.SearchPostsForUser(api.ctx, terms, userID, teamID, isOrSearch, includeDeletedChannels, timeZoneOffset, page, perPage, model.ModifierMessages) + results, appErr := api.app.SearchPostsForUser(api.ctx, terms, userID, teamID, isOrSearch, includeDeletedChannels, timeZoneOffset, page, perPage, model.ModifierMessages) + if results != nil { + results = results.ForPlugin() + } + return results, appErr } func (api *PluginAPI) AddChannelMember(channelID, userID string) (*model.ChannelMember, *model.AppError) { @@ -627,7 +631,11 @@ func (api *PluginAPI) GetGroupsForUser(userID string) ([]*model.Group, *model.Ap } func (api *PluginAPI) CreatePost(post *model.Post) (*model.Post, *model.AppError) { - return api.app.CreatePostMissingChannel(api.ctx, post, true) + post, appErr := api.app.CreatePostMissingChannel(api.ctx, post, true) + if post != nil { + post = post.ForPlugin() + } + return post, appErr } func (api *PluginAPI) AddReaction(reaction *model.Reaction) (*model.Reaction, *model.AppError) { @@ -643,11 +651,11 @@ func (api *PluginAPI) GetReactions(postID string) ([]*model.Reaction, *model.App } func (api *PluginAPI) SendEphemeralPost(userID string, post *model.Post) *model.Post { - return api.app.SendEphemeralPost(api.ctx, userID, post) + return api.app.SendEphemeralPost(api.ctx, userID, post).ForPlugin() } func (api *PluginAPI) UpdateEphemeralPost(userID string, post *model.Post) *model.Post { - return api.app.UpdateEphemeralPost(api.ctx, userID, post) + return api.app.UpdateEphemeralPost(api.ctx, userID, post).ForPlugin() } func (api *PluginAPI) DeleteEphemeralPost(userID, postID string) { @@ -660,31 +668,59 @@ func (api *PluginAPI) DeletePost(postID string) *model.AppError { } func (api *PluginAPI) GetPostThread(postID string) (*model.PostList, *model.AppError) { - return api.app.GetPostThread(postID, model.GetPostsOptions{}, "") + list, appErr := api.app.GetPostThread(postID, model.GetPostsOptions{}, "") + if list != nil { + list = list.ForPlugin() + } + return list, appErr } func (api *PluginAPI) GetPost(postID string) (*model.Post, *model.AppError) { - return api.app.GetSinglePost(postID, false) + post, appErr := api.app.GetSinglePost(postID, false) + if post != nil { + post = post.ForPlugin() + } + return post, appErr } func (api *PluginAPI) GetPostsSince(channelID string, time int64) (*model.PostList, *model.AppError) { - return api.app.GetPostsSince(model.GetPostsSinceOptions{ChannelId: channelID, Time: time}) + list, appErr := api.app.GetPostsSince(model.GetPostsSinceOptions{ChannelId: channelID, Time: time}) + if list != nil { + list = list.ForPlugin() + } + return list, appErr } func (api *PluginAPI) GetPostsAfter(channelID, postID string, page, perPage int) (*model.PostList, *model.AppError) { - return api.app.GetPostsAfterPost(model.GetPostsOptions{ChannelId: channelID, PostId: postID, Page: page, PerPage: perPage}) + list, appErr := api.app.GetPostsAfterPost(model.GetPostsOptions{ChannelId: channelID, PostId: postID, Page: page, PerPage: perPage}) + if list != nil { + list = list.ForPlugin() + } + return list, appErr } func (api *PluginAPI) GetPostsBefore(channelID, postID string, page, perPage int) (*model.PostList, *model.AppError) { - return api.app.GetPostsBeforePost(model.GetPostsOptions{ChannelId: channelID, PostId: postID, Page: page, PerPage: perPage}) + list, appErr := api.app.GetPostsBeforePost(model.GetPostsOptions{ChannelId: channelID, PostId: postID, Page: page, PerPage: perPage}) + if list != nil { + list = list.ForPlugin() + } + return list, appErr } func (api *PluginAPI) GetPostsForChannel(channelID string, page, perPage int) (*model.PostList, *model.AppError) { - return api.app.GetPostsPage(model.GetPostsOptions{ChannelId: channelID, Page: page, PerPage: perPage}) + list, appErr := api.app.GetPostsPage(model.GetPostsOptions{ChannelId: channelID, Page: page, PerPage: perPage}) + if list != nil { + list = list.ForPlugin() + } + return list, appErr } func (api *PluginAPI) UpdatePost(post *model.Post) (*model.Post, *model.AppError) { - return api.app.UpdatePost(api.ctx, post, false) + post, appErr := api.app.UpdatePost(api.ctx, post, false) + if post != nil { + post = post.ForPlugin() + } + return post, appErr } func (api *PluginAPI) GetProfileImage(userID string) ([]byte, *model.AppError) { diff --git a/app/post.go b/app/post.go index 2379ee300f..487537029d 100644 --- a/app/post.go +++ b/app/post.go @@ -259,7 +259,7 @@ func (a *App) CreatePost(c request.CTX, post *model.Post, channel *model.Channel var rejectionError *model.AppError pluginContext := pluginContext(c) pluginsEnvironment.RunMultiPluginHook(func(hooks plugin.Hooks) bool { - replacementPost, rejectionReason := hooks.MessageWillBePosted(pluginContext, post) + replacementPost, rejectionReason := hooks.MessageWillBePosted(pluginContext, post.ForPlugin()) if rejectionReason != "" { id := "Post rejected by plugin. " + rejectionReason if rejectionReason == plugin.DismissPostError { @@ -269,6 +269,7 @@ func (a *App) CreatePost(c request.CTX, post *model.Post, channel *model.Channel return false } if replacementPost != nil { + // the original post's metadata (if there ever was any) is lost, and will be rebuilt. post = replacementPost } @@ -309,21 +310,14 @@ func (a *App) CreatePost(c request.CTX, post *model.Post, channel *model.Channel // might be duplicating requests. a.Srv().seenPendingPostIdsCache.SetWithExpiry(post.PendingPostId, rpost.Id, PendingPostIDsCacheTTL) - // We make a copy of the post for the plugin hook to avoid a race condition. - rPostCopy := rpost.Clone() - - // FIXME: Removes PreviewPost from the post payload sent to the MessageHasBeenPosted hook so that plugins compiled with older versions of - // Mattermost—without the gob registration of the PreviewPost struct—won't crash. - if rPostCopy.Metadata != nil { - rPostCopy.Metadata = rPostCopy.Metadata.Copy() - } - rPostCopy.RemovePreviewPost() - + // We make a copy of the post for the plugin hook to avoid a race condition, + // and to remove the non-GOB-encodable Metadata from it. if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { + pluginPost := rpost.ForPlugin() a.Srv().Go(func() { pluginContext := pluginContext(c) pluginsEnvironment.RunMultiPluginHook(func(hooks plugin.Hooks) bool { - hooks.MessageHasBeenPosted(pluginContext, rPostCopy) + hooks.MessageHasBeenPosted(pluginContext, pluginPost) return true }, plugin.MessageHasBeenPostedID) }) @@ -650,12 +644,15 @@ func (a *App) UpdatePost(c *request.Context, post *model.Post, safeUpdate bool) var rejectionReason string pluginContext := pluginContext(c) pluginsEnvironment.RunMultiPluginHook(func(hooks plugin.Hooks) bool { - newPost, rejectionReason = hooks.MessageWillBeUpdated(pluginContext, newPost, oldPost) + newPost, rejectionReason = hooks.MessageWillBeUpdated(pluginContext, newPost.ForPlugin(), oldPost.ForPlugin()) return post != nil }, plugin.MessageWillBeUpdatedID) if newPost == nil { return nil, model.NewAppError("UpdatePost", "Post rejected by plugin. "+rejectionReason, nil, "", http.StatusBadRequest) } + // Restore the post metadata that was stripped by the plugin. Set it to + // the last known good. + newPost.Metadata = oldPost.Metadata } rpost, nErr := a.Srv().Store.Post().Update(newPost, oldPost) @@ -670,10 +667,12 @@ func (a *App) UpdatePost(c *request.Context, post *model.Post, safeUpdate bool) } if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { + pluginOldPost := oldPost.ForPlugin() + pluginNewPost := newPost.ForPlugin() a.Srv().Go(func() { pluginContext := pluginContext(c) pluginsEnvironment.RunMultiPluginHook(func(hooks plugin.Hooks) bool { - hooks.MessageHasBeenUpdated(pluginContext, newPost, oldPost) + hooks.MessageHasBeenUpdated(pluginContext, pluginNewPost, pluginOldPost) return true }, plugin.MessageHasBeenUpdatedID) }) diff --git a/model/post.go b/model/post.go index 29ac05b263..f8e3240154 100644 --- a/model/post.go +++ b/model/post.go @@ -736,18 +736,10 @@ func (o *Post) ToNilIfInvalid() *Post { return o } -func (o *Post) RemovePreviewPost() { - if o.Metadata == nil || o.Metadata.Embeds == nil { - return - } - n := 0 - for _, embed := range o.Metadata.Embeds { - if embed.Type != PostEmbedPermalink { - o.Metadata.Embeds[n] = embed - n++ - } - } - o.Metadata.Embeds = o.Metadata.Embeds[:n] +func (o *Post) ForPlugin() *Post { + p := o.Clone() + p.Metadata = nil + return p } func (o *Post) GetPreviewPost() *PreviewPost { diff --git a/model/post_list.go b/model/post_list.go index 0801b5c8e1..34fc031e3f 100644 --- a/model/post_list.go +++ b/model/post_list.go @@ -46,6 +46,14 @@ func (o *PostList) Clone() *PostList { } } +func (o *PostList) ForPlugin() *PostList { + copy := o.Clone() + for k, p := range copy.Posts { + copy.Posts[k] = p.ForPlugin() + } + return copy +} + func (o *PostList) ToSlice() []*Post { var posts []*Post diff --git a/model/post_search_results.go b/model/post_search_results.go index a3afc7231a..23511039a7 100644 --- a/model/post_search_results.go +++ b/model/post_search_results.go @@ -33,3 +33,9 @@ func (o *PostSearchResults) EncodeJSON(w io.Writer) error { o.PostList.StripActionIntegrations() return json.NewEncoder(w).Encode(o) } + +func (o *PostSearchResults) ForPlugin() *PostSearchResults { + copy := *o + copy.PostList = copy.PostList.ForPlugin() + return © +} diff --git a/plugin/client_rpc.go b/plugin/client_rpc.go index aaf4f66a1d..12c54b61ba 100644 --- a/plugin/client_rpc.go +++ b/plugin/client_rpc.go @@ -21,7 +21,6 @@ import ( "reflect" "sync" - "github.com/dyatlov/go-opengraph/opengraph" "github.com/go-sql-driver/mysql" "github.com/hashicorp/go-plugin" "github.com/lib/pq" @@ -164,7 +163,6 @@ func init() { gob.Register(&pq.Error{}) gob.Register(&mysql.MySQLError{}) gob.Register(&ErrorString{}) - gob.Register(&opengraph.OpenGraph{}) gob.Register(&model.AutocompleteDynamicListArg{}) gob.Register(&model.AutocompleteStaticListArg{}) gob.Register(&model.AutocompleteTextArg{})