CSHARP-3268: Investigate Military Time Failures #1687

rstam · 2025-05-06T23:08:14Z

No description provided.

rstam · 2025-05-06T23:09:54Z

src/MongoDB.Bson/IO/JsonReader.cs

@@ -1373,7 +1373,7 @@ private DateTime ParseJavaScriptDateTimeString(string dateTimeString)
        {
            // if DateTime.TryParse succeeds we're done, otherwise assume it's an RFC 822 formatted DateTime string
            DateTime dateTime;
-            if (DateTime.TryParse(dateTimeString, out dateTime))
+            if (!dateTimeString.EndsWith("A") && DateTime.TryParse(dateTimeString, out dateTime))


For some reason DateTime.TryParse returns true for time zone A but who knows what it thinks the A means.

If the string ends in A then we should use our own parsing method.

Any chance there might be a collision with some other format ending with "A"?
Also there is not handling for "Z", is it a known abbreviation for UTC?

In the existing test data nothing else ends with "A". But you are right, there might be a timezone abbreviation we don't know about that ends in A.

We could use a regular expression:

if (!Regex.Matches(dateString, "[0-9 ]A$") && ...)

We have a test case for Z. I think it is a standard abbreviation for UTC and is handled by DateTime.TryParse.

Thanks, just wanted to confirm that Z is for UTC.
Looking at .net code, seems that A is related to Era (AD is also valid).

What worries me is that "Mon, 10 Oct 2011 11:22:33 A" is parsed to a certain value in .NET, and we'll be overriding that. If going with this change we need to convince ourselves that this is the right thing to do.

Also how will this "Mon, 10 Oct 2011 11:22:33 AD" get parsed?

rstam · 2025-05-06T23:10:14Z

src/MongoDB.Bson/IO/JsonReader.cs

-                        case "W": offset = TimeSpan.FromHours(10); break;
-                        case "X": offset = TimeSpan.FromHours(11); break;
-                        case "Y": offset = TimeSpan.FromHours(12); break;
+                        case "A": offset = TimeSpan.FromHours(1); break;


The sign of all the offsets for A to Y was backwards.

rstam · 2025-05-06T23:10:32Z

tests/MongoDB.Bson.Tests/Jira/CSharp275Tests.cs

-
-        private Test[] _tests = new Test[]
-        {
+        public static object[][] TestParseDatesData => [


Convert to supply data to a Theory.

BorisDog

Looks good, just a question about "A" and "Z" abbreviations.

BorisDog · 2025-05-07T22:24:58Z

src/MongoDB.Bson/IO/JsonReader.cs

@@ -1373,7 +1373,7 @@ private DateTime ParseJavaScriptDateTimeString(string dateTimeString)
        {
            // if DateTime.TryParse succeeds we're done, otherwise assume it's an RFC 822 formatted DateTime string
            DateTime dateTime;
-            if (DateTime.TryParse(dateTimeString, out dateTime))
+            if (!dateTimeString.EndsWith("A") && DateTime.TryParse(dateTimeString, out dateTime))


Any chance there might be a collision with some other format ending with "A"?
Also there is not handling for "Z", is it a known abbreviation for UTC?

rstam · 2025-05-07T23:10:19Z

src/MongoDB.Bson/IO/JsonReader.cs

@@ -1373,7 +1373,7 @@ private DateTime ParseJavaScriptDateTimeString(string dateTimeString)
        {
            // if DateTime.TryParse succeeds we're done, otherwise assume it's an RFC 822 formatted DateTime string
            DateTime dateTime;
-            if (DateTime.TryParse(dateTimeString, out dateTime))
+            if (!dateTimeString.EndsWith("A") && DateTime.TryParse(dateTimeString, out dateTime))


In the existing test data nothing else ends with "A". But you are right, there might be a timezone abbreviation we don't know about that ends in A.

We could use a regular expression:

if (!Regex.Matches(dateString, "[0-9 ]A$") && ...)

rstam · 2025-05-07T23:12:29Z

src/MongoDB.Bson/IO/JsonReader.cs

@@ -1373,7 +1373,7 @@ private DateTime ParseJavaScriptDateTimeString(string dateTimeString)
        {
            // if DateTime.TryParse succeeds we're done, otherwise assume it's an RFC 822 formatted DateTime string
            DateTime dateTime;
-            if (DateTime.TryParse(dateTimeString, out dateTime))
+            if (!dateTimeString.EndsWith("A") && DateTime.TryParse(dateTimeString, out dateTime))


We have a test case for Z. I think it is a standard abbreviation for UTC and is handled by DateTime.TryParse.

rstam · 2025-05-07T23:13:20Z

src/MongoDB.Bson/IO/JsonReader.cs

+                        case "V": offset = TimeSpan.FromHours(-9); break;
+                        case "W": offset = TimeSpan.FromHours(-10); break;
+                        case "X": offset = TimeSpan.FromHours(-11); break;
+                        case "Y": offset = TimeSpan.FromHours(-12); break;


Did you mean that we should add a case for "Z" here?

I don't see how we would ever reach here with a "Z" because DateTime.TryParse would have already handled it.

BorisDog

Follow up on A (and AD) "timezones".

BorisDog · 2025-05-09T17:41:12Z

src/MongoDB.Bson/IO/JsonReader.cs

@@ -1373,7 +1373,7 @@ private DateTime ParseJavaScriptDateTimeString(string dateTimeString)
        {
            // if DateTime.TryParse succeeds we're done, otherwise assume it's an RFC 822 formatted DateTime string
            DateTime dateTime;
-            if (DateTime.TryParse(dateTimeString, out dateTime))
+            if (!dateTimeString.EndsWith("A") && DateTime.TryParse(dateTimeString, out dateTime))


Also how will this "Mon, 10 Oct 2011 11:22:33 AD" get parsed?

…arse and use our own parsing.

rstam · 2025-05-29T18:22:48Z

src/MongoDB.Bson/IO/JsonReader.cs

-            if (DateTime.TryParse(dateTimeString, out dateTime))
+
+            // DateTime.Parse doesn't understand military time zones, so don't call it when a military time zone is present
+            if (!Regex.IsMatch(dateTimeString, " [A-Y]$") && DateTime.TryParse(dateTimeString, out dateTime))


Here's my suggestion:

Either we support military time zones or we don't. And if we do support them, then we need to support ALL military time zones reliably.

Since DateTime.TryParse does not understand military time zones, use a regex to detect the presence of a military time zone and do NOT call DateTime.TryParse in that case. The reason to NOT call DateTime.TryParse is that DateTime.TryParse gets confused by SOME military time zones and interprets them as something else.

I'm not sure why we call DateTime.TryParse at all here... since we have our own parsing for RFC 822 format date strings. I guess the idea here is to be "lenient" in what we accept, and that we will parse anything that DateTime.TryParse understands and fall back to our own RFC 822 datetime parser for anything DateTime.TryParse does not understand.

CSHARP-3268: Investigate Military Time Failures

b49a075

rstam requested review from BorisDog and rishitb-mongodb May 6, 2025 23:08

rstam requested a review from a team as a code owner May 6, 2025 23:08

rstam removed the request for review from a team May 6, 2025 23:08

rstam commented May 6, 2025

View reviewed changes

BorisDog reviewed May 7, 2025

View reviewed changes

rstam commented May 7, 2025

View reviewed changes

rstam requested a review from BorisDog May 7, 2025 23:14

BorisDog requested changes May 9, 2025

View reviewed changes

CSHARP-3268: If a military time zone is present don't call DateTime.P…

96a441a

…arse and use our own parsing.

rstam commented May 29, 2025

View reviewed changes

rstam requested a review from BorisDog May 29, 2025 18:25

CSHARP-3268: Investigate Military Time Failures #1687

Are you sure you want to change the base?

CSHARP-3268: Investigate Military Time Failures #1687

Uh oh!

Conversation

rstam commented May 6, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BorisDog left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BorisDog left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!