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

Bug in Date validation, isDate() function #1395

Closed
dcjha opened this issue Jul 28, 2020 · 2 comments · Fixed by #1402
Closed

Bug in Date validation, isDate() function #1395

dcjha opened this issue Jul 28, 2020 · 2 comments · Fixed by #1402

Comments

@dcjha
Copy link

dcjha commented Jul 28, 2020

Incorrect date formats like YYYY-MM/DD and YYYY/MM-DD which contains both hyphen - and forward slash / are being wrongly validated as true.

validator.isDate("2020-10/12"); // returns true
validator.isDate("2020/10-12");  // returns true

Expected Behavior
I think in such cases, isDate() function should return false

Additional context
Validator.js version:^13.1.1
Node.js version:v12.16.1
OS platform: [windows]

@dcjha dcjha added the 🐛 bug label Jul 28, 2020
@kathawala
Copy link

I am not totally sure these are incorrect.... I mean, they can be parsed, and to a human eye these are readable dates...

If this is only supposed to accept ISO8601 dates, then slashes are not allowed at all. But if both are allowed, it makes sense to allow a mix of the two...

@ezkemboi
Copy link
Member

ezkemboi commented Aug 4, 2020

In my opinion, I will go with the version that has a strict mode and one without.
For instance, if we pass

const date = "2020/10-12"

// strictMode is false in default
validator.isDate(date, stictMode: true) // false
validator.isDate(date) // return true
validator.isDate(date, strictMode: false) // return true

Also, if you take keen consideration with isDate(), there is an option to pass format.
I am still open for more discussion on the same.

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.

4 participants