Ensure JSON TryGetXXX sets values to default if returning false dotnet/corefx#37119...

* Add tests to ensure TryGetXXX returns default on false dotnet/corefx#37119

* Ensure TryGetXXX value is default when result is false dotnet/corefx#37119

* Ensure JsonDocument's TryGetXXX use default value on false dotnet/corefx#37119

* Ensure value never leaks on exception path per PR review

- Also use `0` with non-generic default assignments to primitive number types per PR review

Commit migrated from https://github.com/dotnet/corefx/commit/f2c2e2f5907c66f9c4912db55f2cb34923059ea6

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs
index 3691e26..c71d5ff 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs
@@ -391,7 +391,7 @@ namespace System.Text.Json
                 return true;
             }
 
-            value = default;
+            value = 0;
             return false;
         }
 
@@ -413,7 +413,7 @@ namespace System.Text.Json
                 return true;
             }
 
-            value = default;
+            value = 0;
             return false;
         }
 
@@ -435,7 +435,7 @@ namespace System.Text.Json
                 return true;
             }
 
-            value = default;
+            value = 0;
             return false;
         }
 
@@ -457,7 +457,7 @@ namespace System.Text.Json
                 return true;
             }
 
-            value = default;
+            value = 0;
             return false;
         }
 
@@ -481,7 +481,7 @@ namespace System.Text.Json
                 return true;
             }
 
-            value = default;
+            value = 0;
             return false;
         }
 
@@ -505,7 +505,7 @@ namespace System.Text.Json
                 return true;
             }
 
-            value = default;
+            value = 0;
             return false;
         }
 
@@ -529,7 +529,7 @@ namespace System.Text.Json
                 return true;
             }
 
-            value = default;
+            value = 0;
             return false;
         }
 
@@ -558,10 +558,16 @@ namespace System.Text.Json
 
             Debug.Assert(segment.IndexOf(JsonConstants.BackSlash) == -1);
 
+            if (segment.Length <= JsonConstants.MaximumDateTimeOffsetParseLength
+                && JsonHelpers.TryParseAsISO(segment, out DateTime tmp, out int bytesConsumed)
+                && segment.Length == bytesConsumed)
+            {
+                value = tmp;
+                return true;
+            }
+
             value = default;
-            return (segment.Length <= JsonConstants.MaximumDateTimeOffsetParseLength)
-                && JsonHelpers.TryParseAsISO(segment, out value, out int bytesConsumed)
-                && segment.Length == bytesConsumed;
+            return false;
         }
 
         internal bool TryGetValue(int index, out DateTimeOffset value)
@@ -589,10 +595,16 @@ namespace System.Text.Json
 
             Debug.Assert(segment.IndexOf(JsonConstants.BackSlash) == -1);
 
+            if (segment.Length <= JsonConstants.MaximumDateTimeOffsetParseLength
+                && JsonHelpers.TryParseAsISO(segment, out DateTimeOffset tmp, out int bytesConsumed)
+                && segment.Length == bytesConsumed)
+            {
+                value = tmp;
+                return true;
+            }
+
             value = default;
-            return (segment.Length <= JsonConstants.MaximumDateTimeOffsetParseLength)
-                && JsonHelpers.TryParseAsISO(segment, out value, out int bytesConsumed)
-                && segment.Length == bytesConsumed;
+            return false;
         }
 
         internal bool TryGetValue(int index, out Guid value)
@@ -620,8 +632,15 @@ namespace System.Text.Json
 
             Debug.Assert(segment.IndexOf(JsonConstants.BackSlash) == -1);
 
+            if (segment.Length == JsonConstants.MaximumFormatGuidLength
+                && Utf8Parser.TryParse(segment, out Guid tmp, out _, 'D'))
+            {
+                value = tmp;
+                return true;
+            }
+
             value = default;
-            return (segment.Length == JsonConstants.MaximumFormatGuidLength) && Utf8Parser.TryParse(segment, out value, out _, 'D');
+            return false;
         }
 
         internal string GetRawValueAsString(int index)

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderHelper.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderHelper.cs
index f676a43..e8bd2b7 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderHelper.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderHelper.cs
@@ -275,10 +275,16 @@ namespace System.Text.Json
             sourceUnescaped = sourceUnescaped.Slice(0, written);
             Debug.Assert(!sourceUnescaped.IsEmpty);
 
+            if (sourceUnescaped.Length <= JsonConstants.MaximumDateTimeOffsetParseLength
+                && JsonHelpers.TryParseAsISO(sourceUnescaped, out DateTime tmp, out int bytesConsumed)
+                && sourceUnescaped.Length == bytesConsumed)
+            {
+                value = tmp;
+                return true;
+            }
+
             value = default;
-            return (sourceUnescaped.Length <= JsonConstants.MaximumDateTimeOffsetParseLength)
-                && JsonHelpers.TryParseAsISO(sourceUnescaped, out value, out int bytesConsumed)
-                && sourceUnescaped.Length == bytesConsumed;
+            return false;
         }
 
         public static bool TryGetEscapedDateTimeOffset(ReadOnlySpan<byte> source, out DateTimeOffset value)
@@ -295,10 +301,16 @@ namespace System.Text.Json
             sourceUnescaped = sourceUnescaped.Slice(0, written);
             Debug.Assert(!sourceUnescaped.IsEmpty);
 
+            if (sourceUnescaped.Length <= JsonConstants.MaximumDateTimeOffsetParseLength
+                && JsonHelpers.TryParseAsISO(sourceUnescaped, out DateTimeOffset tmp, out int bytesConsumed)
+                && sourceUnescaped.Length == bytesConsumed)
+            {
+                value = tmp;
+                return true;
+            }
+
             value = default;
-            return (sourceUnescaped.Length <= JsonConstants.MaximumDateTimeOffsetParseLength)
-                && JsonHelpers.TryParseAsISO(sourceUnescaped, out value, out int bytesConsumed)
-                && sourceUnescaped.Length == bytesConsumed;
+            return false;
         }
 
         public static bool TryGetEscapedGuid(ReadOnlySpan<byte> source, out Guid value)
@@ -316,8 +328,15 @@ namespace System.Text.Json
             utf8Unescaped = utf8Unescaped.Slice(0, written);
             Debug.Assert(!utf8Unescaped.IsEmpty);
 
+            if (utf8Unescaped.Length == JsonConstants.MaximumFormatGuidLength 
+                && Utf8Parser.TryParse(utf8Unescaped, out Guid tmp, out _, 'D'))
+            {
+                value = tmp;
+                return true;
+            }
+
             value = default;
-            return (utf8Unescaped.Length == JsonConstants.MaximumFormatGuidLength) && Utf8Parser.TryParse(utf8Unescaped, out value, out _, 'D');
+            return false;
         }
     }
 }

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Reader/Utf8JsonReader.TryGet.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Reader/Utf8JsonReader.TryGet.cs
index 2151d40..22b0abb 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Reader/Utf8JsonReader.TryGet.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Reader/Utf8JsonReader.TryGet.cs
@@ -389,7 +389,15 @@ namespace System.Text.Json
             }
 
             ReadOnlySpan<byte> span = HasValueSequence ? ValueSequence.ToArray() : ValueSpan;
-            return Utf8Parser.TryParse(span, out value, out int bytesConsumed) && span.Length == bytesConsumed;
+            if (Utf8Parser.TryParse(span, out int tmp, out int bytesConsumed)
+                && span.Length == bytesConsumed)
+            {
+                value = tmp;
+                return true;
+            }
+
+            value = 0;
+            return false;
         }
 
         /// <summary>
@@ -410,7 +418,15 @@ namespace System.Text.Json
             }
 
             ReadOnlySpan<byte> span = HasValueSequence ? ValueSequence.ToArray() : ValueSpan;
-            return Utf8Parser.TryParse(span, out value, out int bytesConsumed) && span.Length == bytesConsumed;
+            if (Utf8Parser.TryParse(span, out long tmp, out int bytesConsumed) 
+                && span.Length == bytesConsumed)
+            {
+                value = tmp;
+                return true;
+            }
+
+            value = 0;
+            return false;
         }
 
         /// <summary>
@@ -432,7 +448,15 @@ namespace System.Text.Json
             }
 
             ReadOnlySpan<byte> span = HasValueSequence ? ValueSequence.ToArray() : ValueSpan;
-            return Utf8Parser.TryParse(span, out value, out int bytesConsumed) && span.Length == bytesConsumed;
+            if (Utf8Parser.TryParse(span, out uint tmp, out int bytesConsumed)
+                && span.Length == bytesConsumed)
+            {
+                value = tmp;
+                return true;
+            }
+
+            value = 0;
+            return false;
         }
 
         /// <summary>
@@ -454,7 +478,15 @@ namespace System.Text.Json
             }
 
             ReadOnlySpan<byte> span = HasValueSequence ? ValueSequence.ToArray() : ValueSpan;
-            return Utf8Parser.TryParse(span, out value, out int bytesConsumed) && span.Length == bytesConsumed;
+            if (Utf8Parser.TryParse(span, out ulong tmp, out int bytesConsumed) 
+                && span.Length == bytesConsumed)
+            {
+                value = tmp;
+                return true;
+            }
+
+            value = 0;
+            return false;
         }
 
         /// <summary>
@@ -475,7 +507,15 @@ namespace System.Text.Json
             }
 
             ReadOnlySpan<byte> span = HasValueSequence ? ValueSequence.ToArray() : ValueSpan;
-            return Utf8Parser.TryParse(span, out value, out int bytesConsumed, _numberFormat) && span.Length == bytesConsumed;
+            if (Utf8Parser.TryParse(span, out float tmp, out int bytesConsumed, _numberFormat) 
+                && span.Length == bytesConsumed)
+            {
+                value = tmp;
+                return true;
+            }
+
+            value = 0;
+            return false;
         }
 
         /// <summary>
@@ -496,7 +536,15 @@ namespace System.Text.Json
             }
 
             ReadOnlySpan<byte> span = HasValueSequence ? ValueSequence.ToArray() : ValueSpan;
-            return Utf8Parser.TryParse(span, out value, out int bytesConsumed, _numberFormat) && span.Length == bytesConsumed;
+            if (Utf8Parser.TryParse(span, out double tmp, out int bytesConsumed, _numberFormat)
+                && span.Length == bytesConsumed)
+            {
+                value = tmp;
+                return true;
+            }
+
+            value = 0;
+            return false;
         }
 
         /// <summary>
@@ -517,7 +565,15 @@ namespace System.Text.Json
             }
 
             ReadOnlySpan<byte> span = HasValueSequence ? ValueSequence.ToArray() : ValueSpan;
-            return Utf8Parser.TryParse(span, out value, out int bytesConsumed, _numberFormat) && span.Length == bytesConsumed;
+            if (Utf8Parser.TryParse(span, out decimal tmp, out int bytesConsumed, _numberFormat)
+                && span.Length == bytesConsumed)
+            {
+                value = tmp;
+                return true;
+            }
+
+            value = 0;
+            return false;
         }
 
         /// <summary>
@@ -573,10 +629,16 @@ namespace System.Text.Json
 
             Debug.Assert(span.IndexOf(JsonConstants.BackSlash) == -1);
 
+            if (span.Length <= JsonConstants.MaximumDateTimeOffsetParseLength
+                && JsonHelpers.TryParseAsISO(span, out DateTime tmp, out int bytesConsumed)
+                && span.Length == bytesConsumed)
+            {
+                value = tmp;
+                return true;
+            }
+
             value = default;
-            return (span.Length <= JsonConstants.MaximumDateTimeOffsetParseLength)
-                && JsonHelpers.TryParseAsISO(span, out value, out int bytesConsumed)
-                && span.Length == bytesConsumed;
+            return false;
         }
 
         /// <summary>
@@ -632,10 +694,16 @@ namespace System.Text.Json
 
             Debug.Assert(span.IndexOf(JsonConstants.BackSlash) == -1);
 
+            if (span.Length <= JsonConstants.MaximumDateTimeOffsetParseLength
+                && JsonHelpers.TryParseAsISO(span, out DateTimeOffset tmp, out int bytesConsumed)
+                && span.Length == bytesConsumed)
+            {
+                value = tmp;
+                return true;
+            }
+
             value = default;
-            return (span.Length <= JsonConstants.MaximumDateTimeOffsetParseLength)
-                && JsonHelpers.TryParseAsISO(span, out value, out int bytesConsumed)
-                && span.Length == bytesConsumed;
+            return false;
         }
 
         /// <summary>
@@ -691,8 +759,15 @@ namespace System.Text.Json
 
             Debug.Assert(span.IndexOf(JsonConstants.BackSlash) == -1);
 
+            if (span.Length == JsonConstants.MaximumFormatGuidLength
+                && Utf8Parser.TryParse(span, out Guid tmp, out _, 'D'))
+            {
+                value = tmp;
+                return true;
+            }
+
             value = default;
-            return (span.Length == JsonConstants.MaximumFormatGuidLength) && Utf8Parser.TryParse(span, out value, out _, 'D');
+            return false;
         }
     }
 }

diff --git a/src/libraries/System.Text.Json/tests/JsonDocumentTests.cs b/src/libraries/System.Text.Json/tests/JsonDocumentTests.cs
index 1131ddb..5232518 100644 (file)
--- a/src/libraries/System.Text.Json/tests/JsonDocumentTests.cs
+++ b/src/libraries/System.Text.Json/tests/JsonDocumentTests.cs
@@ -1141,7 +1141,9 @@ namespace System.Text.Json.Tests
                 Assert.Equal(JsonValueType.String, root.Type);
 
                 Assert.False(root.TryGetDateTime(out DateTime DateTimeVal));
+                Assert.Equal(default, DateTimeVal);
                 Assert.False(root.TryGetDateTimeOffset(out DateTimeOffset DateTimeOffsetVal));
+                Assert.Equal(default, DateTimeOffsetVal);
 
                 Assert.Throws<FormatException>(() => root.GetDateTime());
                 Assert.Throws<FormatException>(() => root.GetDateTimeOffset());
@@ -1192,6 +1194,7 @@ namespace System.Text.Json.Tests
                 Assert.Equal(JsonValueType.String, root.Type);
 
                 Assert.False(root.TryGetGuid(out Guid GuidVal));
+                Assert.Equal(default, GuidVal);
 
                 Assert.Throws<FormatException>(() => root.GetGuid());
             }
@@ -1641,10 +1644,15 @@ namespace System.Text.Json.Tests
                 const string NotPresent = "Not Present";
                 byte[] notPresentUtf8 = Encoding.UTF8.GetBytes(NotPresent);
 
-                Assert.False(root.TryGetProperty(NotPresent, out _));
-                Assert.False(root.TryGetProperty(NotPresent.AsSpan(), out _));
-                Assert.False(root.TryGetProperty(notPresentUtf8, out _));
-                Assert.False(root.TryGetProperty(new string('z', 512), out _));
+                JsonElement element;
+                Assert.False(root.TryGetProperty(NotPresent, out element));
+                Assert.Equal(default, element);
+                Assert.False(root.TryGetProperty(NotPresent.AsSpan(), out element));
+                Assert.Equal(default, element);
+                Assert.False(root.TryGetProperty(notPresentUtf8, out element));
+                Assert.Equal(default, element);
+                Assert.False(root.TryGetProperty(new string('z', 512), out element));
+                Assert.Equal(default, element);
 
                 Assert.Throws<KeyNotFoundException>(() => root.GetProperty(NotPresent));
                 Assert.Throws<KeyNotFoundException>(() => root.GetProperty(NotPresent.AsSpan()));

diff --git a/src/libraries/System.Text.Json/tests/Utf8JsonReaderTests.TryGet.cs b/src/libraries/System.Text.Json/tests/Utf8JsonReaderTests.TryGet.cs
index 2a2774f..4878a8b 100644 (file)
--- a/src/libraries/System.Text.Json/tests/Utf8JsonReaderTests.TryGet.cs
+++ b/src/libraries/System.Text.Json/tests/Utf8JsonReaderTests.TryGet.cs
@@ -253,6 +253,7 @@ namespace System.Text.Json.Tests
                 if (json.TokenType == JsonTokenType.Number)
                 {
                     Assert.False(json.TryGetInt32(out int value));
+                    Assert.Equal(default, value);
                     Assert.True(json.TryGetDouble(out double doubleValue));
                     Assert.Equal(expected, doubleValue);
 
@@ -291,6 +292,7 @@ namespace System.Text.Json.Tests
                 if (json.TokenType == JsonTokenType.Number)
                 {
                     Assert.False(json.TryGetInt64(out long value));
+                    Assert.Equal(default, value);
                     Assert.True(json.TryGetDouble(out double doubleValue));
                     Assert.Equal(expected, doubleValue);
 
@@ -330,6 +332,7 @@ namespace System.Text.Json.Tests
                 if (json.TokenType == JsonTokenType.Number)
                 {
                     Assert.False(json.TryGetUInt32(out uint value));
+                    Assert.Equal(default, value);
                     Assert.True(json.TryGetDouble(out double doubleValue));
                     Assert.Equal(expected, doubleValue);
 
@@ -368,6 +371,7 @@ namespace System.Text.Json.Tests
                 if (json.TokenType == JsonTokenType.Number)
                 {
                     Assert.False(json.TryGetUInt64(out ulong value));
+                    Assert.Equal(default, value);
                     Assert.True(json.TryGetDouble(out double doubleValue));
                     Assert.Equal(expected, doubleValue);
 
@@ -453,6 +457,7 @@ namespace System.Text.Json.Tests
                 if (json.TokenType == JsonTokenType.Number)
                 {
                     Assert.False(json.TryGetDecimal(out decimal value));
+                    Assert.Equal(default, value);
                     Assert.True(json.TryGetDouble(out double doubleValue));
                     Assert.Equal(expected, doubleValue);
 
@@ -1147,6 +1152,7 @@ namespace System.Text.Json.Tests
             while (json.Read())
             {
                 Assert.False(json.TryGetDateTime(out DateTime actualDateTime));
+                Assert.Equal(default, actualDateTime);
 
                 try
                 {
@@ -1170,6 +1176,7 @@ namespace System.Text.Json.Tests
                 if (json.TokenType == JsonTokenType.String)
                 {
                     Assert.False(json.TryGetDateTimeOffset(out DateTimeOffset actualDateTime));
+                    Assert.Equal(default, actualDateTime);
 
                     try
                     {
@@ -1241,7 +1248,7 @@ namespace System.Text.Json.Tests
             Assert.Equal(JsonTokenType.String, json.TokenType);
 
             Assert.False(json.TryGetGuid(out Guid actual));
-            Assert.Equal(Guid.Empty, actual);
+            Assert.Equal(default, actual);
 
             JsonTestHelper.AssertThrows<FormatException>(json, (jsonReader) => jsonReader.GetGuid());
         }