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

Incorrect validation result when passing explicit undefined in options to isInt #1532

Open
timo92 opened this issue Nov 27, 2020 · 7 comments · May be fixed by #1542
Open

Incorrect validation result when passing explicit undefined in options to isInt #1532

timo92 opened this issue Nov 27, 2020 · 7 comments · May be fixed by #1542

Comments

@timo92
Copy link

timo92 commented Nov 27, 2020

Describe the bug
Due to my use case I'm currently wrapping the isInt validator and passing the options object as required. In conformance with the typescript type definitions I'm explicitly passing undefined for options.max. Because isInt uses hasOwnProperty to check for the presence of the option it compares the value with undefined and the validation fails.

As a workaround I changed my wrapper to pass only defined options, but would have expected isInt to handle this case.
I noticed that hasOwnProperty is used to check for options in other validators as well if no default option exists. So if addressed this issue would span over multiple validators.

Examples

import validator from "validator";

validator.isInt("2", { min: 1, max: undefined }) // yields false

Additional context
Validator.js version: 13.1.17

@fedeci
Copy link
Contributor

fedeci commented Nov 27, 2020

This also checks for truthy values:

let minCheckPassed = (!options.hasOwnProperty('min') || !options.min || str >= options.min);

@profnandaa what do you think? Should I submit a PR?

@timo92
Copy link
Author

timo92 commented Nov 27, 2020

That would however fail for
isInt("-2", { min: 0 })
as 0 is falsy.

A parseInt or typeof may be sufficient.

@fedeci
Copy link
Contributor

fedeci commented Nov 28, 2020

Right, I haven't thought about that. However parseInt would only work for integers, so it is not applicable to strings or whatever other type the option is. I excluded typeof because it is unable to check for null, but a strict equality can.

const minCheckPassed = (
  options.min === null ||
  typeof options.min === 'undefined' ||
  str >= options.min
)

@ezkemboi
Copy link
Member

@fedeci are you still willing to open a PR for this use-case/fix?

@fedeci
Copy link
Contributor

fedeci commented Nov 30, 2020

Sure!

@ezkemboi
Copy link
Member

ezkemboi commented Nov 30, 2020

Go ahead and raise a Pr for this fix and tag the issue.
I will be checking on a similar issue on #1531

@profnandaa, @tux-tn, and @rubiin what do you think?

@rubiin
Copy link
Member

rubiin commented Dec 1, 2020

Go ahead and raise a Pr for this fix and tag the issue.
I will be checking on a similar issue on #1531

@profnandaa, @tux-tn, and @rubiin what do you think?

I think this should be fixed as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants