Added recursive support for protected props

[dan-omniscience]

Hi,
I added support for recursive reduction of the props.
please review and merge if you like.
If you have any comments or suggestions I'll be happy to hear from you.

Thank you,
Dan

[tarlepp]

hmm, imho if (....) { because that is used elsewhere in this code.

[mikermcneil]

@dan-omniscience yeah, if you wouldn't mind, that'd be helpful just for consistency
@tarlepp thanks, good catch

[tarlepp]

same here

[tarlepp]

and here

[tarlepp]

and here

[dan-omniscience]

I'm sorry but I didn't understand what exactly do you mean by your comments, can you be more specific so I could fix them?

Thank you,
Dan

[tarlepp]

if(...){ vs if (...) {

[mikermcneil]

@dan-omniscience Nice work! Here's some of the stuff going through my head:

• This feels a little risky, since it's such a security-sensitive setting and recursive parsing of unknown JS values can get kind of crazy (see .sanitize x_x)
• I'm imagining a scenario where someone manually attaches a Buffer instance or circular reference to req.query, req.body, or req.params and it crashes their app. (Or, for example, another hook might do this automatically)
• recursive makes me wonder "recursive what?" (i.e. could it be referring to something else? Recursive requests due to redirects? etc)
• The notion of what a "recursive param" means is pretty self-explanatory, but could lead to misunderstanding of how the blacklist should be configured (e.g. should I use user.password or password?)

So, after that consideration, and in the interest of keeping this hook lightweight: I think it might be best to add a new, configurable function to configure the default redaction behavior, instead of a recursive flag-- e.g. along the lines of the new sails.config.blueprints.parseBlueprintOptions in Sails v1.

e.g.

cleanseParams: function (allParams) {
  // ...userland can do anything it likes here...
  return allParams;
},

At load-time in the configure function, I think we'd need to check that you do not specify BOTH dontLogParams and this new setting-- if you did, we should throw an error explaining that you should use one or the other, not both. (This is just for clarity, and to keep configuration as simple as possible to reduce edge cases that could bite us unexpectedly)