From 9ee7f661c76d7ef07ac03772a7cf0394217203f3 Mon Sep 17 00:00:00 2001 From: Chris Date: Wed, 12 Jul 2017 06:43:07 -0700 Subject: PLT-7077: ignore null array items in slack attachments (#6904) * ignore null array items in incoming webhooks / command responses * consolidate code, process announcements in command response as well * make a bit more idiomatic, add tests * add missing file --- model/command_response.go | 23 +++++++------------ model/command_response_test.go | 14 ++++++++++++ model/incoming_webhook.go | 52 ++++++++---------------------------------- model/incoming_webhook_test.go | 14 ++++++++++++ model/slack_attachment.go | 52 ++++++++++++++++++++++++++++++++++++++++++ model/slack_attachment_test.go | 38 ++++++++++++++++++++++++++++++ 6 files changed, 135 insertions(+), 58 deletions(-) create mode 100644 model/slack_attachment_test.go (limited to 'model') diff --git a/model/command_response.go b/model/command_response.go index 1b2e06cdf..e3253acc0 100644 --- a/model/command_response.go +++ b/model/command_response.go @@ -5,7 +5,6 @@ package model import ( "encoding/json" - "fmt" "io" ) @@ -15,12 +14,12 @@ const ( ) type CommandResponse struct { - ResponseType string `json:"response_type"` - Text string `json:"text"` - Username string `json:"username"` - IconURL string `json:"icon_url"` - GotoLocation string `json:"goto_location"` - Attachments []*SlackAttachment `json:"attachments"` + ResponseType string `json:"response_type"` + Text string `json:"text"` + Username string `json:"username"` + IconURL string `json:"icon_url"` + GotoLocation string `json:"goto_location"` + Attachments SlackAttachments `json:"attachments"` } func (o *CommandResponse) ToJson() string { @@ -40,14 +39,8 @@ func CommandResponseFromJson(data io.Reader) *CommandResponse { return nil } - // Ensure attachment fields are stored as strings - for _, attachment := range o.Attachments { - for _, field := range attachment.Fields { - if field.Value != nil { - field.Value = fmt.Sprintf("%v", field.Value) - } - } - } + o.Text = ExpandAnnouncement(o.Text) + o.Attachments.Process() return &o } diff --git a/model/command_response_test.go b/model/command_response_test.go index b57a77608..df478ff2c 100644 --- a/model/command_response_test.go +++ b/model/command_response_test.go @@ -82,3 +82,17 @@ func TestCommandResponseFromJson(t *testing.T) { t.Fatal("should've received correct second attachment value") } } + +func TestCommandResponseNullArrayItems(t *testing.T) { + payload := `{"attachments":[{"fields":[{"title":"foo","value":"bar","short":true}, null]}, null]}` + cr := CommandResponseFromJson(strings.NewReader(payload)) + if cr == nil { + t.Fatal("CommandResponse should not be nil") + } + if len(cr.Attachments) != 1 { + t.Fatalf("expected one attachment") + } + if len(cr.Attachments[0].Fields) != 1 { + t.Fatalf("expected one field") + } +} diff --git a/model/incoming_webhook.go b/model/incoming_webhook.go index 2235cb2c6..4ebc7e8b1 100644 --- a/model/incoming_webhook.go +++ b/model/incoming_webhook.go @@ -6,10 +6,8 @@ package model import ( "bytes" "encoding/json" - "fmt" "io" "regexp" - "strings" ) const ( @@ -29,13 +27,13 @@ type IncomingWebhook struct { } type IncomingWebhookRequest struct { - Text string `json:"text"` - Username string `json:"username"` - IconURL string `json:"icon_url"` - ChannelName string `json:"channel"` - Props StringInterface `json:"props"` - Attachments []*SlackAttachment `json:"attachments"` - Type string `json:"type"` + Text string `json:"text"` + Username string `json:"username"` + IconURL string `json:"icon_url"` + ChannelName string `json:"channel"` + Props StringInterface `json:"props"` + Attachments SlackAttachments `json:"attachments"` + Type string `json:"type"` } func (o *IncomingWebhook) ToJson() string { @@ -193,39 +191,6 @@ func decodeIncomingWebhookRequest(by []byte) (*IncomingWebhookRequest, error) { } } -// To mention @channel via a webhook in Slack, the message should contain -// , as explained at the bottom of this article: -// https://get.slack.help/hc/en-us/articles/202009646-Making-announcements -func expandAnnouncement(text string) string { - c1 := "" - c2 := "@channel" - if strings.Contains(text, c1) { - return strings.Replace(text, c1, c2, -1) - } - return text -} - -// Expand announcements in incoming webhooks from Slack. Those announcements -// can be found in the text attribute, or in the pretext, text, title and value -// attributes of the attachment structure. The Slack attachment structure is -// documented here: https://api.slack.com/docs/attachments -func expandAnnouncements(i *IncomingWebhookRequest) { - i.Text = expandAnnouncement(i.Text) - - for _, attachment := range i.Attachments { - attachment.Pretext = expandAnnouncement(attachment.Pretext) - attachment.Text = expandAnnouncement(attachment.Text) - attachment.Title = expandAnnouncement(attachment.Title) - - for _, field := range attachment.Fields { - if field.Value != nil { - // Ensure the value is set to a string if it is set - field.Value = expandAnnouncement(fmt.Sprintf("%v", field.Value)) - } - } - } -} - func IncomingWebhookRequestFromJson(data io.Reader) *IncomingWebhookRequest { buf := new(bytes.Buffer) buf.ReadFrom(data) @@ -241,7 +206,8 @@ func IncomingWebhookRequestFromJson(data io.Reader) *IncomingWebhookRequest { } } - expandAnnouncements(o) + o.Text = ExpandAnnouncement(o.Text) + o.Attachments.Process() return o } diff --git a/model/incoming_webhook_test.go b/model/incoming_webhook_test.go index f6baca988..36f8ed6e6 100644 --- a/model/incoming_webhook_test.go +++ b/model/incoming_webhook_test.go @@ -230,3 +230,17 @@ func TestIncomingWebhookRequestFromJson(t *testing.T) { } } } + +func TestIncomingWebhookNullArrayItems(t *testing.T) { + payload := `{"attachments":[{"fields":[{"title":"foo","value":"bar","short":true}, null]}, null]}` + iwr := IncomingWebhookRequestFromJson(strings.NewReader(payload)) + if iwr == nil { + t.Fatal("IncomingWebhookRequest should not be nil") + } + if len(iwr.Attachments) != 1 { + t.Fatalf("expected one attachment") + } + if len(iwr.Attachments[0].Fields) != 1 { + t.Fatalf("expected one field") + } +} diff --git a/model/slack_attachment.go b/model/slack_attachment.go index 6fd0071b4..a3199c44c 100644 --- a/model/slack_attachment.go +++ b/model/slack_attachment.go @@ -3,6 +3,11 @@ package model +import ( + "fmt" + "strings" +) + type SlackAttachment struct { Id int64 `json:"id"` Fallback string `json:"fallback"` @@ -27,3 +32,50 @@ type SlackAttachmentField struct { Value interface{} `json:"value"` Short bool `json:"short"` } + +type SlackAttachments []*SlackAttachment + +// To mention @channel via a webhook in Slack, the message should contain +// , as explained at the bottom of this article: +// https://get.slack.help/hc/en-us/articles/202009646-Making-announcements +func ExpandAnnouncement(text string) string { + c1 := "" + c2 := "@channel" + if strings.Contains(text, c1) { + return strings.Replace(text, c1, c2, -1) + } + return text +} + +// Expand announcements in incoming webhooks from Slack. Those announcements +// can be found in the text attribute, or in the pretext, text, title and value +// attributes of the attachment structure. The Slack attachment structure is +// documented here: https://api.slack.com/docs/attachments +func (a *SlackAttachments) Process() { + var nonNilAttachments []*SlackAttachment + for _, attachment := range *a { + if attachment == nil { + continue + } + nonNilAttachments = append(nonNilAttachments, attachment) + + attachment.Pretext = ExpandAnnouncement(attachment.Pretext) + attachment.Text = ExpandAnnouncement(attachment.Text) + attachment.Title = ExpandAnnouncement(attachment.Title) + + var nonNilFields []*SlackAttachmentField + for _, field := range attachment.Fields { + if field == nil { + continue + } + nonNilFields = append(nonNilFields, field) + + if field.Value != nil { + // Ensure the value is set to a string if it is set + field.Value = ExpandAnnouncement(fmt.Sprintf("%v", field.Value)) + } + } + attachment.Fields = nonNilFields + } + *a = nonNilAttachments +} diff --git a/model/slack_attachment_test.go b/model/slack_attachment_test.go new file mode 100644 index 000000000..ff4df888f --- /dev/null +++ b/model/slack_attachment_test.go @@ -0,0 +1,38 @@ +package model + +import ( + "testing" +) + +func TestExpandAnnouncement(t *testing.T) { + if ExpandAnnouncement(" foo ") != "@channel foo @channel" { + t.Fail() + } +} + +func TestSlackAnnouncementProcess(t *testing.T) { + attachments := SlackAttachments{ + { + Pretext: " pretext", + Text: " text", + Title: " title", + Fields: []*SlackAttachmentField{ + { + Title: "foo", + Value: " bar", + Short: true, + }, nil, + }, + }, nil, + } + attachments.Process() + if len(attachments) != 1 || len(attachments[0].Fields) != 1 { + t.Fail() + } + if attachments[0].Pretext != "@channel pretext" || + attachments[0].Text != "@channel text" || + attachments[0].Title != "@channel title" || + attachments[0].Fields[0].Value != "@channel bar" { + t.Fail() + } +} -- cgit v1.2.3-1-g7c22