Stricter time format constraints #481
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kylef
changed the title
kylef
commented
Apr 22, 2021
karenetheridge
approved these changes
Apr 22, 2021
We are failing on `Z+` not the invalid minute, so let's make sure that implementors are catching that case not the (mis-copy of 00:60).
|
I'll leave this open for a bit to give others a chance to weigh in, but this is really good, and you found some bugs in my implementation because I was being sloppy, so 🎉 👯 ❤️ |
|
Can someone elaborate a bit on the leap second thinking? I'm no RFC 3339 expert, but why is the leap second test valid? Whether a leap second is allowed depends on the leap second rules in the RFC yes? Which means knowing what day it is? What interpretation makes us say including one is always a valid instance? |
|
Without the date, all we can verify is that leap seconds can only occur just before midnight (UTC/Zulu). Technically any date in the future could have a leap second in it, since leap seconds are often only announced just before they happen. But without any date at all, all we know is this could describe a real point in time, therefore it's not invalid. |
davishmcclurg
added a commit
to davishmcclurg/json_schemer
that referenced
this pull request
Oct 21, 2021
Exposed by: json-schema-org/JSON-Schema-Test-Suite#481 Ruby incorrectly allows `24` as the hour in an RFC3339 timestamp. It also does not handle leap seconds very well: ``` -> Time.parse("23:59:60Z") => 2021-10-18 00:00:00 UTC ``` They should only be allowed at 23:59 UTC.
davishmcclurg
added a commit
to davishmcclurg/json_schemer
that referenced
this pull request
Apr 22, 2022
Exposed by: json-schema-org/JSON-Schema-Test-Suite#481 Ruby incorrectly allows `24` as the hour in an RFC3339 timestamp. It also does not handle leap seconds very well: ``` -> Time.parse("23:59:60Z") => 2021-10-18 00:00:00 UTC ``` They should only be allowed at 23:59 UTC.
davishmcclurg
added a commit
to davishmcclurg/json_schemer
that referenced
this pull request
May 15, 2023
Exposed by: json-schema-org/JSON-Schema-Test-Suite#481 Ruby incorrectly allows `24` as the hour in an RFC3339 timestamp. It also does not handle leap seconds very well: ``` -> Time.parse("23:59:60Z") => 2021-10-18 00:00:00 UTC ``` They should only be allowed at 23:59 UTC.
davishmcclurg
added a commit
to davishmcclurg/json_schemer
that referenced
this pull request
May 16, 2023
Exposed by: json-schema-org/JSON-Schema-Test-Suite#481 Ruby incorrectly allows `24` as the hour in an RFC3339 timestamp. It also does not handle leap seconds very well: ``` -> Time.parse("23:59:60Z") => 2021-10-18 00:00:00 UTC ``` They should only be allowed at 23:59 UTC.
davishmcclurg
added a commit
to davishmcclurg/json_schemer
that referenced
this pull request
May 22, 2023
Exposed by: json-schema-org/JSON-Schema-Test-Suite#481 Ruby incorrectly allows `24` as the hour in an RFC3339 timestamp. It also does not handle leap seconds very well: ``` -> Time.parse("23:59:60Z") => 2021-10-18 00:00:00 UTC ``` They should only be allowed at 23:59 UTC.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Based on RFC3339's ABNF, I think this should cover all possibilities:
/c @jnewc