PLT-2188 Integrations: Support raw new lines in the text payload (#2993)

* Integrations: Support raw new lines in the text payload

* Improve support for raw new lines in text payload

The regexp used to escape control characters now also searches for
additional fields:
text|fallback|pretext|author_name|title|value

3 files changed, 161 insertions, 4 deletions

diff --git a/api/webhook_test.go b/api/webhook_test.go
index 5198056cc..1b13bb5d4 100644
--- a/api/webhook_test.go
+++ b/api/webhook_test.go
@@ -511,6 +511,7 @@ func TestRegenOutgoingHookToken(t *testing.T) {
 		t.Fatal("should have errored - webhooks turned off")
 	}
 }
+
 func TestIncomingWebhooks(t *testing.T) {
 	th := Setup().InitSystemAdmin()
 	Client := th.SystemAdminClient
@@ -529,11 +530,17 @@ func TestIncomingWebhooks(t *testing.T) {
 	hook = Client.Must(Client.CreateIncomingWebhook(hook)).Data.(*model.IncomingWebhook)
 
 	url := "/hooks/" + hook.Id
+	text := `this is a \"test\"
+	that contains a newline and a tab`
 
 	if _, err := Client.DoPost(url, "{\"text\":\"this is a test\"}", "application/json"); err != nil {
 		t.Fatal(err)
 	}
 
+	if _, err := Client.DoPost(url, "{\"text\":\""+text+"\"}", "application/json"); err != nil {
+		t.Fatal(err)
+	}
+
 	if _, err := Client.DoPost(url, fmt.Sprintf("{\"text\":\"this is a test\", \"channel\":\"%s\"}", channel1.Name), "application/json"); err != nil {
 		t.Fatal(err)
 	}
@@ -552,6 +559,10 @@ func TestIncomingWebhooks(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	if _, err := Client.DoPost(url, "payload={\"text\":\""+text+"\"}", "application/x-www-form-urlencoded"); err != nil {
+		t.Fatal(err)
+	}
+
 	attachmentPayload := `{
         "text": "this is a test",
         "attachments": [
diff --git a/model/incoming_webhook.go b/model/incoming_webhook.go
index 0763b443e..4bad8b262 100644
--- a/model/incoming_webhook.go
+++ b/model/incoming_webhook.go
@@ -4,8 +4,10 @@
 package model
 
 import (
+	"bytes"
 	"encoding/json"
 	"io"
+	"regexp"
 )
 
 const (
@@ -125,13 +127,85 @@ func (o *IncomingWebhook) PreUpdate() {
 	o.UpdateAt = GetMillis()
 }
 
-func IncomingWebhookRequestFromJson(data io.Reader) *IncomingWebhookRequest {
-	decoder := json.NewDecoder(data)
+// escapeControlCharsFromPayload escapes control chars (\n, \t) from a byte slice.
+// Context:
+// JSON strings are not supposed to contain control characters such as \n, \t,
+// ... but some incoming webhooks might still send invalid JSON and we want to
+// try to handle that. An example invalid JSON string from an incoming webhook
+// might look like this (strings for both "text" and "fallback" attributes are
+// invalid JSON strings because they contain unescaped newlines and tabs):
+//  `{
+//    "text": "this is a test
+//						 that contains a newline and tabs",
+//    "attachments": [
+//      {
+//        "fallback": "Required plain-text summary of the attachment
+//										that contains a newline and tabs",
+//        "color": "#36a64f",
+//  			...
+//        "text": "Optional text that appears within the attachment
+//								 that contains a newline and tabs",
+//  			...
+//        "thumb_url": "http://example.com/path/to/thumb.png"
+//      }
+//    ]
+//  }`
+// This function will search for `"key": "value"` pairs, and escape \n, \t
+// from the value.
+func escapeControlCharsFromPayload(by []byte) []byte {
+	// we'll search for `"text": "..."` or `"fallback": "..."`, ...
+	keys := "text|fallback|pretext|author_name|title|value"
+
+	// the regexp reads like this:
+	// (?s): this flag let . match \n (default is false)
+	// "(keys)": we search for the keys defined above
+	// \s*:\s*: followed by 0..n spaces/tabs, a colon then 0..n spaces/tabs
+	// ": a double-quote
+	// (\\"|[^"])*: any number of times the `\"` string or any char but a double-quote
+	// ": a double-quote
+	r := `(?s)"(` + keys + `)"\s*:\s*"(\\"|[^"])*"`
+	re := regexp.MustCompile(r)
+
+	// the function that will escape \n and \t on the regexp matches
+	repl := func(b []byte) []byte {
+		if bytes.Contains(b, []byte("\n")) {
+			b = bytes.Replace(b, []byte("\n"), []byte("\\n"), -1)
+		}
+		if bytes.Contains(b, []byte("\t")) {
+			b = bytes.Replace(b, []byte("\t"), []byte("\\t"), -1)
+		}
+
+		return b
+	}
+
+	return re.ReplaceAllFunc(by, repl)
+}
+
+func decodeIncomingWebhookRequest(by []byte) (*IncomingWebhookRequest, error) {
+	decoder := json.NewDecoder(bytes.NewReader(by))
 	var o IncomingWebhookRequest
 	err := decoder.Decode(&o)
 	if err == nil {
-		return &o
+		return &o, nil
 	} else {
-		return nil
+		return nil, err
 	}
 }
+
+func IncomingWebhookRequestFromJson(data io.Reader) *IncomingWebhookRequest {
+	buf := new(bytes.Buffer)
+	buf.ReadFrom(data)
+	by := buf.Bytes()
+
+	// Try to decode the JSON data. Only if it fails, try to escape control
+	// characters from the strings contained in the JSON data.
+	o, err := decodeIncomingWebhookRequest(by)
+	if err != nil {
+		o, err = decodeIncomingWebhookRequest(escapeControlCharsFromPayload(by))
+		if err != nil {
+			return nil
+		}
+	}
+
+	return o
+}
diff --git a/model/incoming_webhook_test.go b/model/incoming_webhook_test.go
index 3f1754244..8b62bbbdf 100644
--- a/model/incoming_webhook_test.go
+++ b/model/incoming_webhook_test.go
@@ -100,3 +100,75 @@ func TestIncomingWebhookPreUpdate(t *testing.T) {
 	o := IncomingWebhook{}
 	o.PreUpdate()
 }
+
+func TestIncomingWebhookRequestFromJson(t *testing.T) {
+	texts := []string{
+		`this is a test`,
+		`this is a test
+			that contains a newline and tabs`,
+		`this is a test \"foo
+			that contains a newline and tabs`,
+		`this is a test \"foo\"
+			that contains a newline and tabs`,
+		`this is a test \"foo\"
+		\"			that contains a newline and tabs`,
+		`this is a test \"foo\"
+
+		\"			that contains a newline and tabs
+		`,
+	}
+
+	for i, text := range texts {
+		// build a sample payload with the text
+		payload := `{
+        "text": "` + text + `",
+        "attachments": [
+            {
+                "fallback": "` + text + `",
+
+                "color": "#36a64f",
+
+                "pretext": "` + text + `",
+
+                "author_name": "` + text + `",
+                "author_link": "http://flickr.com/bobby/",
+                "author_icon": "http://flickr.com/icons/bobby.jpg",
+
+                "title": "` + text + `",
+                "title_link": "https://api.slack.com/",
+
+                "text": "` + text + `",
+
+                "fields": [
+                    {
+                        "title": "` + text + `",
+                        "value": "` + text + `",
+                        "short": false
+                    }
+                ],
+
+                "image_url": "http://my-website.com/path/to/image.jpg",
+                "thumb_url": "http://example.com/path/to/thumb.png"
+            }
+        ]
+    }`
+
+		// try to create an IncomingWebhookRequest from the payload
+		data := strings.NewReader(payload)
+		iwr := IncomingWebhookRequestFromJson(data)
+
+		// After it has been decoded, the JSON string won't contain the escape char anymore
+		expected := strings.Replace(text, `\"`, `"`, -1)
+		if iwr == nil {
+			t.Fatal("IncomingWebhookRequest should not be nil")
+		}
+		if iwr.Text != expected {
+			t.Fatalf("Sample %d text should be: %s, got: %s", i, expected, iwr.Text)
+		}
+		attachments := iwr.Attachments.([]interface{})
+		attachment := attachments[0].(map[string]interface{})
+		if attachment["text"] != expected {
+			t.Fatalf("Sample %d attachment text should be: %s, got: %s", i, expected, attachment["text"])
+		}
+	}
+}