Return 400 bad request codes for webhooks when attachment or text is too long (#4879)

2 files changed, 67 insertions, 2 deletions

diff --git a/api/webhook.go b/api/webhook.go
index dce739239..b164d0ae7 100644
--- a/api/webhook.go
+++ b/api/webhook.go
@@ -7,6 +7,7 @@ import (
 	"io"
 	"net/http"
 	"strings"
+	"unicode/utf8"
 
 	l4g "github.com/alecthomas/log4go"
 	"github.com/gorilla/mux"
@@ -387,18 +388,35 @@ func incomingWebhook(c *Context, w http.ResponseWriter, r *http.Request) {
 	text := parsedRequest.Text
 	if len(text) == 0 && parsedRequest.Attachments == nil {
 		c.Err = model.NewLocAppError("incomingWebhook", "web.incoming_webhook.text.app_error", nil, "")
+		c.Err.StatusCode = http.StatusBadRequest
+		return
+	}
+
+	textSize := utf8.RuneCountInString(text)
+	if textSize > model.POST_MESSAGE_MAX_RUNES {
+		c.Err = model.NewLocAppError("incomingWebhook", "web.incoming_webhook.text.length.app_error", map[string]interface{}{"Max": model.POST_MESSAGE_MAX_RUNES, "Actual": textSize}, "")
+		c.Err.StatusCode = http.StatusBadRequest
 		return
 	}
 
 	channelName := parsedRequest.ChannelName
 	webhookType := parsedRequest.Type
 
-	//attachments is in here for slack compatibility
+	// attachments is in here for slack compatibility
 	if parsedRequest.Attachments != nil {
 		if len(parsedRequest.Props) == 0 {
 			parsedRequest.Props = make(model.StringInterface)
 		}
 		parsedRequest.Props["attachments"] = parsedRequest.Attachments
+
+		attachmentSize := utf8.RuneCountInString(model.StringInterfaceToJson(parsedRequest.Props))
+		// Minus 100 to leave room for setting post type in the Props
+		if attachmentSize > model.POST_PROPS_MAX_RUNES-100 {
+			c.Err = model.NewLocAppError("incomingWebhook", "web.incoming_webhook.attachment.app_error", map[string]interface{}{"Max": model.POST_PROPS_MAX_RUNES - 100, "Actual": attachmentSize}, "")
+			c.Err.StatusCode = http.StatusBadRequest
+			return
+		}
+
 		webhookType = model.POST_SLACK_ATTACHMENT
 	}
 
diff --git a/api/webhook_test.go b/api/webhook_test.go
index ae3b48032..6daa0c334 100644
--- a/api/webhook_test.go
+++ b/api/webhook_test.go
@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"github.com/mattermost/platform/model"
 	"github.com/mattermost/platform/utils"
+	"net/http"
 	"testing"
 )
 
@@ -646,10 +647,56 @@ func TestIncomingWebhooks(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	if _, err := Client.DoPost(url, "{\"text\":\"\"}", "application/json"); err == nil {
+	if _, err := Client.DoPost(url, "{\"text\":\"\"}", "application/json"); err == nil || err.StatusCode != http.StatusBadRequest {
 		t.Fatal("should have failed - no text")
 	}
 
+	tooLongText := ""
+	for i := 0; i < 8200; i++ {
+		tooLongText += "a"
+	}
+
+	if _, err := Client.DoPost(url, "{\"text\":\""+tooLongText+"\"}", "application/json"); err == nil || err.StatusCode != http.StatusBadRequest {
+		t.Fatal("should have failed - text too long")
+	}
+
+	attachmentPayload = `{
+        "text": "this is a test",
+        "attachments": [
+            {
+                "fallback": "Required plain-text summary of the attachment.",
+
+                "color": "#36a64f",
+
+                "pretext": "Optional text that appears above the attachment block",
+
+                "author_name": "Bobby Tables",
+                "author_link": "http://flickr.com/bobby/",
+                "author_icon": "http://flickr.com/icons/bobby.jpg",
+
+                "title": "Slack API Documentation",
+                "title_link": "https://api.slack.com/",
+
+                "text": "` + tooLongText + `",
+
+                "fields": [
+                    {
+                        "title": "Priority",
+                        "value": "High",
+                        "short": false
+                    }
+                ],
+
+                "image_url": "http://my-website.com/path/to/image.jpg",
+                "thumb_url": "http://example.com/path/to/thumb.png"
+            }
+        ]
+    }`
+
+	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")
+	}
+
 	utils.Cfg.ServiceSettings.EnableIncomingWebhooks = false
 
 	if _, err := Client.DoPost(url, "{\"text\":\"this is a test\"}", "application/json"); err == nil {