From 5580c28e54015b78893c33cc3bf41da75712c4cb Mon Sep 17 00:00:00 2001 From: Thomas Balthazar Date: Tue, 17 May 2016 14:56:38 +0200 Subject: 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 --- api/webhook_test.go | 11 ++++++ model/incoming_webhook.go | 82 +++++++++++++++++++++++++++++++++++++++--- model/incoming_webhook_test.go | 72 +++++++++++++++++++++++++++++++++++++ 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"]) + } + } +} -- cgit v1.2.3-1-g7c22