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

Some date strings are silently parsed to wrong dates #124

Closed
rybakit opened this issue Feb 2, 2022 · 3 comments
Closed

Some date strings are silently parsed to wrong dates #124

rybakit opened this issue Feb 2, 2022 · 3 comments
Labels

Comments

@rybakit
Copy link

rybakit commented Feb 2, 2022

While working on an implementation of the MessagePack Timestamp extension, I encountered some unexpected issues with parsing string dates with DateTime. The lowest timestamp the extension can store is -292277022657-01-27 08:29:52 UTC, and the highest is 292277026596-12-04 15:30:07.999999999 UTC. When I feed these dates into the DataTime constructor, I get:

var_dump((new DateTime("@-9223372036854775808"))->getTimestamp());
var_dump((new DateTime('-292277022657-01-27 08:29:52 UTC'))->getTimestamp());
var_dump((new DateTime('-292277022657-01-27 08:29:53 UTC'))->getTimestamp());

var_dump((new DateTime("@9223372036854775807"))->getTimestamp());
var_dump((new DateTime('292277026596-12-04 15:30:07 UTC'))->getTimestamp());
echo (new DateTime('2512370164-01-01 00:00:00Z'))->format(DATE_RFC3339_EXTENDED);
int(-9223372036854775807)  // wrong, one second is lost
int(-9223372036854775808)  // correct
int(-9223372036854775807)  // correct

int(9223372036854775807)      // correct
int(146011735807)             // wrong, the year 292277026596 is parsed to 6596
2064-01-01T00:00:00.000+00:00 // wrong, the year 2512370164 is parsed to 2064

I understand that these can be hard to fix problems, but maybe it would at least be possible to detect such cases and throw an exception "Failed to parse time string" (as is done, for example, for 15121370164-01-01 00:00:00Z) instead of silently returning a wrong date? Now the user doesn't even know if they sent or received the wrong date.

@derickr
Copy link
Owner

derickr commented Apr 7, 2022

This is possibly a floating point rounding issue:

var_dump((new DateTime("@-9223372036854775808"))->getTimestamp());

These two are expected outcomes:

var_dump((new DateTime('292277026596-12-04 15:30:07 UTC'))->getTimestamp());
echo (new DateTime('2512370164-01-01 00:00:00Z'))->format(DATE_RFC3339_EXTENDED);

The parser only understands 4 digit years. I don't understand (yet) why it does not also produces a warning for the "odd" prefixed numbers.

@derickr derickr added the bug label Apr 7, 2022
@rybakit
Copy link
Author

rybakit commented Apr 7, 2022

The parser only understands 4 digit years.

For the record, there is a way to fool the parser and bypass the 4-digit year restriction (which gives hope that it might not be too much trouble to tweak the parser to support > 4-digit years?):

var_dump((new DateTime('292277026596-12-04 15:30:07 UTC'))->getTimestamp());
var_dump((new DateTime('+292277026596-12-04 15:30:07 UTC'))->getTimestamp());

echo (new DateTime('2512370164-01-01 00:00:00Z'))->format(DATE_RFC3339_EXTENDED), "\n";
echo (new DateTime('+2512370164-01-01 00:00:00Z'))->format(DATE_RFC3339_EXTENDED);
int(146011735807)
int(9223372036854775807)
2064-01-01T00:00:00.000+00:00
2512370164-01-01T00:00:00.000+00:00

https://3v4l.org/DfjkT.

@derickr
Copy link
Owner

derickr commented Jul 29, 2022

I've made a PR to fix the "@-9223372036854775808" case, which is the only bug indicated with this report. I've merged this too.

For parsing dates with a large year, I can't change the year size to larger than 4 digits without breaking other features. The "workaround" by using a + or - sign is the right approach.

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

No branches or pull requests

2 participants