ValidatePayload does not correctly parse mime type from Content-Type header

[shmyer]

When validating webhook payloads the Content-Type header is compared in a switch statement with either application/json or application/x-www-form-urlencoded

go-github/github/messages.go

Lines 157 to 206 in 0318e77

	func ValidatePayload(r *http.Request, secretToken []byte) (payload []byte, err error) {
	var body []byte // Raw body that GitHub uses to calculate the signature.
	switch ct := r.Header.Get("Content-Type"); ct {
	case "application/json":
	var err error
	if body, err = ioutil.ReadAll(r.Body); err != nil {
	return nil, err
	}
	// If the content type is application/json,
	// the JSON payload is just the original body.
	payload = body
	case "application/x-www-form-urlencoded":
	// payloadFormParam is the name of the form parameter that the JSON payload
	// will be in if a webhook has its content type set to application/x-www-form-urlencoded.
	const payloadFormParam = "payload"
	var err error
	if body, err = ioutil.ReadAll(r.Body); err != nil {
	return nil, err
	}
	// If the content type is application/x-www-form-urlencoded,
	// the JSON payload will be under the "payload" form param.
	form, err := url.ParseQuery(string(body))
	if err != nil {
	return nil, err
	}
	payload = []byte(form.Get(payloadFormParam))
	default:
	return nil, fmt.Errorf("Webhook request has unsupported Content-Type %q", ct)
	}
	// Only validate the signature if a secret token exists. This is intended for
	// local development only and all webhooks should ideally set up a secret token.
	if len(secretToken) > 0 {
	sig := r.Header.Get(sha256SignatureHeader)
	if sig == "" {
	sig = r.Header.Get(sha1SignatureHeader)
	}
	if err := ValidateSignature(sig, body, secretToken); err != nil {
	return nil, err
	}
	}
	return payload, nil
	}

This is not working for Content-Type headers which contain for example a charset, resulting in those webhooks being rejected:
Content-Type: application/json; charset=UTF-8

The correct way to validate the mime type might look something like this:

https://gist.github.com/rjz/fe283b02cbaa50c5991e1ba921adf7c9

[gmlewis]

Thank you, @shmyer - would you like to work on this issue and provide a PR, or would you like me to open up this issue to all volunteer contributors to this repo?

[shmyer]

@gmlewis I don't have any experience with go, so please feel free to open up the issue :)

[gmlewis]

This would be a great PR for any new contributor to this repo or a new Go developer.
All contributions are greatly appreciated!

Note that this PR will require some very careful testing, as this is a change in behavior where we don't want to break any existing clients.

Feel free to volunteer for any issue and the issue can be assigned to you so that others don't attempt to duplicate the work.

Please check out our CONTRIBUTING.md guide to get started.

Thank you!

[sagar23sj]

Hi @gmlewis. I would like to work on this issue.

Just wanted to clarify one thing before starting. Do we just need to handle the parameters if they are present in content-type?
For example, in the case of Content-Type: application/json; charset=UTF-8, I just need to check that the content type has valid parameters with mime type??

I hope my question is clear.
Thanks

[gmlewis]

Just wanted to clarify one thing before starting. Do we just need to handle the parameters if they are present in content-type?
For example, in the case of Content-Type: application/json; charset=UTF-8, I just need to check that the content type has valid parameters with mime type??

Yes, that's correct. We had never seen the charset being passed back from GitHub before, and maybe this is a recent change.
In your PR you need to just make sure that if charset is included, that the existing checks still are handled correctly.
In other words, the presence of ; charset=... should not break anything.

I took a look at this: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Type
and I don't think we need to expect anything else.

One (simple and hopefully robust) way to handle it might be simply to split on a semicolon (;) and ignore everything after it.
Then the remaining code can be used unaltered.