Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Joi.validate() returns default object reference instead of clone #773

Closed
tnunes opened this issue Nov 25, 2015 · 11 comments
Closed

Joi.validate() returns default object reference instead of clone #773

tnunes opened this issue Nov 25, 2015 · 11 comments
Assignees
Labels
bug Bug or defect
Milestone

Comments

@tnunes
Copy link
Contributor

tnunes commented Nov 25, 2015

Hi,

Apparently, Joi is returning a reference to the original default object declared in a schema, allowing schema mutation by validation results consumers, which can result in nasty, hard to find bugs.

var schema = Joi.object().default({ key: 'original_value' });

var firstResult = Joi.validate(undefined, schema);
console.log(firstResult);   // <- { error: null, value: { key: 'original_value' } }

firstResult.value.key = 'mutated_value';

var secondResult = Joi.validate(undefined, schema);
console.log(secondResult);  // <- { error: null, value: { key: 'mutated_value' } }

As shown in the code snippet, various calls to validate() that cause a default value to be returned, return the original object, instead of a copy of it.

This can be confirmed by checking that the schema._flags.default property is indeed mutated after the firstResult.value.key = 'mutated_value'; statement.

I'm not sure if this is by design (performance reasons?) or if it's a bug, but either way it allows a validation schema to be changed by any code that consumes the resulting object.

I believe validate() should always return a copy of the default object, therefore preventing unintended changes to the original schema.

@jjvieira
Copy link

👍

1 similar comment
@pmarques
Copy link

👍

@Marsup
Copy link
Collaborator

Marsup commented Nov 25, 2015

Feel free to make a PR.

@Marsup Marsup added the bug Bug or defect label Nov 25, 2015
@Marsup Marsup self-assigned this Nov 25, 2015
@Marsup Marsup added this to the 7.0.1 milestone Nov 25, 2015
@tnunes
Copy link
Contributor Author

tnunes commented Nov 25, 2015

@Marsup just sent PR #774.

I'm currently stuck on Node 0.12, so I need to backport this fix to Joi 6.10. How should I go about doing that? Can you create a branch for me to PR into?

@Marsup
Copy link
Collaborator

Marsup commented Nov 26, 2015

I have created branch node-0.x for that purpose, better fix your PR before making the other one though.

For future reference, while I tolerate this, I will NOT personally put any effort into that branch nor will I accept any new feature or breaking change on it, only bug fixes. If you want a fix to happen there, you'll have to do the work. Any pull request on that branch will have to have its counterpart on the latest version if impacted.

@tnunes
Copy link
Contributor Author

tnunes commented Nov 26, 2015

Fair enough. Thank you!

@tnunes
Copy link
Contributor Author

tnunes commented Nov 26, 2015

Sent PR #777 for node-0.x. Looking forward to see this published as v6.10.1!

@Marsup Marsup modified the milestones: 6.10.1, 7.0.1 Nov 26, 2015
@Marsup
Copy link
Collaborator

Marsup commented Nov 26, 2015

Published both.

@jjvieira
Copy link

👍 Thanks @Marsup!

@Marsup Marsup closed this as completed Nov 26, 2015
@tnunes
Copy link
Contributor Author

tnunes commented Nov 26, 2015

Awesome! Was really needing this. Thank you!

@pmarques
Copy link

Awesome! thanks @Marsup

@lock lock bot locked as resolved and limited conversation to collaborators Jan 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Bug or defect
Projects
None yet
Development

No branches or pull requests

4 participants