json-schema-org
Should the "time" format allow leap seconds?
I've been implementing format validation recently and I was surprised that the time format includes tests for leap seconds. I don't think that makes sense. Leap seconds are tied to specific dates. Outside of the context of a date, a time with a leap second doesn't make sense.
There are some general rules about leap seconds like they are only allowed at the end of a day at the end of a month in UTC. The time tests check those rules, but without knowing the date, it's impossible to know if it's really a valid leap second.
The spec isn't clear one way or another whether leap seconds should be allowed.
It seems to me that if you're using time instead of date-time, you're not talking about a specific moment, you're talking about something that isn't specific to a date. For example, I might want to model that a class starts at 9:00AM every Saturday. The time isn't specific to a moment in time, but a range of moments. Saying that, for example, a scheduled job should run at 12:59:60 on the last day of every month doesn't make sense because that job would run randomly and only assuming your system is updated before the leap second takes place. The only time it make sense to include a leap second is if you're logging the moment something happened, and that always implies a date.
Is there any practical reason to want the time format to allow leap seconds? I don't think so and if not, we shouldn't have tests suggesting that it should be allowed.
While I agree that your example of a scheduled job doesn't make much sense, what if we're checking the timestamp of an event? That might be worth supporting.
I don't want to support it, but it's a valid use case.
Furthermore, the document referenced in RFC3339 is no longer available: 404
While it seems that you shouldn't be allowed to represent a leap second in a time because they are tied to a specific date, time is defined in exactly the same way as the time part of a date-time and explicitly is allowed to have a '60' in seconds field.
While date-time validation would require you to do a lookup in the table of current leap seconds - and that you should keep that table up to date - in theory time validation should just accept '60' as a valid value.
But because this is so poorly supported in the wild (e.g. they are not supported by any of the standard built in or NodaTime time types) I suspect that allowing that as a valid time would break all sorts of clients.
Should the "time" format allow leap seconds?
Yes. The ABNF clearly specifies it should. Any valid date-time string has a valid full-time component, therefore a date-time string representing a leap second has a full-time string with a leap second. Stripping the date portion doesn't make the time invalid.
@karenetheridge's correct. The spec is quite explicit and detailed about leap seconds.
While I agree that your example of a scheduled job doesn't make much sense, what if we're checking the timestamp of an event? That might be worth supporting.
A timestamp includes a date. It would have to use date-time, not time. I suspect you actually mean that they are recording just the time that something occurred and the date is implied or recorded in another field somehow. In that case, it could make sense, but there would be no way to validate it fully because the date is out of context. Allowing for leap seconds protects from false negatives, but creates the possibility of false positives not just for that case, but also for cases where time is used the way it's intended, which is without the context of a date.
Formats can't always be everything for every use case and I think it's best that we stick to supporting the normal case especially if supporting other use cases introduces the chance of false negatives/positives.
timeis defined in exactly the same way as thetimepart of adate-timeand explicitly is allowed to have a '60' in seconds field.Yes. The ABNF clearly specifies it should.
The time part is explicitly allowed to have "60" if it's a leap second. The ABNF includes comments for a couple of patterns describing how the allowed values change with the context provided by other parts.
date-mday = 2DIGIT ; 01-28, 01-29, 01-30, 01-31 based on month/year time-second = 2DIGIT ; 00-58, 00-59, 00-60 based on leap second rules
The spec doesn't say how to handle these additional constraints when the context they depend on isn't present. Therefore, our spec needs to make a decision and specify how to handle that case. Currently, it's ambiguous. I think it makes most sense that it should default to the normal case (59). It's not unreasonable to default to the most forgiving case (60), but I don't think that's in line with how the format is expected to be used and could cause issues with false positives.
This is turning more into a spec issue than a tests issue. By my own argument, the spec is ambiguous, so the tests should be allowed to remain. But, we need to make a decision and fix the ambiguity in the spec.
I don't think it is ambiguous - it is just "not the common sense understanding". Times are allowed to include leap seconds. Divorced from the context of a date, "60" in the seconds part indicates a leap second. You don't need a context to say "this is a leap second" with a time. If it says "60" it simply is a leap second.
Leap seconds were just a bad idea :)
So yes, I agree it is a spec problem. We'd need a different format to disallow leap seconds.
I strongly disagree that it's not ambiguous. But, I'm convinced that this isn't a test repo issue. Allowing leap seconds is at least a reasonable interpretation of the spec. Since we historically have allowed optional tests to include things where the spec isn't clear, it doesn't matter whether it's ambiguous or not. Either way the tests should be allowed.
I'm transferring this issue to the spec repo to further address the spec concerns.
Leaving aside the debate about how the time format should be interpreted for the current release, we need to make a decision on what the intended interpretation will be for the next release.
Options:
- Allow leap seconds as best can be validated given the context (only the last second of the day in UTC)
- Never allow leap seconds
- Leap second validation is optional
- Introduce a new format that unambiguously allows leap seconds
- Introduce a new format that unambiguously disallows leap seconds
(1) or (2) could be considered a breaking change depending on how you interpret the current spec. (Remember, we're allowing necessary breaking changes for the next release, but after that we can't have breaking changes anymore.)
(3) would allow for some inconsistency between implementations, but considering how unlikely the issue is to come up, it's not a huge concern. This option also has the benefit of allowing us to avoid the debate about what it should do. Each implementation can decide for themselves.
(4) and (5) avoids all controversy about the current interpretation and would result in consistent behavior across implementations, but would be obnoxious because it would amount to a name change for an edge case that's unlikely anyone will ever encounter.
My preference is (2), but I'd be totally fine with (3) if there are strong feelings that leap seconds should be allowed.
I don't think it is ambiguous - it is just "not the common sense understanding". Times are allowed to include leap seconds. Divorced from the context of a date, "60" in the seconds part indicates a leap second. You don't need a context to say "this is a leap second" with a time. If it says "60" it simply is a leap second.
I don't see how that interpretation is any more supported by the ABNF or any part of the spec than my interpretation. It's a logical and reasonable interpretation, but so is mine. The ABNF says that whether 58, 59, or 60 seconds are the max depends on the rules for leap seconds. It says nothing about what to do if the leap seconds rules can't be applied because there's no date. Saying that leap seconds are always allowed is just as much filling in the blanks as saying leap seconds are never allowed.
(6) Make no changes, and follow the RFC's ABNF.
Unless someone can point out a flaw in my reasoning, the ABNF is not sufficient to unambiguously specify the correct behavior and "make no changes" isn't an option.
Even if you're convinced that one interpretation was always correct and we shouldn't change it, we should at least include a note in the spec clarifying that that's the interpretation we expect. We wouldn't be having this discussion if the correct thing to do was straightforward and obvious. So, regardless of what behavior we decide is right, I don't think "make no changes" is an option.
I think this text from RFC3339 is pretty indicative and supportive of @jdesrosiers' argument.
Applications should not generate timestamps involving inserted leap seconds until after the leap seconds are announced.
The leap second is only valid in the context of a date and then only after announcement of the dates to which they apply.
(Granted, the text has "should not" and not "SHOULD NOT" or "MUST NOT".)
If the recommendation to applications is not to generate these values, then it seems to me that there is an implied recommendation to us is not to validate these values.
I suppose that if an implementation wanted to incorporate a system to know about leap second announcements, they could support them for date-time, but that's it. time on its own only supports seconds values 00-59.
Furthermore, we need to be cognizant of this text that follows as well:
Although ISO 8601 permits the hour to be "24", this profile of ISO 8601 only allows values between 00" and "23" for the hour in order to reduce confusion.
If there's no objections, I'm going to add a clarification that time doesn't have leap seconds citing the language that @gregsdennis quoted.