You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
DateTimeOffset does not parse a valid ISO-8601 datetime string #51740
DateTimeOffset does not correctly parse a valid ISO-8601 datetime string. Wikipedia claims the current ISO-8601 definition prefers a comma as the decimal mark for the lowest time order present:
However, DateTimeOffset is not capable of parsing a datetime string with a comma decimal mark for the lowest time order when that time order is seconds:
@chris-steema You are right that the comma can replace the period in the ISO-8601 but currently we don't support that. We only support the format we use when formatting the DateTimeOffset which always uses the period. You may workaround it by calling ParseExact and passing the format yyyy'-'MM'-'dd'T'HH':'mm':'ss','fffffffzzz.
XML Schema Part 2: Datatypes Second Edition seems to specify only . for fractional seconds. Thus, even if DateTimeOffset.Parse is changed to support comma as well, I think XmlConvert should be left as is.
Thank you @tarekgh, you're right, please let me explain why this is a concern for us. Our .NET app runs on Linux, and collates information from both Linux and Windows systems. In short, the issue is this:
tofol@neptune:~$ date --iso-8601=ns
2021-04-23T22:07:49,016860795+02:00
Linux appears to understand the decimal mark of ISO-8601 as a comma. What this means on our system, when receiving hundreds of inputs a second from both Windows and Linux, is a degradation of performance as we have to call TryParse to determine which interpretation of ISO-8601 is arriving to our system.
Although DateTimeOffset.Parse (as opposed to ParseExact) allows excess digits of fractional seconds, it looks like you cannot change the separator character there by cloning CultureInfo.InvariantCulture and then setting its NumberFormat.NumberDecimalSeparator or anything else, because . is hardcoded near the ParseFraction calls:
Besides, it is a bit misleading that failing the length check causes result.SetBadDateTimeFailure() to be called even though the problem is in the format string (can never match any input) and not in the input string.
@chris-steema my proposal was trying to avoid allocating any new string. but if this is not issue to you, then that is ok to do what you have proposed.
Activity
GrabYourPitchforks commentedon Apr 23, 2021
/cc globalization crew (@maryamariyan @michaelgsharp @safern @tarekgh) in case you all have further thoughts
tarekgh commentedon Apr 23, 2021
@chris-steema You are right that the comma can replace the period in the ISO-8601 but currently we don't support that. We only support the format we use when formatting the DateTimeOffset which always uses the period. You may workaround it by calling ParseExact and passing the format
yyyy'-'MM'-'dd'T'HH':'mm':'ss','fffffffzzz.KalleOlaviNiemitalo commentedon Apr 23, 2021
XML Schema Part 2: Datatypes Second Edition seems to specify only
.for fractional seconds. Thus, even if DateTimeOffset.Parse is changed to support comma as well, I think XmlConvert should be left as is.chris-steema commentedon Apr 23, 2021
Thank you @tarekgh, you're right, please let me explain why this is a concern for us. Our .NET app runs on Linux, and collates information from both Linux and Windows systems. In short, the issue is this:
Linux appears to understand the decimal mark of ISO-8601 as a comma. What this means on our system, when receiving hundreds of inputs a second from both Windows and Linux, is a degradation of performance as we have to call TryParse to determine which interpretation of ISO-8601 is arriving to our system.
tarekgh commentedon Apr 23, 2021
@chris-steema I am not objecting we try to fix that in the .NET. I was just pointing at some temporary workaround.
tarekgh commentedon Apr 23, 2021
Also, are all dates you are receiving in ISO format? I am asking because if this is the case, you can check
This should mitigate the performance concern.
2 remaining items
KalleOlaviNiemitalo commentedon Apr 26, 2021
Although DateTimeOffset.Parse (as opposed to ParseExact) allows excess digits of fractional seconds, it looks like you cannot change the separator character there by cloning CultureInfo.InvariantCulture and then setting its NumberFormat.NumberDecimalSeparator or anything else, because
.is hardcoded near the ParseFraction calls:runtime/src/libraries/System.Private.CoreLib/src/System/Globalization/DateTimeParse.cs
Lines 647 to 654 in 2f740ad
runtime/src/libraries/System.Private.CoreLib/src/System/Globalization/DateTimeParse.cs
Lines 2980 to 2982 in 2f740ad
If .NET removed the following length check, then I think you could use "FFFFFFFFF" (upper case so that it also accepts fewer than nine digits):
runtime/src/libraries/System.Private.CoreLib/src/System/Globalization/DateTimeParse.cs
Line 4052 in 2f740ad
Besides, it is a bit misleading that failing the length check causes
result.SetBadDateTimeFailure()to be called even though the problem is in the format string (can never match any input) and not in the input string.Clockwork-Muse commentedon Apr 26, 2021
.... note that, last time I checked, parsing code handled excess digits in one of two ways (depending on the precise path)
double...? Been a while since I looked).tarekgh commentedon Apr 26, 2021
@chris-steema please try the following code:
chris-steema commentedon Apr 26, 2021
Thanks again @tarekgh. Maybe just as easy to do:
Until the feature request has been implemented.
tarekgh commentedon Apr 26, 2021
@chris-steema my proposal was trying to avoid allocating any new string. but if this is not issue to you, then that is ok to do what you have proposed.
refactor(dotnet#51740): Extracting some constants.
fix(dotnet#51740): Adding comma as valid time fraction delimiter.