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

make iso parsing a bit stricter #1092

Merged
merged 1 commit into from Sep 16, 2013

Conversation

icambron
Copy link
Member

Fix for #1083. To try fixing this without breaking anything, I experimented with trying new Date(str) first and only trying the ISO regex if that didn't work, That had strange results, namely that new Date('1983-10-14') gets interpreted as UTC, so you end up with "Thu Oct 13 1983 20:00:00 GMT-0400 (EDT)".

So I stuck with my original idea of forcing ISO strings to end the string. This will of course break code that has a valid ISO-8601 string followed by a bunch of unparsable stuff. I think that's acceptable, since you don't normally expect ISO-compliant strings to also contain gibberish.

@@ -29,8 +29,8 @@ exports.create = {
test.ok(moment({year: 2010, month: 1, day: 12}).toDate() instanceof Date, "{year: 2010, month: 1, day: 12}");
test.ok(moment({year: 2010, month: 1, day: 12, hours: 1}).toDate() instanceof Date, "{year: 2010, month: 1, day: 12, hours: 1}");
test.ok(moment({year: 2010, month: 1, day: 12, hours: 1, minutes: 1}).toDate() instanceof Date, "{year: 2010, month: 1, hours: 12, minutes: 1, seconds: 1}");
test.ok(moment({year: 2010, month: 1, day: 12, hours: 1, minutes: 1, seconds: 1}).toDate() instanceof Date, "{year: 2010, month: 1, day: 12, hours: 1, minutes: 1, seconds: 1]");
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just fixing the comment slightly while I was here

@ichernev
Copy link
Contributor

Looks great! Thanks!

ichernev added a commit that referenced this pull request Sep 16, 2013
@ichernev ichernev merged commit 6f54715 into moment:develop Sep 16, 2013
@jeverden
Copy link

This breaks parsing valid ISO Formats. Take for instance a date from postgres timestamp with time zone. Parses as invalid date - 2013-12-10T21:56:00-05 which is a perfectly valid ISO Date - http://www.cl.cam.ac.uk/~mgk25/iso-time.html

@icambron
Copy link
Member Author

@jeverden thanks. The right solution is to fix the regex, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants