Repository: adnanh/webhook
Pull request #373 (2019-12-23 22:09:22)

Add multipart form data support
Please take a look.  See #372 for proposal discussion.

TODO:

- [x] Tests
- [x] Docs
- [x] Examples

adnanh: @moorereason This looks great, can you just resolve conflicts?
moorereason: Rebased.  :+1:
adnanh: Looks like #376 messed it up again, oops

Patch:
diff --git a/README.md b/README.md
index 23f00f4..083c36f 100644
--- a/README.md
+++ b/README.md
@@ -77,6 +77,18 @@ By performing a simple HTTP GET or POST request to that endpoint, your specified
 
 However, hook defined like that could pose a security threat to your system, because anyone who knows your endpoint, can send a request and execute your command. To prevent that, you can use the `"trigger-rule"` property for your hook, to specify the exact circumstances under which the hook would be triggered. For example, you can use them to add a secret that you must supply as a parameter in order to successfully trigger the hook. Please check out the [Hook rules page](docs/Hook-Rules.md) for detailed list of available rules and their  usage.
 
+## Multipart Form Data
+[webhook][w] provides limited support the parsing of multipart form data.
+Multipart form data can contain two types of parts: values and files.
+All form _values_ are automatically added to the `payload` scope.
+Use the `parse-parameters-as-json` settings to parse a given value as JSON.
+All files are ignored unless they match one of the following criteria:
+
+1. The `Content-Type` header is `application/json`.
+1. The part is named in the `parse-parameters-as-json` setting.
+
+In either case, the given file part will be parsed as JSON and added to the `payload` map.
+
 ## Templates
 [webhook][w] can parse the `hooks.json` input file as a Go template when given the `-template` [CLI parameter](docs/Webhook-Parameters.md). See the [Templates page](docs/Templates.md) for more details on template usage.
 
diff --git a/docs/Hook-Examples.md b/docs/Hook-Examples.md
index 1f0a031..23896a7 100644
--- a/docs/Hook-Examples.md
+++ b/docs/Hook-Examples.md
@@ -287,12 +287,12 @@ __Not recommended in production due to low security__
 ]
 ```
 
-# JIRA Webhooks
+## JIRA Webhooks
 [Guide by @perfecto25](https://sites.google.com/site/mrxpalmeiras/notes/jira-webhooks)
 
-# Pass File-to-command sample
+## Pass File-to-command sample
 
-## Webhook configuration
+### Webhook configuration
 
 <pre>
 [
@@ -315,7 +315,7 @@ __Not recommended in production due to low security__
 ]
 </pre>
 
-## Sample client usage 
+### Sample client usage 
 
 Store the following file as `testRequest.json`. 
 
@@ -474,3 +474,47 @@ Given the following payload:
   }
 ]
 ```
+
+## Multipart Form Data
+
+Example of a [Plex Media Server webhook](https://support.plex.tv/articles/115002267687-webhooks/).
+The Plex Media Server will send two parts: payload and thumb.
+We only care about the payload part.
+
+```json
+[
+  {
+    "id": "plex",
+    "execute-command": "play-command.sh",
+    "parse-parameters-as-json": [
+      {
+        "source": "payload",
+        "name": "payload"
+      }
+    ],
+    "trigger-rule":
+    {
+      "match":
+      {
+        "type": "value",
+        "parameter": {
+          "source": "payload",
+          "name": "payload.event"
+        },
+        "value": "media.play"
+      }
+    }
+  }
+]
+
+```
+
+Each part of a multipart form data body will have a `Content-Disposition` header.
+Some example headers:
+
+```
+Content-Disposition: form-data; name="payload"
+Content-Disposition: form-data; name="thumb"; filename="thumb.jpg"
+```
+
+We key off of the `name` attribute in the `Content-Disposition` value.
diff --git a/webhook.go b/webhook.go
index cce1fbd..ca53a7d 100644
--- a/webhook.go
+++ b/webhook.go
@@ -46,6 +46,7 @@ var (
 	tlsCipherSuites    = flag.String("cipher-suites", "", "comma-separated list of supported TLS cipher suites")
 	useXRequestID      = flag.Bool("x-request-id", false, "use X-Request-Id header, if present, as request ID")
 	xRequestIDLimit    = flag.Int("x-request-id-limit", 0, "truncate X-Request-Id header to limit; default no limit")
+	maxMultipartMem    = flag.Int64("max-multipart-mem", 1<<20, "maximum memory in bytes for parsing multipart form data before disk caching")
 
 	responseHeaders hook.ResponseHeaders
 	hooksFiles      hook.HooksFiles
@@ -230,9 +231,24 @@ func hookHandler(w http.ResponseWriter, r *http.Request) {
 	if matchedHook := matchLoadedHook(id); matchedHook != nil {
 		log.Printf("[%s] %s got matched\n", rid, id)
 
-		body, err := ioutil.ReadAll(r.Body)
-		if err != nil {
-			log.Printf("[%s] error reading the request body. %+v\n", rid, err)
+		var (
+			body []byte
+			err  error
+		)
+
+		// set contentType to IncomingPayloadContentType or header value
+		contentType := r.Header.Get("Content-Type")
+		if len(matchedHook.IncomingPayloadContentType) != 0 {
+			contentType = matchedHook.IncomingPayloadContentType
+		}
+
+		isMultipart := strings.HasPrefix(contentType, "multipart/form-data;")
+
+		if !isMultipart {
+			body, err = ioutil.ReadAll(r.Body)
+			if err != nil {
+				log.Printf("[%s] error reading the request body: %+v\n", rid, err)
+			}
 		}
 
 		// parse headers
@@ -244,21 +260,16 @@ func hookHandler(w http.ResponseWriter, r *http.Request) {
 		// parse body
 		var payload map[string]interface{}
 
-		// set contentType to IncomingPayloadContentType or header value
-		contentType := r.Header.Get("Content-Type")
-		if len(matchedHook.IncomingPayloadContentType) != 0 {
-			contentType = matchedHook.IncomingPayloadContentType
-		}
-
 		switch {
 		case strings.Contains(contentType, "json"):
-			decoder := json.NewDecoder(strings.NewReader(string(body)))
+			decoder := json.NewDecoder(bytes.NewReader(body))
 			decoder.UseNumber()
 
 			err := decoder.Decode(&payload)
 			if err != nil {
 				log.Printf("[%s] error parsing JSON payload %+v\n", rid, err)
 			}
+
 		case strings.Contains(contentType, "x-www-form-urlencoded"):
 			fd, err := url.ParseQuery(string(body))
 			if err != nil {
@@ -266,11 +277,87 @@ func hookHandler(w http.ResponseWriter, r *http.Request) {
 			} else {
 				payload = valuesToMap(fd)
 			}
+
 		case strings.Contains(contentType, "xml"):
 			payload, err = mxj.NewMapXmlReader(bytes.NewReader(body))
 			if err != nil {
 				log.Printf("[%s] error parsing XML payload: %+v\n", rid, err)
 			}
+
+		case isMultipart:
+			err = r.ParseMultipartForm(*maxMultipartMem)
+			if err != nil {
+				msg := fmt.Sprintf("[%s] error parsing multipart form: %+v\n", rid, err)
+				log.Println(msg)
+				w.WriteHeader(http.StatusInternalServerError)
+				fmt.Fprint(w, "Error occurred while parsing multipart form.")
+				return
+			}
+
+			for k, v := range r.MultipartForm.Value {
+				log.Printf("[%s] found multipart form value %q", rid, k)
+
+				if payload == nil {
+					payload = make(map[string]interface{})
+				}
+
+				// TODO(moorereason): support duplicate, named values
+				payload[k] = v[0]
+			}
+
+			for k, v := range r.MultipartForm.File {
+				// Force parsing as JSON regardless of Content-Type.
+				var parseAsJSON bool
+				for _, j := range matchedHook.JSONStringParameters {
+					if j.Source == "payload" && j.Name == k {
+						parseAsJSON = true
+						break
+					}
+				}
+
+				// TODO(moorereason): we need to support multiple parts
+				// with the same name instead of just processing the first
+				// one. Will need #215 resolved first.
+
+				// MIME encoding can contain duplicate headers, so check them
+				// all.
+				if !parseAsJSON && len(v[0].Header["Content-Type"]) > 0 {
+					for _, j := range v[0].Header["Content-Type"] {
+						if j == "application/json" {
+							parseAsJSON = true
+							break
+						}
+					}
+				}
+
+				if parseAsJSON {
+					log.Printf("[%s] parsing multipart form file %q as JSON\n", rid, k)
+
+					f, err := v[0].Open()
+					if err != nil {
+						msg := fmt.Sprintf("[%s] error parsing multipart form file: %+v\n", rid, err)
+						log.Println(msg)
+						w.WriteHeader(http.StatusInternalServerError)
+						fmt.Fprint(w, "Error occurred while parsing multipart form file.")
+						return
+					}
+
+					decoder := json.NewDecoder(f)
+					decoder.UseNumber()
+
+					var part map[string]interface{}
+					err = decoder.Decode(&part)
+					if err != nil {
+						log.Printf("[%s] error parsing JSON payload file: %+v\n", rid, err)
+					}
+
+					if payload == nil {
+						payload = make(map[string]interface{})
+					}
+					payload[k] = part
+				}
+			}
+
 		default:
 			log.Printf("[%s] error parsing body payload due to unsupported content type header: %s\n", rid, contentType)
 		}


Test patch:
diff --git a/test/hooks.json.tmpl b/test/hooks.json.tmpl
index 9661472..40c6de5 100644
--- a/test/hooks.json.tmpl
+++ b/test/hooks.json.tmpl
@@ -167,6 +167,30 @@
       ],
     }
   },
+  {
+    "id": "plex",
+    "execute-command": "{{ .Hookecho }}",
+    "command-working-directory": "/",
+    "response-message": "success",
+    "parse-parameters-as-json": [
+      {
+        "source": "payload",
+        "name": "payload"
+      }
+    ],
+    "trigger-rule":
+    {
+      "match":
+      {
+        "type": "value",
+        "parameter": {
+          "source": "payload",
+          "name": "payload.event"
+        },
+        "value": "media.play"
+      }
+    }
+  },
   {
     "id": "capture-command-output-on-success-not-by-default",
     "pass-arguments-to-command": [
diff --git a/test/hooks.yaml.tmpl b/test/hooks.yaml.tmpl
index cf90471..bfeacc2 100644
--- a/test/hooks.yaml.tmpl
+++ b/test/hooks.yaml.tmpl
@@ -95,6 +95,21 @@
           name: "app.messages.message.#text"
         value: "Hello!!"
 
+- id: plex
+  trigger-rule:
+    match:
+      type: value
+      parameter:
+        source: payload
+        name: payload.event
+      value: media.play
+  parse-parameters-as-json:
+  - source: payload
+    name: payload
+  execute-command: '{{ .Hookecho }}'
+  response-message: success
+  command-working-directory: /
+
 - id: capture-command-output-on-success-not-by-default
   pass-arguments-to-command:
   - source: string
diff --git a/webhook_test.go b/webhook_test.go
index adcfd3c..f7ba36b 100644
--- a/webhook_test.go
+++ b/webhook_test.go
@@ -75,7 +75,7 @@ func TestWebhook(t *testing.T) {
 		defer cleanupConfigFn()
 
 		for _, tt := range hookHandlerTests {
-			t.Run(tt.desc, func(t *testing.T) {
+			t.Run(tt.desc+"@"+hookTmpl, func(t *testing.T) {
 				ip, port := serverAddress(t)
 				args := []string{fmt.Sprintf("-hooks=%s", configPath), fmt.Sprintf("-ip=%s", ip), fmt.Sprintf("-port=%s", port), "-verbose"}
 
@@ -106,13 +106,7 @@ func TestWebhook(t *testing.T) {
 
 				var res *http.Response
 
-				if tt.urlencoded == true {
-					req.Header.Add("Content-Type", "application/x-www-form-urlencoded")
-				} else {
-					if req.Header.Get("Content-Type") == "" {
-						req.Header.Add("Content-Type", "application/json")
-					}
-				}
+				req.Header.Add("Content-Type", tt.contentType)
 
 				client := &http.Client{}
 				res, err = client.Do(req)
@@ -127,7 +121,7 @@ func TestWebhook(t *testing.T) {
 				}
 
 				if res.StatusCode != tt.respStatus || string(body) != tt.respBody {
-					t.Errorf("failed %q (id: %s):\nexpected status: %#v, response: %s\ngot status: %#v, response: %s", tt.desc, tt.id, tt.respStatus, tt.respBody, res.StatusCode, body)
+					t.Errorf("failed %q (id: %s):\nexpected status: %#v, response: %s\ngot status: %#v, response: %s\ncommand output:\n%s\n", tt.desc, tt.id, tt.respStatus, tt.respBody, res.StatusCode, body, b)
 				}
 
 				if tt.logMatch == "" {
@@ -293,11 +287,11 @@ func webhookEnv() (env []string) {
 }
 
 var hookHandlerTests = []struct {
-	desc       string
-	id         string
-	headers    map[string]string
-	body       string
-	urlencoded bool
+	desc        string
+	id          string
+	headers     map[string]string
+	contentType string
+	body        string
 
 	respStatus int
 	respBody   string
@@ -307,6 +301,7 @@ var hookHandlerTests = []struct {
 		"github",
 		"github",
 		map[string]string{"X-Hub-Signature": "f68df0375d7b03e3eb29b4cf9f9ec12e08f42ff8"},
+		"application/json",
 		`{
 			"after":"1481a2de7b2a7d02428ad93446ab166be7793fbb",
 			"before":"17c497ccc7cca9c2f735aa07e9e3813060ce9a6a",
@@ -451,7 +446,6 @@ var hookHandlerTests = []struct {
 				"watchers":1
 			}
 		}`,
-		false,
 		http.StatusOK,
 		`arg: 1481a2de7b2a7d02428ad93446ab166be7793fbb lolwut@noway.biz
 env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00
@@ -462,8 +456,8 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00
 		"bitbucket", // bitbucket sends their payload using uriencoded params.
 		"bitbucket",
 		nil,
+		"application/x-www-form-urlencoded",
 		`payload={"canon_url": "https://bitbucket.org","commits": [{"author": "marcus","branch": "master","files": [{"file": "somefile.py","type": "modified"}],"message": "Added some more things to somefile.py\n","node": "620ade18607a","parents": ["702c70160afc"],"raw_author": "Marcus Bertrand <marcus@somedomain.com>","raw_node": "620ade18607ac42d872b568bb92acaa9a28620e9","revision": null,"size": -1,"timestamp": "2012-05-30 05:58:56","utctimestamp": "2014-11-07 15:19:02+00:00"}],"repository": {"absolute_url": "/webhook/testing/","fork": false,"is_private": true,"name": "Project X","owner": "marcus","scm": "git","slug": "project-x","website": "https://atlassian.com/"},"user": "marcus"}`,
-		true,
 		http.StatusOK,
 		`success`,
 		``,
@@ -472,6 +466,7 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00
 		"gitlab",
 		"gitlab",
 		map[string]string{"X-Gitlab-Event": "Push Hook"},
+		"application/json",
 		`{
 			"object_kind": "push",
 			"before": "95790bf891e76fee5e1747ab589903a6a1f80f22",
@@ -514,7 +509,6 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00
 			],
 			"total_commits_count": 4
 		}`,
-		false,
 		http.StatusOK,
 		`arg: b6568db1bc1dcd7f8b4d5a946b0b91f9dacd7327 John Smith john@example.com
 `,
@@ -524,6 +518,7 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00
 		"xml",
 		"xml",
 		map[string]string{"Content-Type": "application/xml"},
+		"application/xml",
 		`<app>
    <users>
      <user id="1" name="Jeff" />
@@ -533,15 +528,46 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00
      <message id="1" from_user="1" to_user="2">Hello!!</message>
    </messages>
 </app>`,
-		false,
 		http.StatusOK,
 		`success`,
 		``,
 	},
+	{
+		"multipart",
+		"plex",
+		nil,
+		"multipart/form-data; boundary=xxx",
+		`--xxx
+Content-Disposition: form-data; name="payload"
+
+{
+   "event": "media.play",
+   "user": true,
+   "owner": true,
+   "Account": {
+      "id": 1,
+      "thumb": "https://plex.tv/users/1022b120ffbaa/avatar?c=1465525047",
+      "title": "elan"
+   }
+}
+
+--xxx
+Content-Disposition: form-data; name="thumb"; filename="thumb.jpg"
+Content-Type: application/octet-stream
+Content-Transfer-Encoding: binary
+
+binary data
+--xxx--`,
+		http.StatusOK,
+		`success`,
+		``,
+	},
+
 	{
 		"missing-cmd-arg", // missing head_commit.author.email
 		"github",
 		map[string]string{"X-Hub-Signature": "ab03955b9377f530aa298b1b6d273ae9a47e1e40"},
+		"application/json",
 		`{
 			"head_commit":{
 				"added":[
@@ -571,7 +597,6 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00
 			},
 			"ref":"refs/heads/master"
 		}`,
-		false,
 		http.StatusOK,
 		`arg: 1481a2de7b2a7d02428ad93446ab166be7793fbb lolwut@noway.biz
 env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00
@@ -583,6 +608,7 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00
 		"missing-env-arg", // missing head_commit.timestamp
 		"github",
 		map[string]string{"X-Hub-Signature": "2cf8b878cb6b74a25090a140fa4a474be04b97fa"},
+		"application/json",
 		`{
 			"head_commit":{
 				"added":[
@@ -611,7 +637,6 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00
 			},
 			"ref":"refs/heads/master"
 		}`,
-		false,
 		http.StatusOK,
 		`arg: 1481a2de7b2a7d02428ad93446ab166be7793fbb lolwut@noway.biz
 `,
@@ -619,24 +644,24 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00
 	},
 
 	// test with custom return code
-	{"empty payload", "github", nil, `{}`, false, http.StatusBadRequest, `Hook rules were not satisfied.`, ``},
+	{"empty payload", "github", nil, "application/json", `{}`, http.StatusBadRequest, `Hook rules were not satisfied.`, ``},
 	// test with custom invalid http code, should default to 200 OK
-	{"empty payload", "bitbucket", nil, `{}`, false, http.StatusOK, `Hook rules were not satisfied.`, ``},
+	{"empty payload", "bitbucket", nil, "application/json", `{}`, http.StatusOK, `Hook rules were not satisfied.`, ``},
 	// test with no configured http return code, should default to 200 OK
-	{"empty payload", "gitlab", nil, `{}`, false, http.StatusOK, `Hook rules were not satisfied.`, ``},
+	{"empty payload", "gitlab", nil, "application/json", `{}`, http.StatusOK, `Hook rules were not satisfied.`, ``},
 
 	// test capturing command output
-	{"don't capture output on success by default", "capture-command-output-on-success-not-by-default", nil, `{}`, false, http.StatusOK, ``, ``},
-	{"capture output on success with flag set", "capture-command-output-on-success-yes-with-flag", nil, `{}`, false, http.StatusOK, `arg: exit=0
+	{"don't capture output on success by default", "capture-command-output-on-success-not-by-default", nil, "application/json", `{}`, http.StatusOK, ``, ``},
+	{"capture output on success with flag set", "capture-command-output-on-success-yes-with-flag", nil, "application/json", `{}`, http.StatusOK, `arg: exit=0
 `, ``},
-	{"don't capture output on error by default", "capture-command-output-on-error-not-by-default", nil, `{}`, false, http.StatusInternalServerError, `Error occurred while executing the hook's command. Please check your logs for more details.`, ``},
-	{"capture output on error with extra flag set", "capture-command-output-on-error-yes-with-extra-flag", nil, `{}`, false, http.StatusInternalServerError, `arg: exit=1
+	{"don't capture output on error by default", "capture-command-output-on-error-not-by-default", nil, "application/json", `{}`, http.StatusInternalServerError, `Error occurred while executing the hook's command. Please check your logs for more details.`, ``},
+	{"capture output on error with extra flag set", "capture-command-output-on-error-yes-with-extra-flag", nil, "application/json", `{}`, http.StatusInternalServerError, `arg: exit=1
 `, ``},
 
 	// Check logs
-	{"static params should pass", "static-params-ok", nil, `{}`, false, http.StatusOK, "arg: passed\n", `(?s)command output: arg: passed`},
-	{"command with space logs warning", "warn-on-space", nil, `{}`, false, http.StatusInternalServerError, "Error occurred while executing the hook's command. Please check your logs for more details.", `(?s)unable to locate command.*use 'pass[-]arguments[-]to[-]command' to specify args`},
-	{"unsupported content type error", "github", map[string]string{"Content-Type": "nonexistent/format"}, `{}`, false, http.StatusBadRequest, `Hook rules were not satisfied.`, `(?s)error parsing body payload due to unsupported content type header:`},
+	{"static params should pass", "static-params-ok", nil, "application/json", `{}`, http.StatusOK, "arg: passed\n", `(?s)command output: arg: passed`},
+	{"command with space logs warning", "warn-on-space", nil, "application/json", `{}`, http.StatusInternalServerError, "Error occurred while executing the hook's command. Please check your logs for more details.", `(?s)unable to locate command.*use 'pass[-]arguments[-]to[-]command' to specify args`},
+	{"unsupported content type error", "github", map[string]string{"Content-Type": "nonexistent/format"}, "application/json", `{}`, http.StatusBadRequest, `Hook rules were not satisfied.`, `(?s)error parsing body payload due to unsupported content type header:`},
 }
 
 // buffer provides a concurrency-safe bytes.Buffer to tests above.