Skip to content

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

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

Description

@ValterSantosMatos
ValterSantosMatos
opened on 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

Activity

  1. mattjohnsonpint commented on Jan 17, 2017

    @mattjohnsonpint
    mattjohnsonpint
    on Jan 17, 2017
    Contributor

    Weird. But reproducible. Thanks for finding it!

  2. mattjohnsonpint
    added
    Bug
    on Jan 17, 2017

  3. marwahaha commented on Jan 31, 2017

    @marwahaha
    marwahaha
    on Jan 31, 2017
    Member

    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.

  4. icambron commented on Feb 8, 2017

    @icambron
    icambron
    on Feb 8, 2017
    Contributor

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

  5. marwahaha commented on Feb 9, 2017

    @marwahaha
    marwahaha
    on Feb 9, 2017
    Member

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

    Probably, the expected result should be an invalid date.

  6. maggiepint commented on Feb 28, 2017

    @maggiepint
    maggiepint
    on Feb 28, 2017
    Member

    Closing in favor of #3787

  7. maggiepint
    closed this as completedon Feb 28, 2017
  8. ichernev
    added a commit that references this issue on Mar 2, 2017

    Merge pull request #3787 from marwahaha:fix/3717

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Bug

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions