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

Serious error with format "2006-01-02T15:04:05Z" (also see #22) #51

Closed
lss4 opened this issue Nov 18, 2021 · 1 comment
Closed

Serious error with format "2006-01-02T15:04:05Z" (also see #22) #51

lss4 opened this issue Nov 18, 2021 · 1 comment
Assignees

Comments

@lss4
Copy link
Contributor

lss4 commented Nov 18, 2021

Reproducible Example

See this comment.

Here's a playground link with more examples demonstrating this issue.

The playground link uses now.Parse() and now.MustParse(). The return output consists of two parts.
The first part uses now.Parse(), with time.Now() as reference.
The second part uses now.MustParse(). using the n from your testcase as reference.

Actual playground output (notice the hour and minute):

2009-11-10 23:00:00 +0000 UTC m=+0.000000001
2021-07-20 23:59:10 +0000 UTC
2021-07-20 23:00:10 +0000 UTC

2013-11-18 17:51:49.123456789 +0000 UTC
2002-10-12 17:14:56 +0000 UTC
2002-10-12 17:51:56 +0000 UTC

The expected playground output should be:

2009-11-10 23:00:00 +0000 UTC m=+0.000000001
2021-07-20 00:59:10 +0000 UTC
2021-07-20 00:00:10 +0000 UTC

2013-11-18 17:51:49.123456789 +0000 UTC
2002-10-12 00:14:56 +0000 UTC
2002-10-12 00:00:56 +0000 UTC

Description

When parsing 2006-01-02T15:04:05Z (2006-01-02T15:04:05Z07:00 is also affected as tested), the resulted time is incorrect (see the playground output above).

  • If the time part is 00:00:00 it works as intended.
  • If hour and minute are both 00 but second is nonzero, the current hour and minute (or the hour and minute specified in n) will be used instead.
  • If hour is 00 but minute is nonzero (second doesn't matter), the current hour will be used instead.

I originally posted about this in the now-closed issue #22 as follow-up, but I'm yet to see any change or fix. As a result, I have switched back to the time.Parse() method to process time strings and removed all references to this package (refactored a good amount of code in the progress). A side effect would be that users must now explicitly configure the time format the data use, as time.Parse() requires a time format whereas now.Parse() does not.

PS: Issue #22 (9003839) did fix this issue for time format 2006-01-02 15:04:05, though, as existing test cases (which covered it) are passing.

@jinzhu
Copy link
Owner

jinzhu commented Nov 18, 2021

Fixed, thank you for your report.

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

No branches or pull requests

2 participants