[Preview 6] Use JsonReader interop with JsonSerializer (#10733)

src/SignalR/clients/csharp/Client/test/FunctionalTests/HubConnectionTests.cs

@@ -1044,8 +1044,8 @@ namespace Microsoft.AspNetCore.SignalR.Client.FunctionalTests
         }
 
         [Theory]
-        [MemberData(nameof(HubProtocolsAndTransportsAndHubPaths))]
+        [MemberData(nameof(HubProtocolsList))]
-        public async Task ServerThrowsHubExceptionOnHubMethodArgumentCountMismatch(string hubProtocolName, HttpTransportType transportType, string hubPath)
+        public async Task ServerThrowsHubExceptionOnHubMethodArgumentCountMismatch(string hubProtocolName)
         {
             bool ExpectedErrors(WriteContext writeContext)
             {
@@ -1056,7 +1056,7 @@ namespace Microsoft.AspNetCore.SignalR.Client.FunctionalTests
             var hubProtocol = HubProtocols[hubProtocolName];
             using (StartServer<Startup>(out var server, ExpectedErrors))
             {
-                var connection = CreateHubConnection(server.Url, hubPath, transportType, hubProtocol, LoggerFactory);
+                var connection = CreateHubConnection(server.Url, "/default", HttpTransportType.LongPolling, hubProtocol, LoggerFactory);
                 try
                 {
                     await connection.StartAsync().OrTimeout();
@@ -1076,7 +1076,7 @@ namespace Microsoft.AspNetCore.SignalR.Client.FunctionalTests
             }
         }
 
-        [Theory(Skip = "Will be fixed by https://github.com/dotnet/corefx/issues/36901")]
+        [Theory]
         [MemberData(nameof(HubProtocolsAndTransportsAndHubPaths))]
         public async Task ServerThrowsHubExceptionOnHubMethodArgumentTypeMismatch(string hubProtocolName, HttpTransportType transportType, string hubPath)
         {
@@ -1873,6 +1873,17 @@ namespace Microsoft.AspNetCore.SignalR.Client.FunctionalTests
             }
         }
 
+        public static IEnumerable<object[]> HubProtocolsList
+        {
+            get
+            {
+                foreach (var protocol in HubProtocols)
+                {
+                    yield return new object[] { protocol.Key };
+                }
+            }
+        }
+
         // This list excludes "special" hub paths like "default-nowebsockets" which exist for specific tests.
         public static string[] HubPaths = new[] { "/default", "/dynamic", "/hubT" };
 

src/SignalR/clients/ts/FunctionalTests/ComplexObject.cs

@@ -10,6 +10,7 @@ namespace FunctionalTests
         public string String { get; set; }
         public int[] IntArray { get; set; }
         public byte[] ByteArray { get; set; }
+        public Guid Guid { get; set; }
         public DateTime DateTime { get;set; }
     }
 }

src/SignalR/clients/ts/FunctionalTests/TestHub.cs

@@ -125,6 +125,7 @@ namespace FunctionalTests
             {
                 ByteArray = new byte[] { 0x1, 0x2, 0x3 },
                 DateTime = new DateTime(2000, 1, 1, 0, 0, 0, DateTimeKind.Utc),
+                Guid = new Guid("00010203-0405-0607-0706-050403020100"),
                 IntArray = new int[] { 1, 2, 3 },
                 String = "hello world",
             };

src/SignalR/clients/ts/FunctionalTests/ts/HubConnectionTests.ts

@@ -462,6 +462,7 @@ describe("hubConnection", () => {
                     DateTime: protocol.name === "json"
                         ? "2002-04-01T10:20:15Z"
                         : new Date(Date.UTC(2002, 3, 1, 10, 20, 15)), // Apr 1, 2002, 10:20:15am UTC
+                    Guid: "00010203-0405-0607-0706-050403020100",
                     IntArray: [0x01, 0x02, 0x03, 0xff],
                     String: "Hello, World!",
                 };
@@ -504,6 +505,7 @@ describe("hubConnection", () => {
                     DateTime: protocol.name === "json"
                         ? "2000-01-01T00:00:00Z"
                         : new Date(Date.UTC(2000, 0, 1)),
+                    Guid: "00010203-0405-0607-0706-050403020100",
                     IntArray: [0x01, 0x02, 0x03],
                     String: "hello world",
                 };

src/SignalR/common/Protocols.Json/src/Microsoft.AspNetCore.SignalR.Protocols.Json.csproj

@@ -1,4 +1,4 @@
-<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
     <Description>Implements the SignalR Hub Protocol using System.Text.Json.</Description>

src/SignalR/common/Protocols.Json/src/Protocol/JsonHubProtocol.cs

@@ -124,9 +124,12 @@ namespace Microsoft.AspNetCore.SignalR.Protocol
                 var hasArguments = false;
                 object[] arguments = null;
                 string[] streamIds = null;
-                JsonDocument argumentsToken = null;
+                bool hasArgumentsToken = false;
-                JsonDocument itemsToken = null;
+                Utf8JsonReader argumentsToken = default;
-                JsonDocument resultToken = null;
+                bool hasItemsToken = false;
+                Utf8JsonReader itemsToken = default;
+                bool hasResultToken = false;
+                Utf8JsonReader resultToken = default;
                 ExceptionDispatchInfo argumentBindingException = null;
                 Dictionary<string, string> headers = null;
                 var completed = false;
@@ -192,15 +195,16 @@ namespace Microsoft.AspNetCore.SignalR.Protocol
 
                                 if (string.IsNullOrEmpty(invocationId))
                                 {
-                                    // If we don't have an invocation id then we need to store it as a JsonDocument so we can parse it later
+                                    // If we don't have an invocation id then we need to value copy the reader so we can parse it later
-                                    resultToken = JsonDocument.ParseValue(ref reader);
+                                    hasResultToken = true;
+                                    resultToken = reader;
+                                    reader.Skip();
                                 }
                                 else
                                 {
                                     // If we have an invocation id already we can parse the end result
                                     var returnType = binder.GetReturnType(invocationId);
-                                    using var token = JsonDocument.ParseValue(ref reader);
+                                    result = BindType(ref reader, returnType);
-                                    result = BindType(token.RootElement, returnType);
                                 }
                             }
                             else if (reader.TextEquals(ItemPropertyNameBytes.EncodedUtf8Bytes))
@@ -216,16 +220,17 @@ namespace Microsoft.AspNetCore.SignalR.Protocol
                                 }
                                 else
                                 {
-                                    // If we don't have an id yet then we need to store it as a JsonDocument to parse later
+                                    // If we don't have an id yet then we need to value copy the reader so we can parse it later
-                                    itemsToken = JsonDocument.ParseValue(ref reader);
+                                    hasItemsToken = true;
+                                    itemsToken = reader;
+                                    reader.Skip();
                                     continue;
                                 }
 
                                 try
                                 {
                                     var itemType = binder.GetStreamItemType(id);
-                                    using var token = JsonDocument.ParseValue(ref reader);
+                                    item = BindType(ref reader, itemType);
-                                    item = BindType(token.RootElement, itemType);
                                 }
                                 catch (Exception ex)
                                 {
@@ -246,16 +251,17 @@ namespace Microsoft.AspNetCore.SignalR.Protocol
 
                                 if (string.IsNullOrEmpty(target))
                                 {
-                                    // We don't know the method name yet so just store the array in JsonDocument
+                                    // We don't know the method name yet so just value copy the reader so we can parse it later
-                                    argumentsToken = JsonDocument.ParseValue(ref reader);
+                                    hasArgumentsToken = true;
+                                    argumentsToken = reader;
+                                    reader.Skip();
                                 }
                                 else
                                 {
                                     try
                                     {
                                         var paramTypes = binder.GetParameterTypes(target);
-                                        using var token = JsonDocument.ParseValue(ref reader);
+                                        arguments = BindTypes(ref reader, paramTypes);
-                                        arguments = BindTypes(token.RootElement, paramTypes);
                                     }
                                     catch (Exception ex)
                                     {
@@ -295,22 +301,18 @@ namespace Microsoft.AspNetCore.SignalR.Protocol
                 {
                     case HubProtocolConstants.InvocationMessageType:
                         {
-                            if (argumentsToken != null)
+                            if (hasArgumentsToken)
                             {
                                 // We weren't able to bind the arguments because they came before the 'target', so try to bind now that we've read everything.
                                 try
                                 {
                                     var paramTypes = binder.GetParameterTypes(target);
-                                    arguments = BindTypes(argumentsToken.RootElement, paramTypes);
+                                    arguments = BindTypes(ref argumentsToken, paramTypes);
                                 }
                                 catch (Exception ex)
                                 {
                                     argumentBindingException = ExceptionDispatchInfo.Capture(ex);
                                 }
-                                finally
-                                {
-                                    argumentsToken.Dispose();
-                                }
                             }
 
                             message = argumentBindingException != null
@@ -320,22 +322,18 @@ namespace Microsoft.AspNetCore.SignalR.Protocol
                         break;
                     case HubProtocolConstants.StreamInvocationMessageType:
                         {
-                            if (argumentsToken != null)
+                            if (hasArgumentsToken)
                             {
                                 // We weren't able to bind the arguments because they came before the 'target', so try to bind now that we've read everything.
                                 try
                                 {
                                     var paramTypes = binder.GetParameterTypes(target);
-                                    arguments = BindTypes(argumentsToken.RootElement, paramTypes);
+                                    arguments = BindTypes(ref argumentsToken, paramTypes);
                                 }
                                 catch (Exception ex)
                                 {
                                     argumentBindingException = ExceptionDispatchInfo.Capture(ex);
                                 }
-                                finally
-                                {
-                                    argumentsToken.Dispose();
-                                }
                             }
 
                             message = argumentBindingException != null
@@ -344,38 +342,27 @@ namespace Microsoft.AspNetCore.SignalR.Protocol
                         }
                         break;
                     case HubProtocolConstants.StreamItemMessageType:
-                        if (itemsToken != null)
+                        if (hasItemsToken)
                         {
                             try
                             {
                                 var returnType = binder.GetStreamItemType(invocationId);
-                                item = BindType(itemsToken.RootElement, returnType);
+                                item = BindType(ref itemsToken, returnType);
                             }
                             catch (JsonException ex)
                             {
                                 message = new StreamBindingFailureMessage(invocationId, ExceptionDispatchInfo.Capture(ex));
                                 break;
                             }
-                            finally
-                            {
-                                itemsToken.Dispose();
-                            }
                         }
 
                         message = BindStreamItemMessage(invocationId, item, hasItem, binder);
                         break;
                     case HubProtocolConstants.CompletionMessageType:
-                        if (resultToken != null)
+                        if (hasResultToken)
                         {
-                            try
+                            var returnType = binder.GetReturnType(invocationId);
-                            {
+                            result = BindType(ref resultToken, returnType);
-                                var returnType = binder.GetReturnType(invocationId);
-                                result = BindType(resultToken.RootElement, returnType);
-                            }
-                            finally
-                            {
-                                resultToken.Dispose();
-                            }
                         }
 
                         message = BindCompletionMessage(invocationId, error, result, hasResult, binder);
@@ -698,48 +685,47 @@ namespace Microsoft.AspNetCore.SignalR.Protocol
             return new InvocationMessage(invocationId, target, arguments, streamIds);
         }
 
-        private object BindType(JsonElement jsonObject, Type type)
+        private object BindType(ref Utf8JsonReader reader, Type type)
         {
-            if (type == typeof(DateTime))
+            return JsonSerializer.ReadValue(ref reader, type, _payloadSerializerOptions);
-            {
-                return jsonObject.GetDateTime();
-            }
-            else if (type == typeof(DateTimeOffset))
-            {
-                return jsonObject.GetDateTimeOffset();
-            }
-
-            return JsonSerializer.Parse(jsonObject.GetRawText(), type, _payloadSerializerOptions);
         }
 
-        private object[] BindTypes(JsonElement jsonArray, IReadOnlyList<Type> paramTypes)
+        private object[] BindTypes(ref Utf8JsonReader reader, IReadOnlyList<Type> paramTypes)
         {
             object[] arguments = null;
             var paramIndex = 0;
-            var argumentsCount = jsonArray.GetArrayLength();
             var paramCount = paramTypes.Count;
 
-            if (argumentsCount != paramCount)
+            var depth = reader.CurrentDepth;
+            reader.CheckRead();
+
+            while (reader.TokenType != JsonTokenType.EndArray && reader.CurrentDepth > depth)
             {
-                throw new InvalidDataException($"Invocation provides {argumentsCount} argument(s) but target expects {paramCount}.");
+                if (paramIndex < paramCount)
+                {
+                    arguments ??= new object[paramCount];
+
+                    try
+                    {
+                        arguments[paramIndex] = BindType(ref reader, paramTypes[paramIndex]);
+                    }
+                    catch (Exception ex)
+                    {
+                        throw new InvalidDataException("Error binding arguments. Make sure that the types of the provided values match the types of the hub method being invoked.", ex);
+                    }
+                }
+                else
+                {
+                    // Skip extra arguments and throw error after reading them all
+                    reader.Skip();
+                }
+                reader.CheckRead();
+                paramIndex++;
             }
 
-            foreach (var element in jsonArray.EnumerateArray())
+            if (paramIndex != paramCount)
             {
-                if (arguments == null)
+                throw new InvalidDataException($"Invocation provides {paramIndex} argument(s) but target expects {paramCount}.");
-                {
-                    arguments = new object[paramCount];
-                }
-
-                try
-                {
-                    arguments[paramIndex] = BindType(element, paramTypes[paramIndex]);
-                    paramIndex++;
-                }
-                catch (Exception ex)
-                {
-                    throw new InvalidDataException("Error binding arguments. Make sure that the types of the provided values match the types of the hub method being invoked.", ex);
-                }
             }
 
             return arguments ?? Array.Empty<object>();

src/SignalR/common/SignalR.Common/test/Internal/Protocol/JsonHubProtocolTestsBase.cs

@@ -205,9 +205,8 @@ namespace Microsoft.AspNetCore.SignalR.Common.Tests.Internal.Protocol
         [InlineData("{\"type\":4,\"invocationId\":\"42\",\"target\":\"foo\",\"arguments\":[ \"abc\", \"xyz\"]}", "Error binding arguments. Make sure that the types of the provided values match the types of the hub method being invoked.")]
         [InlineData("{\"type\":1,\"invocationId\":\"42\",\"target\":\"foo\",\"arguments\":[1,\"\",{\"1\":1,\"2\":2}]}", "Invocation provides 3 argument(s) but target expects 2.")]
         [InlineData("{\"type\":1,\"arguments\":[1,\"\",{\"1\":1,\"2\":2}]},\"invocationId\":\"42\",\"target\":\"foo\"", "Invocation provides 3 argument(s) but target expects 2.")]
-        // Both of these should be fixed by https://github.com/dotnet/corefx/issues/36901
+        [InlineData("{\"type\":1,\"invocationId\":\"42\",\"target\":\"foo\",\"arguments\":[1,[1]]}", "Error binding arguments. Make sure that the types of the provided values match the types of the hub method being invoked.")]
-        // [InlineData("{\"type\":1,\"invocationId\":\"42\",\"target\":\"foo\",\"arguments\":[1,[1]]}", "Error binding arguments. Make sure that the types of the provided values match the types of the hub method being invoked.")]
+        [InlineData("{\"type\":1,\"invocationId\":\"42\",\"target\":\"foo\",\"arguments\":[1,[]]}", "Error binding arguments. Make sure that the types of the provided values match the types of the hub method being invoked.")]
-        // [InlineData("{\"type\":1,\"invocationId\":\"42\",\"target\":\"foo\",\"arguments\":[1,[]]}", "Error binding arguments. Make sure that the types of the provided values match the types of the hub method being invoked.")]
         public void ArgumentBindingErrors(string input, string expectedMessage)
         {
             input = Frame(input);
@@ -219,6 +218,18 @@ namespace Microsoft.AspNetCore.SignalR.Common.Tests.Internal.Protocol
             Assert.Equal(expectedMessage, bindingFailure.BindingFailure.SourceException.Message);
         }
 
+        [Theory]
+        [InlineData("{\"type\":1,\"invocationId\":\"42\",\"target\":\"foo\",\"arguments\":[[}]}")]
+        public void InvalidNestingWhileBindingTypesFails(string input)
+        {
+            input = Frame(input);
+
+            var binder = new TestBinder(paramTypes: new[] { typeof(int[]) }, returnType: null);
+            var data = new ReadOnlySequence<byte>(Encoding.UTF8.GetBytes(input));
+            var ex = Assert.Throws<InvalidDataException>(() => JsonHubProtocol.TryParseMessage(ref data, binder, out var message));
+            Assert.Equal("Error reading JSON.", ex.Message);
+        }
+
         [Theory]
         [InlineData("{\"type\":1,\"invocationId\":\"42\",\"target\":\"foo\",\"arguments\":[\"2007-03-01T13:00:00Z\"]}")]
         [InlineData("{\"type\":1,\"invocationId\":\"42\",\"arguments\":[\"2007-03-01T13:00:00Z\"],\"target\":\"foo\"}")]