validatorjs / validator.js

String validation
MIT License
23.01k stars 2.29k forks source link

isISO8601 - error in parsing date string #1046

Open vpeshka opened 5 years ago

vpeshka commented 5 years ago

I've got the strange case with date string parsing. I think case you can see on screenshot is implemented with errors. I expected to see false instead of true, because date string doesn't contain separators.

validator-error

amrHassanAbdallah commented 5 years ago

Can I take this one?

Is it required to have separators, I mean after logging some parts of the code with the strict mode, it turns out the script were able to identify that the day is 2 and month is 3 which is valid.

So my question is strict option means that the date string needs to have separator?

vpeshka commented 5 years ago

@amrHassanAbdallah I think only valid representation of ISO8601 can pass. In this case I guess this string was interpretate as timestamp, but it isn't valid for ISO8601. I guess it can be related to this line https://github.com/validatorjs/validator.js/blob/c5c0bf2eeea2aebfca9be81280f3488868d56f18/src/lib/isISO8601.js#L28

ezkemboi commented 4 years ago

@amrHassanAbdallah PR welcome is welcome.

danielstorch commented 4 years ago

I'm doing something similar like this:

const { body } = require('express-validator');
router.post("/", [body('createdTime').isISO8601({ strict: true })], async (req, res) => {
...
});

Since im using strict: true, i would expect 2020-03-24 08:16:47.297Z without the T or something like 2020-03-24 not to pass. But in my example it does.

Only structure like this 2020-03-24T08:16:47.297Z should be valid.

profnandaa commented 4 years ago

Send a PR please

mum-never-proud commented 4 years ago

@profnandaa I have a quest

https://github.com/validatorjs/validator.js/blob/4e0fc27427ef8cda7d18f6e2c1c6b28baa6bb594/src/lib/isISO8601.js#L7-L35

can't this implementation be replaced by isDate (we already have) for stricter dates, it works more or less the same to me

your thoughts?

mum-never-proud commented 4 years ago

pong @profnandaa

profnandaa commented 4 years ago

@mum-never-proud -- do all the previous tests pass, with a replacement? If yes, we could require isDate module within this...

mum-never-proud commented 4 years ago

sure i will check @profnandaa btw what do mean by previous tests pass? tests for isDate?

like check if iso8601 tests passes in isDate?

correct me if i am wrng

profnandaa commented 4 years ago

Sure, that's what I mean. Sorry for the delayed response.

JuanFML commented 3 years ago

Hello I was looking into this issue, although looking into the ISO8601 format and the library, I have the following concerns:

Maybe an option of noSeparator or something like this could be added to check for the separtor - within a date , or could be added to one of the already existing options.

Regarding the replacement using isDate, the ordinal date case is not supported inside isDate, so from the beginning the tests with this type of case do not pass, although maybe an option inside isDate could be added to be used inside isISO8601.

zlavoie commented 1 year ago

Is there any update on this?