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
JSON timestamps are not valid ISO 8601 #2353
Comments
Closes #2353. Thanks to Norman Rasmussen.
Thanks @normanr, I agree with your points. I've implemented it slightly differently because it's a tiny fraction more efficient this way. |
note: the buffer is still over-sized (100 bytes), it could be smaller (maybe as small as 32 bytes, worth double-checking or giving a small amount of slack). |
some other notes:
|
Closes #2353. Thanks to Norman Rasmussen.
Yes, the buffer is oversized, I'll drop it but I don't think it's a big deal. The size passed to snprintf is 9 because the compiler thinks the int could 9 characters long even though we know it will be no more than 6 characters long. If strftime doesn't give the same length output every time it's a huge bug in strftime. |
These are both just nits, thanks for the fix!
int32 can be 10 characters, so not sure where the 9 comes from.
y10k problem 😀 but fair point. iso 8601 requires "prior agreement between the sender and the receiver", when the year is outside [0000, 9999], so unlikely to be an issue for a while. |
b726e2f added a ISO 8601-like output with a
Z
(indicating UTC) and%z
(offset from UTC). I don't think theZ
should be present (also the man page doesn't include theZ
in the example output).Currently json_print does a special workaround after printing microseconds to replace the
Z
(wheresnprintf
wrote it's null byte). It's probably simplest to just drop the microseconds from the time format, and call strftime a second time to format the timezone after the microseconds have been printed, eg:I think it would also make sense to move the
strftime
/snprintf
calls before the#ifdef WITH_CJSON
because it can be shared with both parts. Also buf could be made smaller, only the first 32 bytes will ever be used.The text was updated successfully, but these errors were encountered: