From 32005b05d919c43828653e8c806e9300754c9d10 Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Wed, 4 Aug 2021 19:43:37 +0530 Subject: [PATCH] MM-37573: Ignore nil fields in attachments (#18052) We extend the same logic done in https://github.com/mattermost/mattermost-server/pull/16556 to Field. Also handled the case elsewhere in the code with a light grep for extra safety. https://mattermost.atlassian.net/browse/MM-37573 ```release-note NONE ``` --- app/post_metadata.go | 6 ++++++ app/slack.go | 2 +- model/post.go | 11 +++++++++++ model/post_test.go | 16 ++++++++++++++++ model/slack_attachment.go | 3 +++ 5 files changed, 37 insertions(+), 1 deletion(-) diff --git a/app/post_metadata.go b/app/post_metadata.go index 44412919b7..43ffbe144d 100644 --- a/app/post_metadata.go +++ b/app/post_metadata.go @@ -283,6 +283,9 @@ func getEmojiNamesForPost(post *model.Post, reactions []*model.Reaction) []strin } for _, field := range attachment.Fields { + if field == nil { + continue + } if value, ok := field.Value.(string); ok { names = append(names, getEmojiNamesForString(value)...) } @@ -361,6 +364,9 @@ func (a *App) getImagesInMessageAttachments(post *model.Post) []string { images = append(images, imagesInPretext...) for _, field := range attachment.Fields { + if field == nil { + continue + } if value, ok := field.Value.(string); ok { _, imagesInFieldValue := a.getFirstLinkAndImages(value) images = append(images, imagesInFieldValue...) diff --git a/app/slack.go b/app/slack.go index f850c90e61..61fa49127f 100644 --- a/app/slack.go +++ b/app/slack.go @@ -72,7 +72,7 @@ func (a *App) ProcessSlackAttachments(attachments []*model.SlackAttachment) []*m attachment.Title = a.ProcessSlackText(attachment.Title) for _, field := range attachment.Fields { - if field.Value != nil { + if field != nil && field.Value != nil { // Ensure the value is set to a string if it is set field.Value = a.ProcessSlackText(fmt.Sprintf("%v", field.Value)) } diff --git a/model/post.go b/model/post.go index e60ff9a735..b711e58b29 100644 --- a/model/post.go +++ b/model/post.go @@ -605,6 +605,7 @@ func (o *Post) Attachments() []*SlackAttachment { if enc, err := json.Marshal(attachment); err == nil { var decoded SlackAttachment if json.Unmarshal(enc, &decoded) == nil { + // Ignoring nil actions i := 0 for _, action := range decoded.Actions { if action != nil { @@ -613,6 +614,16 @@ func (o *Post) Attachments() []*SlackAttachment { } } decoded.Actions = decoded.Actions[:i] + + // Ignoring nil fields + i = 0 + for _, field := range decoded.Fields { + if field != nil { + decoded.Fields[i] = field + i++ + } + } + decoded.Fields = decoded.Fields[:i] ret = append(ret, &decoded) } } diff --git a/model/post_test.go b/model/post_test.go index 44281deef3..ace6be13bc 100644 --- a/model/post_test.go +++ b/model/post_test.go @@ -929,4 +929,20 @@ func TestPostAttachments(t *testing.T) { require.Equal(t, attachments[0].Actions[0].Id, "test1") require.Equal(t, attachments[0].Actions[1].Id, "test2") }) + + t.Run("nil fields", func(t *testing.T) { + p.Props["attachments"] = []interface{}{ + map[string]interface{}{"fields": []interface{}{ + map[string]interface{}{"value": ":emoji1:"}, + nil, + map[string]interface{}{"value": ":emoji2:"}, + }, + }, + } + + attachments := p.Attachments() + require.Len(t, attachments[0].Fields, 2) + assert.Equal(t, attachments[0].Fields[0].Value, ":emoji1:") + assert.Equal(t, attachments[0].Fields[1].Value, ":emoji2:") + }) } diff --git a/model/slack_attachment.go b/model/slack_attachment.go index 3c167b7de8..94ba9935f3 100644 --- a/model/slack_attachment.go +++ b/model/slack_attachment.go @@ -179,6 +179,9 @@ func ParseSlackAttachment(post *Post, attachments []*SlackAttachment) { attachment.Pretext = ParseSlackLinksToMarkdown(attachment.Pretext) for _, field := range attachment.Fields { + if field == nil { + continue + } if value, ok := field.Value.(string); ok { field.Value = ParseSlackLinksToMarkdown(value) }