From bd5616557d860c5532a94875450bf9a9fc095297 Mon Sep 17 00:00:00 2001
From: Ibrahim Serdar Acikgoz <serdaracikgoz86@gmail.com>
Date: Thu, 24 Dec 2020 09:00:11 +0300
Subject: [PATCH] [MM-30978] avoid parsing nil PostAction in SlackAttachments
 (#16556)

* avoid parsing nil PostAction in SlackAttachments

* reflect review comments

* improve tests

Co-authored-by: Mattermod <mattermod@users.noreply.github.com>
---
 model/integration_action.go |  4 ++--
 model/post.go               |  8 +++++++
 model/post_test.go          | 45 +++++++++++++++++++++++++++++++++++++
 3 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/model/integration_action.go b/model/integration_action.go
index a572c9dec3..7124a7e37b 100644
--- a/model/integration_action.go
+++ b/model/integration_action.go
@@ -394,7 +394,7 @@ func (o *Post) StripActionIntegrations() {
 func (o *Post) GetAction(id string) *PostAction {
 	for _, attachment := range o.Attachments() {
 		for _, action := range attachment.Actions {
-			if action.Id == id {
+			if action != nil && action.Id == id {
 				return action
 			}
 		}
@@ -409,7 +409,7 @@ func (o *Post) GenerateActionIds() {
 	if attachments, ok := o.GetProp("attachments").([]*SlackAttachment); ok {
 		for _, attachment := range attachments {
 			for _, action := range attachment.Actions {
-				if action.Id == "" {
+				if action != nil && action.Id == "" {
 					action.Id = NewId()
 				}
 			}
diff --git a/model/post.go b/model/post.go
index abb88fbf71..0e0795bb39 100644
--- a/model/post.go
+++ b/model/post.go
@@ -557,6 +557,14 @@ func (o *Post) Attachments() []*SlackAttachment {
 			if enc, err := json.Marshal(attachment); err == nil {
 				var decoded SlackAttachment
 				if json.Unmarshal(enc, &decoded) == nil {
+					i := 0
+					for _, action := range decoded.Actions {
+						if action != nil {
+							decoded.Actions[i] = action
+							i++
+						}
+					}
+					decoded.Actions = decoded.Actions[:i]
 					ret = append(ret, &decoded)
 				}
 			}
diff --git a/model/post_test.go b/model/post_test.go
index d1a94a27d4..9eff8115ec 100644
--- a/model/post_test.go
+++ b/model/post_test.go
@@ -884,3 +884,48 @@ func TestSearchParameterFromJson(t *testing.T) {
 		require.Equal(t, "test", *params.Terms)
 	})
 }
+
+func TestPostAttachments(t *testing.T) {
+	p := &Post{
+		Props: map[string]interface{}{
+			"attachments": []byte(`[{
+				"actions" : {null}
+			}]
+			`),
+		},
+	}
+
+	t.Run("empty actions", func(t *testing.T) {
+		p.Props["attachments"] = []interface{}{
+			map[string]interface{}{"actions": []interface{}{}},
+		}
+		attachments := p.Attachments()
+		require.Empty(t, attachments[0].Actions)
+	})
+
+	t.Run("a couple of actions", func(t *testing.T) {
+		p.Props["attachments"] = []interface{}{
+			map[string]interface{}{"actions": []interface{}{
+				map[string]interface{}{"id": "test1"}, map[string]interface{}{"id": "test2"}},
+			},
+		}
+
+		attachments := p.Attachments()
+		require.Len(t, attachments[0].Actions, 2)
+		require.Equal(t, attachments[0].Actions[0].Id, "test1")
+		require.Equal(t, attachments[0].Actions[1].Id, "test2")
+	})
+
+	t.Run("should ignore null actions", func(t *testing.T) {
+		p.Props["attachments"] = []interface{}{
+			map[string]interface{}{"actions": []interface{}{
+				map[string]interface{}{"id": "test1"}, nil, map[string]interface{}{"id": "test2"}, nil, nil},
+			},
+		}
+
+		attachments := p.Attachments()
+		require.Len(t, attachments[0].Actions, 2)
+		require.Equal(t, attachments[0].Actions[0].Id, "test1")
+		require.Equal(t, attachments[0].Actions[1].Id, "test2")
+	})
+}