Is there a bug in the DateTime.Parse method when UTC is used with time zone offsets?

Viewed 355

The Microsoft documentation states that, among other string representations of DateTime structures, DateTime.Parse(string) conforms to ISO 8601 and accepts Coordinated Universal Time (UTC) formats for time zone offsets.

When I run the following unit test cases, DateTime.Parse(string) and DateTime.TryParse(string, out DateTime) accepts an additional character at the end of the source string. This seems to be a bug. When more than one additional character is appended, the method correctly fails to parse the string.

[Theory]
[InlineData("2020-5-7T09:37:00.0000000-07:00")]
[InlineData("2020-5-7T09:37:00.0000000-07:00x")]
[InlineData("2020-5-7T09:37:00.0000000-07:00xx")]
[InlineData("2020-5-7T09:37:00.0000000Z")]
[InlineData("2020-5-7T09:37:00.0000000Zx")]
public void TestParse(string source)
{
    DateTime dt = DateTime.Parse(source);
    Assert.True(dt != null);
    bool b = DateTime.TryParse(source, out dt);
    Assert.True(b);
}

This unit test case was written to simplify my code and illustrate the behavior I am seeing (I recognize that expected failures should be handled differently).

Tests 1 and 2 pass and the 3rd test (with the "xx" suffix) fails. It seems to me that the second test (with the "x" suffix) should fail.

When no time zone designator is provided, test 4 passes and test 5 fails. This seems to be correct behavior.

I'm wondering if anyone has encountered this, and if so, is there general agreement that this is a bug?

2 Answers

Personally, this seems like a bug to me...

2020-5-7T09:37:00.0000000-07:00x is a valid ISO 8601 date with an invalid arbitrary character at the end which incorrectly parses without error, where as two arbitrary characters seems to be failing correctly.

What seems to be happening is the following

  1. When parsing in ParseISO8601 it grabs the correct timezone information -07:00 in ParseTimeZone
  2. This leaves the str with 1 last index
  3. It then checks for str.Match('#') which prefix increments the index (which I believe is the fault).
  4. At this point it assumes the parsing is complete in subsequent checks due to the index being at the end of string

Excerpt from Match

Comments mine

internal bool Match(char ch)
{
    if (++Index >= Length) // reaches the end of the string here
    {
        return false;
    }
    ...
}

The above method returns false, however it has incremented the index! which seems extremely strange in the scheme of things.

What it seems like it's designed to do, is check the current character for hash, then null termination, then fail if there is anything else left.

Excerpt from ParseISO8601

str.SkipWhiteSpaces();
if (str.Match('#'))  // mine... check hash ... no
{
   if (!VerifyValidPunctuation(ref str))
   {
      result.SetFailure(ParseFailureKind.Format, "Format_BadDateTime", null);
      return false;
   }
   str.SkipWhiteSpaces();
}
if (str.Match('\0')) // mine... check null termination ... no
{
   if (!VerifyValidPunctuation(ref str))
   {
      result.SetFailure(ParseFailureKind.Format, "Format_BadDateTime", null);
      return false;
   }
}
if (str.GetNext()) // mine... get anything else, if found fail
{
   // If this is true, there were non-white space characters remaining in the DateTime
   result.SetFailure(ParseFailureKind.Format, "Format_BadDateTime", null);
   return false;
}

Note : This seems to be happening in every framework I have tested.

In short, it ignores any arbitrary character on the end of the date completely, however if there are 2 characters left it behaves as expected due to the fact it decrements the index in the match check.

internal bool Match(char ch) {
    if (++Index >= len) {
        return (false);
    }
    if (Value[Index] == ch) {
        m_current = ch;
        return (true);
    }
    Index--;
    return (false);
}

Update 1

I have reported this as a runtime bug on GitHub which can be tracked here

DateTime.Parse ISO 8601 allowing extra arbitrary character at the end of the date #46477

Update 2

This has officially been labeled a runtime bug, and a fix has been slated for .Net 6

I haven't had time to build the runtime to check what it's actually doing, but from the comments it seems when it pulls the timezone the current index is incremented 1 too far and the match is incorrectly checking the next char.

There is a PR for fixing the issue. The fix should be in the future .NET release (6.0). Thanks for reporting the issue.

Related