Webhook: Proposal: Add multipart form data support

Created on 23 Dec 2019  路  7Comments  路  Source: adnanh/webhook

This proposal seeks to answer the question of how we resolve #311.

Proposed Behavior

Upon receiving a multipart/form-data request, we step through the named parts and parse all parts with the following properties:

  1. The Content-Type is a support type (bascially only JSON right now).
  2. Or the part is named in the parse-parameters-as-json setting.

The resulting value would be something like:

payload = map[string]interface{}{"payload": map[string]interface{}{"event": "media.play"}

I'm assuming the accessing would look like this:

{
  "match": {
    "type": "value",
    "parameter": {
      "source": "payload",
      "name": "payload.event",
    },
    "value": "media.play"
}

Sample Payload

Content-Type: multipart/form-data; boundary=------------------------07831de27f153cbe

--------------------------07831de27f153cbe
Content-Disposition: form-data; name="payload"; filename="payload.json"
Content-Type: application/json

{
  "event": "media.play",
  "user": true,
  "owner": true
}

--------------------------07831de27f153cbe
Content-Disposition: form-data; name="config"; filename="hooks.yaml"
Content-Type: text/plain

- id: redeploy
  execute-command: ./deploy.sh

--------------------------07831de27f153cbe--

Reproduce with:

curl -F "[email protected];type=application/json" \
  -F "[email protected];type=text/plain" \
  http://localhost:9000/hooks/redeploy

Future Enhancement

It's possible for multiple parts to have the same name, so we may need to support that eventually. We would likely need to resolve #215 first.

proposal

All 7 comments

:+1:

Just to have it noted here, the tricky situation webhook can run into is when you have an async hook and files in the multipart payload. In that case, server saves contents to temp files, but the request is terminated right after receiving the payload, so we don't have a guarantee that the temp files will still be present when the running command tries to use them.

At least in this proposal, I don't intend to save these files to disk and make them available to the command. I'm only intending to use it for triggers. Are you think of something else?

I forgot to mention it, but I was thinking of adding a "max multipart form data size" setting so that we could load the body into memory and not have the http package creating temp files. See ParseMultipartForm.

Sounds good for now

I'm having to rethink my approach here as I dig into the details of multipart form data.

My initial testing was using curl to send file data, but I'm assuming that most hook providers will use simple form values instead of files. I think this is what Plex does. Values, unlike files, don't have a Content-Type.

Content-Type: multipart/form-data; boundary=xxx

--xxx
Content-Disposition: form-data; name="payload"

{"a": "b"}
--xxx
Content-Disposition: form-data; name="thumb"; filename="thumb.jpg"
Content-Type: application/octet-stream

binary data
--xxx--

Additionally, a normal web form can send something like this:

Content-Type: multipart/form-data; boundary=xxx

--xxx
Content-Disposition: form-data; name="field1"

value1
--xxx
Content-Disposition: form-data; name="field2"

value2
--xxx
Content-Disposition: form-data; name="thumb"; filename="thumb.jpg"
Content-Type: application/octet-stream

binary data
--xxx--

I suppose I need to just save each _value_ part as a map value in the payload. For example:

map[string]interface{}{
    "payload": `{"a": "b"}`,
}

and

map[string]interface{}{
    "field1": `value1`,
    "field2": `value2`,
}

Then you must use parse-parameters-as-json to target payload.payload in the first example.

We could automatically parse the part content if there is a content-type header present in the part, and it's JSON, but I would be perfectly fine with having to do that explicitly using the parse-parameters-as-json, in the end, you are receiving a form encoded data, and that's one of the main reasons why we introduced the the parse-parameters-as-json hook option.

Agreed. See the PR. It turned out to be pretty simple in the end.

PR merged, closing this.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

Adnn picture Adnn  路  6Comments

foosinn picture foosinn  路  5Comments

MichelDiz picture MichelDiz  路  5Comments

pascalandy picture pascalandy  路  4Comments

scalp42 picture scalp42  路  4Comments