strongloop / strong-remoting

Communicate between objects in servers, mobile apps, and other servers.
www.strongloop.com
Other
105 stars 93 forks source link

Fix coercion of input arguments (HTTP/REST) #312

Closed bajtos closed 8 years ago

bajtos commented 8 years ago

Based on the integration test suite written in #304, I am proposing to make the following changes/fixes in the way how we coerce input arguments.

I think most (if not all) of this changes can be considered as backwards-compatible fixes to be landed on 2.x too.

@STRML @ritch thoughts?


Terminology:

This may be possibly controversial. Should we introduce a config option to control this behaviour?

Convert null input to null value:

Set argument to undefined when not set:

?arg=0 should be converted to new Date(0) for both

urlencoded - boolean should reject all values except true/false, e.g.

Examples: ?arg=undefined, ?arg=true, but also ?arg=2343546576878989879789

Open points to discuss

bajtos commented 8 years ago

Case-insensitive boolean strings

Both ?arg=FALSE and ?arg=FalsE should be coerced to `false.

STRML commented 8 years ago

This all looks really good and I'm glad you guys are taking a serious look at it.

urlencoding is probably the trickiest part because there is so much sloppiness in this string-only input method. By far the most coercion should go there.

On the contentious issues:

ritch commented 8 years ago

I think Date coercion should only happen when the input type is Date, otherwise it's potentially too much magic

Sure, and I thought we all agreed that we were moving away from implicit coercion.

I apologize for being somewhat out of the loop, but if I type a parameter as an actual model type (or is there any anonymous schema type?), will it coerce its attributes? E.g. something like `type: 'object', schema: {date: Date}' or the like.

If you specify type: 'SomeModel' strong-remoting will use the converter specified by loopback to new SomeModel(arg), which will set default properties and apply all setters and getters. It will coerce based on the properties type.

var properties = {date: {type: Date}};
var MyModel = loopback.createModel('MyModel', properties);
var data = {date: '2016-06-10T16:25:55.132Z'};
var inst = new MyModel(data);
assert(inst.date instanceof Date);
bajtos commented 8 years ago

Done and released.