stackvana / hook.io

Open-Source Microservice Hosting Platform
https://hook.io
Other
1.26k stars 117 forks source link

namespace hook.params to avoid conflict with request data #217

Closed zanona closed 8 years ago

zanona commented 8 years ago

Hey there, first of all I want to thank you guys for the excellent product and initiative.

I was playing around with a hook I've created and noticed that sometimes, in the case the author doesn't really know about the fields submitted to a hook, but still want to parse those and send to another service for example. It becomes a bit tricky when we have the default hook, owner and gist, fields which are not namespaced and possibly common names.

So imagine if an user submits a payload containing the property owner, which is related to the application being used, that would correctly override hook.io owner, however the author also created a rule to remove the hook, owner and gist properties from the request in order to send to another service. This would then result in missing data.

Since these may be names that could be used by other applications and services commonly, I would like to suggest namespacing these as hook_io_hook, hook_io_owner and hook_io_gist for example. This would make it much easier to delete those properties knowing it is unlikely any user would provide those to an object.

What do you reckon?

Marak commented 8 years ago

Could this be a duplicate of #166 ?

You can use the safe hook.resource parameter scope?

zanona commented 8 years ago

Hi @Marak, I am not sure if that's completely related because on this situation, I wanted to leave the request parameters intact. If a user happens to provide hook.io/username/hook-name?owner=John this would override the default which is ok. But what I meant is that it's that by adopting names without a namespace makes it difficult for us to rely on those names.

So in this case I would not be sure If owner would be the owner the user provided or the one provided by hook.io. Does that make sense?

Ideally it would be safer to just have the owner, gist, hook properties inside a property called req,params.hook_io so I would know that if I wanted to only deal with data provided by users I could simply do:

delete req.params.hook_io
sendToService(req.params);
Marak commented 8 years ago

@zanona - I think we are talking about the same thing?

You can safely refer to hook.resource.owner and hook.resource.gist etc, they will not get written over by user input. hook.resource is private internal hook.io scope. hook.params is request data.

The previous values of hook.params.owner and hook.params.gist will be removed entirely. They are legacy. Instead, you should use hook.resource namespace for internal hook.io variables, and hook.params for user-submitted variables.

zanona commented 8 years ago

Ahh I completely missed they were deprecated. That's great it has been transferred to hook.resource then.

Thanks for your help.

Marak commented 8 years ago

Glad to help.

I would just remove the legacy parameters from the app, but I don't want to break any of the existing hooks.

I'll probably just run a migration script before the next big update and try to alert users of the breaking change. Should remove this sooner than later...sorry for the confusion.

Marak commented 8 years ago

Tracking #218