-
Notifications
You must be signed in to change notification settings - Fork 110
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
BTP GeneralizedTime's Leap-second should be "smeared" #481
Comments
Makes sense to me. @justmoon @emschwartz WDYT? If I recall correctly the choice to use ISO-8601 was to explicitly support leap seconds however I'm not sure if we checked at that time if any OS parsers actually support leap seconds when parsing ISO 8601 format strings like those shown in the examples. As a side note, I think that section of the IL RFC 30 is out of date as ILP packets no longer use ASN.1 GeneralizedTime. |
@sappenin Makes sense to me, this was an oversight on my part. Thanks for noticing and the quality research. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. If this issue is important, please feel free to bring it up on the next Interledger Community Group Call or in the Gitter chat. |
In IL-RFC-30, the spec mandates that the String value of a leap-second date should BTP-encode to a String that matches its ISO-8601 counterpart (i.e., keep the leap second in each String), like this:
2016-12-31T23.59.60.852Z
->20161231235960.852Z
(leap second)However, I suggest that the RFC should instead specify that leap-second dates should simply be invalid. As an example, it's unlikely that a system would produce a Timestamp like this:
2016-12-31T23.59.60.852Z
My rationale is as follows:
1. Object Construction in JS & Java
2. Parsing
Date
with the String2016-12-31T23:59:60.852Z
produces an error.Instant.parse("2016-12-31T23:59:60.852Z")
throws an exception.btp-packet
is actually broken in this regard when handling leap-seconds per this code here.3. UTC Standards
The text was updated successfully, but these errors were encountered: