-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Support 24:00 in hh:mm for ISO8601 dates #12197
Comments
According to ISO 8601-1:2019 "5.3.2 - Beginning of the day" this is explicitly not supported:
Additionally, "4.3.8 Clock hour and hours duration" is very explicit about valid hours being between ISO 8601-2:2019 "7.3.2 Beginning of the day" also calls out explicitly that "end of day" is not supported. (I don't know if Wikipedia is accurate to some older revision of the specification. I only have access to the most current.) |
I agree with @JeremyKuhne too. the hours should be from 0 to 23. looking at https://tools.ietf.org/html/rfc3339#section-5.7 you can also see it is explicit saying in the section 5.7. Restrictions:
In section 5.6. Internet Date/Time Format it mention:
Although Appendix A. ISO 8601 Collected ABNF suggest
If the latest version of ISO8601 (year 2019) is explicit regarding that as @JeremyKuhne mentioned, then we should stick with that and not support 24 to avoid any confusion. |
I think the key thing here is to be able to parse user input. I've regularly needed to parse "24:00:00" and I usually have to write wrapper methods (or use NodaTime) to handle this case. As soon as the dates are parsed, they are of course normalized to And as per the guidance from the spec, we shouldn't be able to format/output a date with the value For reference, NodaTime took a similar approach: the ability to parse |
I'm fine with considering parsing it, but not as part of the "O" format. |
That will be more tricky. DateTime non-exact parser has a lot heuristics to detect which part of the date/time string is year, month, day, hour, minute...etc. trying to support 24 as hour will have a lot of disadvantages here and can cause a lot of other breaks. |
https://www.w3.org/TR/2004/REC-xmlschema-2-20041028/#dateTime specifically permits 24:00:00 but does not distinguish it from 00:00:00 on the next day. https://www.w3.org/TR/2004/REC-xmlschema-2-20041028/#time is less clear on whether it allows 24:00:00. When using WCF to access a third-party SOAP service that declares some elements as xsd:time and occasionally responds with 24:00:00, I have had to edit the imported WSDL and change the type to xsd:string, so that my application could receive the data and then parse it more tolerantly. It would have been easier, especially during WSDL updates, if .NET Framework had supported 24:00:00 on its own. However, I suppose the support could be implemented in XmlSerializer and XmlConvert, not necessarily in DateTime.Parse. That would avoid messing with the non-exact parser heuristics. Adding DateTimeStyles.AllowEndOfDay and TimeSpanStyles.AllowEndOfDay for use with the ParseExact methods might make sense, though. |
@KalleOlaviNiemitalo I think your suggestion is reasonable. maybe not even using DateTimeStyles.AllowEndOfDay can be OK even if it is a breaking change as long as we restrict the functionality to parse exact.. |
OTOH, supporting 24:00:00 for xsd:time in XmlSerializer would also require changing XmlSerializationWriter.FromTime, which is called by generated code. It currently writes 00:00:00 instead, and changing that could break someone's application. Perhaps the compatibility risk could be minimized by writing 24:00:00 only when This problem does not exist for xsd:dateTime. |
Unfortunately it's no longer valid. |
Although ISO 8601 no longer allows 24:00, I believe XML Schema Part 2 still allows it, and it is actually used. Thus, I would still like to have this in XmlSerializer and XmlConvert, even if not in DateTime and DateTimeOffset themselves. |
It's supported again! See ISO 8601-1:2019/Amd 1:2022. |
|
A time of 24:00 is a valid in ISO8601 dates.
https://en.wikipedia.org/wiki/ISO_8601
Should be fixed in DateTimeOffset.Parse/DateTime.Parse and new Utf8Parser API.
The text was updated successfully, but these errors were encountered: