summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorChris <ccbrown112@gmail.com>2017-10-24 18:36:31 -0500
committerJoram Wilander <jwawilander@gmail.com>2017-10-24 19:36:31 -0400
commit9c0575ce6ef662c18ad7eb91bf6084c6fac1b7ae (patch)
tree635ba5631b382c1e69f9719c6131ce74428b69ed
parent61b023f5dfe796c7ba8183292853f01817016d14 (diff)
downloadchat-9c0575ce6ef662c18ad7eb91bf6084c6fac1b7ae.tar.gz
chat-9c0575ce6ef662c18ad7eb91bf6084c6fac1b7ae.tar.bz2
chat-9c0575ce6ef662c18ad7eb91bf6084c6fac1b7ae.zip
PLT-7599: webhook post splitting (#7707)
* webhook post splitting * style fix * update old webhook test
-rw-r--r--api/webhook_test.go4
-rw-r--r--app/webhook.go118
-rw-r--r--app/webhook_test.go96
-rw-r--r--i18n/en.json4
-rw-r--r--model/post.go1
5 files changed, 187 insertions, 36 deletions
diff --git a/api/webhook_test.go b/api/webhook_test.go
index 285650b5c..68dc4f4e7 100644
--- a/api/webhook_test.go
+++ b/api/webhook_test.go
@@ -1065,8 +1065,8 @@ func TestIncomingWebhooks(t *testing.T) {
]
}`
- if _, err := Client.DoPost(url, attachmentPayload, "application/json"); err == nil || err.StatusCode != http.StatusBadRequest {
- t.Fatal("should have failed with bad request - attachment too long")
+ if _, err := Client.DoPost(url, attachmentPayload, "application/json"); err != nil {
+ t.Fatal(err)
}
th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.EnableIncomingWebhooks = false })
diff --git a/app/webhook.go b/app/webhook.go
index e8f9936f8..56f1f0875 100644
--- a/app/webhook.go
+++ b/app/webhook.go
@@ -123,6 +123,86 @@ func (a *App) TriggerWebhook(payload *model.OutgoingWebhookPayload, hook *model.
}
}
+func SplitWebhookPost(post *model.Post) ([]*model.Post, *model.AppError) {
+ splits := make([]*model.Post, 0)
+ remainingText := post.Message
+
+ base := *post
+ base.Message = ""
+ base.Props = make(map[string]interface{})
+ for k, v := range post.Props {
+ if k != "attachments" {
+ base.Props[k] = v
+ }
+ }
+ if utf8.RuneCountInString(model.StringInterfaceToJson(base.Props)) > model.POST_PROPS_MAX_USER_RUNES {
+ return nil, model.NewAppError("SplitWebhookPost", "web.incoming_webhook.split_props_length.app_error", map[string]interface{}{"Max": model.POST_PROPS_MAX_USER_RUNES}, "", http.StatusBadRequest)
+ }
+
+ for utf8.RuneCountInString(remainingText) > model.POST_MESSAGE_MAX_RUNES {
+ split := base
+ x := 0
+ for index := range remainingText {
+ x++
+ if x > model.POST_MESSAGE_MAX_RUNES {
+ split.Message = remainingText[:index]
+ remainingText = remainingText[index:]
+ break
+ }
+ }
+ splits = append(splits, &split)
+ }
+
+ split := base
+ split.Message = remainingText
+ splits = append(splits, &split)
+
+ attachments, _ := post.Props["attachments"].([]*model.SlackAttachment)
+ for _, attachment := range attachments {
+ newAttachment := *attachment
+ for {
+ lastSplit := splits[len(splits)-1]
+ newProps := make(map[string]interface{})
+ for k, v := range lastSplit.Props {
+ newProps[k] = v
+ }
+ origAttachments, _ := newProps["attachments"].([]*model.SlackAttachment)
+ newProps["attachments"] = append(origAttachments, &newAttachment)
+ newPropsString := model.StringInterfaceToJson(newProps)
+ runeCount := utf8.RuneCountInString(newPropsString)
+
+ if runeCount <= model.POST_PROPS_MAX_USER_RUNES {
+ lastSplit.Props = newProps
+ break
+ }
+
+ if len(origAttachments) > 0 {
+ newSplit := base
+ splits = append(splits, &newSplit)
+ continue
+ }
+
+ truncationNeeded := runeCount - model.POST_PROPS_MAX_USER_RUNES
+ textRuneCount := utf8.RuneCountInString(attachment.Text)
+ if textRuneCount < truncationNeeded {
+ return nil, model.NewAppError("SplitWebhookPost", "web.incoming_webhook.split_props_length.app_error", map[string]interface{}{"Max": model.POST_PROPS_MAX_USER_RUNES}, "", http.StatusBadRequest)
+ }
+ x := 0
+ for index := range attachment.Text {
+ x++
+ if x > textRuneCount-truncationNeeded {
+ newAttachment.Text = newAttachment.Text[:index]
+ break
+ }
+ }
+ lastSplit.Props = newProps
+ break
+ }
+ }
+
+ return splits, nil
+}
+
func (a *App) CreateWebhookPost(userId string, channel *model.Channel, text, overrideUsername, overrideIconUrl string, props model.StringInterface, postType string) (*model.Post, *model.AppError) {
// parse links into Markdown format
linkWithTextRegex := regexp.MustCompile(`<([^<\|]+)\|([^>]+)>`)
@@ -166,37 +246,18 @@ func (a *App) CreateWebhookPost(userId string, channel *model.Channel, text, ove
}
}
- splits := make([]string, 0)
- remainingText := post.Message
-
- for len(remainingText) > model.POST_MESSAGE_MAX_RUNES {
- splits = append(splits, remainingText[:model.POST_MESSAGE_MAX_RUNES])
- remainingText = remainingText[model.POST_MESSAGE_MAX_RUNES:]
+ splits, err := SplitWebhookPost(post)
+ if err != nil {
+ return nil, err
}
- splits = append(splits, remainingText)
-
- var firstPost *model.Post = nil
-
- for _, txt := range splits {
- post.Id = ""
- post.UpdateAt = 0
- post.CreateAt = 0
- post.Message = txt
- if _, err := a.CreatePostMissingChannel(post, false); err != nil {
+ for _, split := range splits {
+ if _, err := a.CreatePostMissingChannel(split, false); err != nil {
return nil, model.NewAppError("CreateWebhookPost", "api.post.create_webhook_post.creating.app_error", nil, "err="+err.Message, http.StatusInternalServerError)
}
-
- if firstPost == nil {
- if len(splits) > 1 {
- firstPost = model.PostFromJson(strings.NewReader(post.ToJson()))
- } else {
- firstPost = post
- }
- }
}
- return firstPost, nil
+ return splits[0], nil
}
func (a *App) CreateIncomingWebhookForChannel(creatorId string, channel *model.Channel, hook *model.IncomingWebhook) (*model.IncomingWebhook, *model.AppError) {
@@ -482,13 +543,6 @@ func (a *App) HandleIncomingWebhook(hookId string, req *model.IncomingWebhookReq
req.Props = make(model.StringInterface)
}
req.Props["attachments"] = req.Attachments
-
- attachmentSize := utf8.RuneCountInString(model.StringInterfaceToJson(req.Props))
- // Minus 100 to leave room for setting post type in the Props
- if attachmentSize > model.POST_PROPS_MAX_RUNES-100 {
- return model.NewAppError("HandleIncomingWebhook", "web.incoming_webhook.attachment.app_error", map[string]interface{}{"Max": model.POST_PROPS_MAX_RUNES - 100, "Actual": attachmentSize}, "", http.StatusBadRequest)
- }
-
webhookType = model.POST_SLACK_ATTACHMENT
}
diff --git a/app/webhook_test.go b/app/webhook_test.go
index d6293e2b7..8dc90b49b 100644
--- a/app/webhook_test.go
+++ b/app/webhook_test.go
@@ -4,8 +4,12 @@
package app
import (
+ "strings"
"testing"
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+
"github.com/mattermost/mattermost-server/model"
"github.com/mattermost/mattermost-server/utils"
)
@@ -50,3 +54,95 @@ func TestCreateWebhookPost(t *testing.T) {
t.Fatal("should have failed - bad post type")
}
}
+
+func TestSplitWebhookPost(t *testing.T) {
+ type TestCase struct {
+ Post *model.Post
+ Expected []*model.Post
+ }
+
+ for name, tc := range map[string]TestCase{
+ "LongPost": {
+ Post: &model.Post{
+ Message: strings.Repeat("本", model.POST_MESSAGE_MAX_RUNES*3/2),
+ },
+ Expected: []*model.Post{
+ {
+ Message: strings.Repeat("本", model.POST_MESSAGE_MAX_RUNES),
+ },
+ {
+ Message: strings.Repeat("本", model.POST_MESSAGE_MAX_RUNES/2),
+ },
+ },
+ },
+ "LongPostAndMultipleAttachments": {
+ Post: &model.Post{
+ Message: strings.Repeat("本", model.POST_MESSAGE_MAX_RUNES*3/2),
+ Props: map[string]interface{}{
+ "attachments": []*model.SlackAttachment{
+ &model.SlackAttachment{
+ Text: strings.Repeat("本", 1000),
+ },
+ &model.SlackAttachment{
+ Text: strings.Repeat("本", 2000),
+ },
+ &model.SlackAttachment{
+ Text: strings.Repeat("本", model.POST_PROPS_MAX_USER_RUNES-1000),
+ },
+ },
+ },
+ },
+ Expected: []*model.Post{
+ {
+ Message: strings.Repeat("本", model.POST_MESSAGE_MAX_RUNES),
+ },
+ {
+ Message: strings.Repeat("本", model.POST_MESSAGE_MAX_RUNES/2),
+ Props: map[string]interface{}{
+ "attachments": []*model.SlackAttachment{
+ &model.SlackAttachment{
+ Text: strings.Repeat("本", 1000),
+ },
+ &model.SlackAttachment{
+ Text: strings.Repeat("本", 2000),
+ },
+ },
+ },
+ },
+ {
+ Props: map[string]interface{}{
+ "attachments": []*model.SlackAttachment{
+ &model.SlackAttachment{
+ Text: strings.Repeat("本", model.POST_PROPS_MAX_USER_RUNES-1000),
+ },
+ },
+ },
+ },
+ },
+ },
+ "UnsplittableProps": {
+ Post: &model.Post{
+ Message: "foo",
+ Props: map[string]interface{}{
+ "foo": strings.Repeat("x", model.POST_PROPS_MAX_USER_RUNES*2),
+ },
+ },
+ },
+ } {
+ t.Run(name, func(t *testing.T) {
+ splits, err := SplitWebhookPost(tc.Post)
+ if tc.Expected == nil {
+ require.NotNil(t, err)
+ } else {
+ require.Nil(t, err)
+ }
+ assert.Equal(t, len(tc.Expected), len(splits))
+ for i, split := range splits {
+ if i < len(tc.Expected) {
+ assert.Equal(t, tc.Expected[i].Message, split.Message)
+ assert.Equal(t, tc.Expected[i].Props["attachments"], split.Props["attachments"])
+ }
+ }
+ })
+ }
+}
diff --git a/i18n/en.json b/i18n/en.json
index 7a8c8f4c1..e20edf2f2 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -6860,8 +6860,8 @@
"translation": "Back"
},
{
- "id": "web.incoming_webhook.attachment.app_error",
- "translation": "Maximum attachments length is {{.Max}} characters, received size is {{.Actual}}"
+ "id": "web.incoming_webhook.split_props_length.app_error",
+ "translation": "Unable to split webhook props into {{.Max}} character parts."
},
{
"id": "web.incoming_webhook.channel.app_error",
diff --git a/model/post.go b/model/post.go
index a5ae0b82a..a7be63432 100644
--- a/model/post.go
+++ b/model/post.go
@@ -32,6 +32,7 @@ const (
POST_HASHTAGS_MAX_RUNES = 1000
POST_MESSAGE_MAX_RUNES = 4000
POST_PROPS_MAX_RUNES = 8000
+ POST_PROPS_MAX_USER_RUNES = POST_PROPS_MAX_RUNES - 400 // Leave some room for system / pre-save modifications
POST_CUSTOM_TYPE_PREFIX = "custom_"
PROPS_ADD_CHANNEL_MEMBER = "add_channel_member"
)