Ensure built-in converters can skip JSON values when invoked via custom converters...

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectDefaultConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectDefaultConverter.cs
index 90b3e09..d232055 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectDefaultConverter.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectDefaultConverter.cs
@@ -455,7 +455,14 @@ namespace System.Text.Json.Serialization.Converters
             // Skip the property if not found.
             if (!jsonPropertyInfo.CanDeserializeOrPopulate)
             {
-                reader.Skip();
+                // The Utf8JsonReader.Skip() method will fail fast if it detects that we're reading
+                // from a partially read buffer, regardless of whether the next value is available.
+                // This can result in erroneous failures in cases where a custom converter is calling
+                // into a built-in converter (cf. https://github.com/dotnet/runtime/issues/74108).
+                // For this reason we need to call the TrySkip() method instead -- the serializer
+                // should guarantee sufficient read-ahead has been performed for the current object.
+                bool success = reader.TrySkip();
+                Debug.Assert(success, "Serializer should guarantee sufficient read-ahead has been done.");
             }
             else
             {

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.cs
index 53be31a..304ca0a 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.cs
@@ -306,7 +306,15 @@ namespace System.Text.Json.Serialization.Converters
 
                     if (!(jsonParameterInfo!.ShouldDeserialize))
                     {
-                        reader.Skip();
+                        // The Utf8JsonReader.Skip() method will fail fast if it detects that we're reading
+                        // from a partially read buffer, regardless of whether the next value is available.
+                        // This can result in erroneous failures in cases where a custom converter is calling
+                        // into a built-in converter (cf. https://github.com/dotnet/runtime/issues/74108).
+                        // For this reason we need to call the TrySkip() method instead -- the serializer
+                        // should guarantee sufficient read-ahead has been performed for the current object.
+                        bool success = reader.TrySkip();
+                        Debug.Assert(success, "Serializer should guarantee sufficient read-ahead has been done.");
+
                         state.Current.EndConstructorParameter();
                         continue;
                     }
@@ -359,7 +367,8 @@ namespace System.Text.Json.Serialization.Converters
                             state.Current.JsonPropertyNameAsString);
                     }
 
-                    reader.Skip();
+                    bool success = reader.TrySkip();
+                    Debug.Assert(success, "Serializer should guarantee sufficient read-ahead has been done.");
 
                     state.Current.EndProperty();
                 }

diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/CustomConverterTests/CustomConverterTests.Array.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/CustomConverterTests/CustomConverterTests.Array.cs
index 2d5f921..59fcf64 100644 (file)
--- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/CustomConverterTests/CustomConverterTests.Array.cs
+++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/CustomConverterTests/CustomConverterTests.Array.cs
@@ -2,6 +2,9 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Collections.Generic;
+using System.IO;
+using System.Linq;
+using System.Threading.Tasks;
 using Xunit;
 
 namespace System.Text.Json.Serialization.Tests
@@ -104,5 +107,75 @@ namespace System.Text.Json.Serialization.Tests
             string jsonSerialized = JsonSerializer.Serialize(obj, options);
             Assert.Equal(json, jsonSerialized);
         }
+
+        [Theory]
+        [InlineData(typeof(ProxyClassConverter))]
+        [InlineData(typeof(ProxyClassWithConstructorConverter))]
+        public static async Task ClassWithProxyConverter_WorksWithAsyncDeserialization(Type converterType)
+        {
+            // Regression test for https://github.com/dotnet/runtime/issues/74108
+
+            const int Count = 1000;
+
+            var stream = new MemoryStream();
+            var data = Enumerable.Range(1, Count).Select(_ => new { OtherValue = "otherValue", Value = "value" });
+            await JsonSerializer.SerializeAsync(stream, data);
+            stream.Position = 0;
+
+            var options = new JsonSerializerOptions
+            {
+                Converters = { (JsonConverter)Activator.CreateInstance(converterType) },
+            };
+
+            PocoWithValue[] result = await JsonSerializer.DeserializeAsync<PocoWithValue[]>(stream, options);
+            Assert.Equal(Count, result.Length);
+            Assert.All(result, entry => Assert.Equal("value", entry.Value));
+        }
+
+        class PocoWithValue
+        {
+            public string? Value { get; set; }
+        }
+
+        class ProxyClass
+        {
+            public string? Value { get; set; }
+        }
+
+        class ProxyClassConverter : JsonConverter<PocoWithValue>
+        {
+            private JsonConverter<ProxyClass> GetConverter(JsonSerializerOptions options)
+                => (JsonConverter<ProxyClass>)options.GetConverter(typeof(ProxyClass));
+
+            public override PocoWithValue? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
+            {
+                var proxy = GetConverter(options).Read(ref reader, typeof(ProxyClass), options);
+                return new PocoWithValue { Value = proxy.Value };
+            }
+
+            public override void Write(Utf8JsonWriter writer, PocoWithValue value, JsonSerializerOptions options)
+                => throw new NotImplementedException();
+        }
+
+        class ProxyClassWithConstructor
+        {
+            public ProxyClassWithConstructor(string? value) => Value = value;
+            public string? Value { get; }
+        }
+
+        class ProxyClassWithConstructorConverter : JsonConverter<PocoWithValue>
+        {
+            private JsonConverter<ProxyClassWithConstructor> GetConverter(JsonSerializerOptions options)
+                => (JsonConverter<ProxyClassWithConstructor>)options.GetConverter(typeof(ProxyClassWithConstructor));
+
+            public override PocoWithValue? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
+            {
+                var proxy = GetConverter(options).Read(ref reader, typeof(ProxyClassWithConstructor), options);
+                return new PocoWithValue { Value = proxy.Value };
+            }
+
+            public override void Write(Utf8JsonWriter writer, PocoWithValue value, JsonSerializerOptions options)
+                => throw new NotImplementedException();
+        }
     }
 }