Skip to content

Off-by-1 month when coercing a date if the coerced month has fewer days than the current month #3

Closed
wants to merge 1 commit into from

1 participant

@Aldaviva
Aldaviva commented Jun 1, 2013

Steps to reproduce

  1. Set your computer's clock to May 31, 2013 (or any 31st where the next month has < 31 days)
  2. Date.toDate('2013-06-20T00-00', 'Y-m-d<T>H-i')

Expected result

  • Thu Jun 20 2013 00:00:00 GMT-0700 (PDT)

Actual result

  • Sat Jul 20 2013 00:00:00 GMT-0700 (PDT)
  • The month is July, not June

Root cause

  1. In coerce.js, parse() creates a new Date object
  2. parse_setDate() assigns the year to the Date object
  3. parse_setDate() assigns the month to the Date object
  4. So far, the Date represents June 31, 2013
  5. June only has 30 days, so the Javascript engine normalizes the Date to July 1, 2013
  6. parse_setDate() assigns the day to the Date object
  7. The final returned date is July 20 instead of June 20

Proposed resolution

  • In parse_setDate(), assign the day first:

     date.setDate( parsers[DAY] ); date.setYear( parsers[YEAR] ); date.setMonth( parsers[MONTH] );
  • Assigning day before year also covers changing the Date object's year from a leap year to a non-leap year when the current date is February 29
  • This doesn't break any existing unit tests

Testing

  • I don't see an easy way to write a unit test for this. The reproduction depends on new Date() returning a Date object for the 31st of a month where the next month has < 31 days. I think Sinon is needed.
@Aldaviva Aldaviva referenced this pull request in Aldaviva/shipit Jun 4, 2013
Closed

Date from Jira is parsed as one month too late #1

@constantology constantology pushed a commit that closed this pull request Jun 6, 2013
constantology fix #3 Off-by-1 month when coercing a date if the coerced month has f…
…ewer days than the current month
3310d6f
@constantology constantology pushed a commit that referenced this pull request Jun 6, 2013
constantology fix #3 Off-by-1 month when coercing a date if the coerced month has f…
…ewer days than the current month
b25855a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.