Data is stripped when no content-type given in request header

[andrewtlove]

I've just spent the last week going round and round with the team at Asana over the fact that their webhooks don't include a content-type header. While it's certainly on them to include it, there should be better default behavior than just stripping the data out altogether.

[Marak]

@andrewtlove -

Could you be more specific as to what needs to change on hook.io to fix this?

Is hook.io removing response headers which it shouldn't? Does hook.io need to add default headers for certain situations?

You tell me and I'll do my best to fix.

[andrewtlove]

@Marak

I believe the corrective action should be to:

1. Default to a commonly-used MIME-type if no MIME-type header is present in the request or
2. Allow for some sort of override, perhaps in a form similar to the way mschema allows for default values/validation.

This is currently a low-priority issue because I expect Asana to be taking changes to production webhooks within the next week, however I imagine this has a greater positive impact for other users over the current fully-automatic model.

[Marak]

@andrewtlove -

Forgive me, but I'm still unclear as to exactly how this manifests itself as a problem when using hook.io.

I believe there is an issue, but I don't have every feature / quirk of our API mapped in my head. If something is happening that is unexpected, it's most likely an un-intended action of our API.

Does this have to do with the parsing of incoming request parameters by content type? Is our automatic parsing not kicking in? Or does this have to do with us stripping incoming data which we should not remove?

The best way for me to understand would be a pseudo code snippet or hook which could show the failure.

Thanks!

[andrewtlove]

To replicate the issue I'm describing, try inspecting the hook.params object for a payload which has no content-type defined in it's header. You'll see that the body of the payload is stripped out because it doesn't match any of the predefined content-types as defined here.

Here's an example of a response using the equivalent of hook.res.end(hook.req.body) to simply echo back the body of the request.

[Marak]

@andrewtlove -

Now I understand!

It seems to me that stripping of the body data due to a missing content-type is unexpected and invalid behavior on our API.

I believe a one-line change here should fix the problem. We aren't passing req.body for some reason. I'll double check the streaming tests ( based on the code comment ), but I think this should be an easy fix.

Good catch! Thank you for the feedback.

[andrewtlove]

Just happy to contribute. :)

[Marak]

@andrewtlove -

I've applied a fix locally with https://github.com/bigcompany/parse-service-request/commit/f5f4be8242cb9d7e84d850d14ede174c13f04270#diff-168726dbe96b3ce427e7fedce31bb0bcL78

The thing is, I don't think all of the helpers we have for populating hook.params will work if the incoming content-type header is missing ( such as JSON parsing ).

It's not ideal, but you should still be able to manually parse the incoming request inside the hook service with this fix.

Should have this deployed in the next few days.

[Marak]

This should have been deployed with the latest changes.

Still don't have a test though...

[Marak]

Closing. Should be fixed and deployed.