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

Parsing a number with 7 characters using moment.ISO_8601 is returning true #3717

Closed
ValterSantosMatos opened this Issue Jan 17, 2017 · 5 comments

Comments

Projects
None yet
5 participants
@ValterSantosMatos

ValterSantosMatos commented Jan 17, 2017

Hi there,

Quite simple to reproduce, parsing a number with 7 characters using moment.ISO_8601 is returning true.

Examples:

moment(7000000, moment.ISO_8601, true).isValid(); // true
moment("7000000", moment.ISO_8601, true).isValid(); // true
moment(7000000, moment.ISO_8601, false).isValid(); // true

I run directly on the moment.js website

Version
Chrome Version 55.0.2883.95 (64-bit) on OSX

@mj1856

This comment has been minimized.

Member

mj1856 commented Jan 17, 2017

Weird. But reproducible. Thanks for finding it!

@mj1856 mj1856 added the Bug label Jan 17, 2017

@marwahaha

This comment has been minimized.

Member

marwahaha commented Jan 31, 2017

I did some digging. This is since part of the ISO 8601 spec seems to include Ordinal Dates (YYYY-DDD or YYYYDDD). MomentJS can handle this, including if YYYYDDD is a 7-digit number. There still is a bug:

  • When the day is set to zero (as shown in the example) for either YYYYDDD or YYYY-DDD, the parser evaluates as Valid and sets the day to January 1st.
@icambron

This comment has been minimized.

Member

icambron commented Feb 8, 2017

Good find @marwahaha. Do you have a suggested fix?

@marwahaha

This comment has been minimized.

Member

marwahaha commented Feb 9, 2017

Unfortunately I couldn't figure it out when I looked at it.

Probably, the expected result should be an invalid date.

@maggiepint

This comment has been minimized.

Member

maggiepint commented Feb 28, 2017

Closing in favor of #3787

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