-
Notifications
You must be signed in to change notification settings - Fork 357
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
Fix: allow explicit 0 secfracs in datetime format #234
Conversation
@@ -138,7 +138,7 @@ protected function validateDateTime($datetime, $format) | |||
// which will fail the above string comparison because the passed | |||
// $datetime may be '2000-05-01T12:12:12.123Z' but format() will return | |||
// '2000-05-01T12:12:12.123000Z' | |||
if ((strpos('u', $format) !== -1) && (intval($dt->format('u')) > 0)) { | |||
if ((strpos('u', $format) !== -1) && (preg_match('/\.\d+Z/', $datetime))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This pattern will produce a match on the invalid test case yet the invalid test case passes because of the absence of the "u" check in this condition.
I have a refiddle where I added the end-of-string anchor that seems to make it a bit more clear: http://refiddle.com/3nf2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, thanks. Also, I hadn't considered 2000-05-01T12:12:12.000000+01:00
, which this will fail on.
EDIT: although that seems okay because format Y-m-d\TH:i:s.uP
is never attempted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have updated with your suggestion.
@mathroc what do you think? |
b6ce179
to
2f0f47a
Compare
that's working for me :) |
2f0f47a
to
ffc856d
Compare
+1 |
I'm trying to add unit tests to cover this (#366), as the return line is never being hit by the current testsuite. However, I have a few questions about this code:
I would appreciate it if someone could clarify what this code is actually for, and whether it still serves a purpose, needs moving, has been superseded, or was simply merged in error after the problem was already solved. // handles the case where a non-6 digit microsecond datetime is passed
// which will fail the above string comparison because the passed
// $datetime may be '2000-05-01T12:12:12.123Z' but format() will return
// '2000-05-01T12:12:12.123000Z'
if ((strpos('u', $format) !== -1) && (preg_match('/\.\d+Z$/', $datetime))) {
return true;
} |
No description provided.