Skip to content

Commit c972489

Browse files
BrennanConroyanalogrelay
BrennanConroy
authored and
analogrelay
committed
[Preview 6] Use JsonReader interop with JsonSerializer (#10733)
1 parent f502017 commit c972489

File tree

7 files changed

+90
-78
lines changed

7 files changed

+90
-78
lines changed

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

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1044,8 +1044,8 @@ bool ExpectedErrors(WriteContext writeContext)
10441044
}
10451045

10461046
[Theory]
1047-
[MemberData(nameof(HubProtocolsAndTransportsAndHubPaths))]
1048-
public async Task ServerThrowsHubExceptionOnHubMethodArgumentCountMismatch(string hubProtocolName, HttpTransportType transportType, string hubPath)
1047+
[MemberData(nameof(HubProtocolsList))]
1048+
public async Task ServerThrowsHubExceptionOnHubMethodArgumentCountMismatch(string hubProtocolName)
10491049
{
10501050
bool ExpectedErrors(WriteContext writeContext)
10511051
{
@@ -1056,7 +1056,7 @@ bool ExpectedErrors(WriteContext writeContext)
10561056
var hubProtocol = HubProtocols[hubProtocolName];
10571057
using (StartServer<Startup>(out var server, ExpectedErrors))
10581058
{
1059-
var connection = CreateHubConnection(server.Url, hubPath, transportType, hubProtocol, LoggerFactory);
1059+
var connection = CreateHubConnection(server.Url, "/default", HttpTransportType.LongPolling, hubProtocol, LoggerFactory);
10601060
try
10611061
{
10621062
await connection.StartAsync().OrTimeout();
@@ -1076,7 +1076,7 @@ bool ExpectedErrors(WriteContext writeContext)
10761076
}
10771077
}
10781078

1079-
[Theory(Skip = "Will be fixed by https://github.com/dotnet/corefx/issues/36901")]
1079+
[Theory]
10801080
[MemberData(nameof(HubProtocolsAndTransportsAndHubPaths))]
10811081
public async Task ServerThrowsHubExceptionOnHubMethodArgumentTypeMismatch(string hubProtocolName, HttpTransportType transportType, string hubPath)
10821082
{
@@ -1873,6 +1873,17 @@ public static IEnumerable<object[]> TransportTypesWithAuth()
18731873
}
18741874
}
18751875

1876+
public static IEnumerable<object[]> HubProtocolsList
1877+
{
1878+
get
1879+
{
1880+
foreach (var protocol in HubProtocols)
1881+
{
1882+
yield return new object[] { protocol.Key };
1883+
}
1884+
}
1885+
}
1886+
18761887
// This list excludes "special" hub paths like "default-nowebsockets" which exist for specific tests.
18771888
public static string[] HubPaths = new[] { "/default", "/dynamic", "/hubT" };
18781889

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ public class ComplexObject
1010
public string String { get; set; }
1111
public int[] IntArray { get; set; }
1212
public byte[] ByteArray { get; set; }
13+
public Guid Guid { get; set; }
1314
public DateTime DateTime { get;set; }
1415
}
1516
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ public ComplexObject SendComplexObject()
125125
{
126126
ByteArray = new byte[] { 0x1, 0x2, 0x3 },
127127
DateTime = new DateTime(2000, 1, 1, 0, 0, 0, DateTimeKind.Utc),
128+
Guid = new Guid("00010203-0405-0607-0706-050403020100"),
128129
IntArray = new int[] { 1, 2, 3 },
129130
String = "hello world",
130131
};

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -462,6 +462,7 @@ describe("hubConnection", () => {
462462
DateTime: protocol.name === "json"
463463
? "2002-04-01T10:20:15Z"
464464
: new Date(Date.UTC(2002, 3, 1, 10, 20, 15)), // Apr 1, 2002, 10:20:15am UTC
465+
Guid: "00010203-0405-0607-0706-050403020100",
465466
IntArray: [0x01, 0x02, 0x03, 0xff],
466467
String: "Hello, World!",
467468
};
@@ -504,6 +505,7 @@ describe("hubConnection", () => {
504505
DateTime: protocol.name === "json"
505506
? "2000-01-01T00:00:00Z"
506507
: new Date(Date.UTC(2000, 0, 1)),
508+
Guid: "00010203-0405-0607-0706-050403020100",
507509
IntArray: [0x01, 0x02, 0x03],
508510
String: "hello world",
509511
};

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<Project Sdk="Microsoft.NET.Sdk">
1+
<Project Sdk="Microsoft.NET.Sdk">
22

33
<PropertyGroup>
44
<Description>Implements the SignalR Hub Protocol using System.Text.Json.</Description>

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

Lines changed: 56 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -124,9 +124,12 @@ private HubMessage ParseMessage(ReadOnlySequence<byte> input, IInvocationBinder
124124
var hasArguments = false;
125125
object[] arguments = null;
126126
string[] streamIds = null;
127-
JsonDocument argumentsToken = null;
128-
JsonDocument itemsToken = null;
129-
JsonDocument resultToken = null;
127+
bool hasArgumentsToken = false;
128+
Utf8JsonReader argumentsToken = default;
129+
bool hasItemsToken = false;
130+
Utf8JsonReader itemsToken = default;
131+
bool hasResultToken = false;
132+
Utf8JsonReader resultToken = default;
130133
ExceptionDispatchInfo argumentBindingException = null;
131134
Dictionary<string, string> headers = null;
132135
var completed = false;
@@ -192,15 +195,16 @@ private HubMessage ParseMessage(ReadOnlySequence<byte> input, IInvocationBinder
192195

193196
if (string.IsNullOrEmpty(invocationId))
194197
{
195-
// If we don't have an invocation id then we need to store it as a JsonDocument so we can parse it later
196-
resultToken = JsonDocument.ParseValue(ref reader);
198+
// If we don't have an invocation id then we need to value copy the reader so we can parse it later
199+
hasResultToken = true;
200+
resultToken = reader;
201+
reader.Skip();
197202
}
198203
else
199204
{
200205
// If we have an invocation id already we can parse the end result
201206
var returnType = binder.GetReturnType(invocationId);
202-
using var token = JsonDocument.ParseValue(ref reader);
203-
result = BindType(token.RootElement, returnType);
207+
result = BindType(ref reader, returnType);
204208
}
205209
}
206210
else if (reader.TextEquals(ItemPropertyNameBytes.EncodedUtf8Bytes))
@@ -216,16 +220,17 @@ private HubMessage ParseMessage(ReadOnlySequence<byte> input, IInvocationBinder
216220
}
217221
else
218222
{
219-
// If we don't have an id yet then we need to store it as a JsonDocument to parse later
220-
itemsToken = JsonDocument.ParseValue(ref reader);
223+
// If we don't have an id yet then we need to value copy the reader so we can parse it later
224+
hasItemsToken = true;
225+
itemsToken = reader;
226+
reader.Skip();
221227
continue;
222228
}
223229

224230
try
225231
{
226232
var itemType = binder.GetStreamItemType(id);
227-
using var token = JsonDocument.ParseValue(ref reader);
228-
item = BindType(token.RootElement, itemType);
233+
item = BindType(ref reader, itemType);
229234
}
230235
catch (Exception ex)
231236
{
@@ -246,16 +251,17 @@ private HubMessage ParseMessage(ReadOnlySequence<byte> input, IInvocationBinder
246251

247252
if (string.IsNullOrEmpty(target))
248253
{
249-
// We don't know the method name yet so just store the array in JsonDocument
250-
argumentsToken = JsonDocument.ParseValue(ref reader);
254+
// We don't know the method name yet so just value copy the reader so we can parse it later
255+
hasArgumentsToken = true;
256+
argumentsToken = reader;
257+
reader.Skip();
251258
}
252259
else
253260
{
254261
try
255262
{
256263
var paramTypes = binder.GetParameterTypes(target);
257-
using var token = JsonDocument.ParseValue(ref reader);
258-
arguments = BindTypes(token.RootElement, paramTypes);
264+
arguments = BindTypes(ref reader, paramTypes);
259265
}
260266
catch (Exception ex)
261267
{
@@ -295,22 +301,18 @@ private HubMessage ParseMessage(ReadOnlySequence<byte> input, IInvocationBinder
295301
{
296302
case HubProtocolConstants.InvocationMessageType:
297303
{
298-
if (argumentsToken != null)
304+
if (hasArgumentsToken)
299305
{
300306
// We weren't able to bind the arguments because they came before the 'target', so try to bind now that we've read everything.
301307
try
302308
{
303309
var paramTypes = binder.GetParameterTypes(target);
304-
arguments = BindTypes(argumentsToken.RootElement, paramTypes);
310+
arguments = BindTypes(ref argumentsToken, paramTypes);
305311
}
306312
catch (Exception ex)
307313
{
308314
argumentBindingException = ExceptionDispatchInfo.Capture(ex);
309315
}
310-
finally
311-
{
312-
argumentsToken.Dispose();
313-
}
314316
}
315317

316318
message = argumentBindingException != null
@@ -320,22 +322,18 @@ private HubMessage ParseMessage(ReadOnlySequence<byte> input, IInvocationBinder
320322
break;
321323
case HubProtocolConstants.StreamInvocationMessageType:
322324
{
323-
if (argumentsToken != null)
325+
if (hasArgumentsToken)
324326
{
325327
// We weren't able to bind the arguments because they came before the 'target', so try to bind now that we've read everything.
326328
try
327329
{
328330
var paramTypes = binder.GetParameterTypes(target);
329-
arguments = BindTypes(argumentsToken.RootElement, paramTypes);
331+
arguments = BindTypes(ref argumentsToken, paramTypes);
330332
}
331333
catch (Exception ex)
332334
{
333335
argumentBindingException = ExceptionDispatchInfo.Capture(ex);
334336
}
335-
finally
336-
{
337-
argumentsToken.Dispose();
338-
}
339337
}
340338

341339
message = argumentBindingException != null
@@ -344,38 +342,27 @@ private HubMessage ParseMessage(ReadOnlySequence<byte> input, IInvocationBinder
344342
}
345343
break;
346344
case HubProtocolConstants.StreamItemMessageType:
347-
if (itemsToken != null)
345+
if (hasItemsToken)
348346
{
349347
try
350348
{
351349
var returnType = binder.GetStreamItemType(invocationId);
352-
item = BindType(itemsToken.RootElement, returnType);
350+
item = BindType(ref itemsToken, returnType);
353351
}
354352
catch (JsonException ex)
355353
{
356354
message = new StreamBindingFailureMessage(invocationId, ExceptionDispatchInfo.Capture(ex));
357355
break;
358356
}
359-
finally
360-
{
361-
itemsToken.Dispose();
362-
}
363357
}
364358

365359
message = BindStreamItemMessage(invocationId, item, hasItem, binder);
366360
break;
367361
case HubProtocolConstants.CompletionMessageType:
368-
if (resultToken != null)
362+
if (hasResultToken)
369363
{
370-
try
371-
{
372-
var returnType = binder.GetReturnType(invocationId);
373-
result = BindType(resultToken.RootElement, returnType);
374-
}
375-
finally
376-
{
377-
resultToken.Dispose();
378-
}
364+
var returnType = binder.GetReturnType(invocationId);
365+
result = BindType(ref resultToken, returnType);
379366
}
380367

381368
message = BindCompletionMessage(invocationId, error, result, hasResult, binder);
@@ -698,48 +685,47 @@ private HubMessage BindInvocationMessage(string invocationId, string target, obj
698685
return new InvocationMessage(invocationId, target, arguments, streamIds);
699686
}
700687

701-
private object BindType(JsonElement jsonObject, Type type)
688+
private object BindType(ref Utf8JsonReader reader, Type type)
702689
{
703-
if (type == typeof(DateTime))
704-
{
705-
return jsonObject.GetDateTime();
706-
}
707-
else if (type == typeof(DateTimeOffset))
708-
{
709-
return jsonObject.GetDateTimeOffset();
710-
}
711-
712-
return JsonSerializer.Parse(jsonObject.GetRawText(), type, _payloadSerializerOptions);
690+
return JsonSerializer.ReadValue(ref reader, type, _payloadSerializerOptions);
713691
}
714692

715-
private object[] BindTypes(JsonElement jsonArray, IReadOnlyList<Type> paramTypes)
693+
private object[] BindTypes(ref Utf8JsonReader reader, IReadOnlyList<Type> paramTypes)
716694
{
717695
object[] arguments = null;
718696
var paramIndex = 0;
719-
var argumentsCount = jsonArray.GetArrayLength();
720697
var paramCount = paramTypes.Count;
721698

722-
if (argumentsCount != paramCount)
723-
{
724-
throw new InvalidDataException($"Invocation provides {argumentsCount} argument(s) but target expects {paramCount}.");
725-
}
699+
var depth = reader.CurrentDepth;
700+
reader.CheckRead();
726701

727-
foreach (var element in jsonArray.EnumerateArray())
702+
while (reader.TokenType != JsonTokenType.EndArray && reader.CurrentDepth > depth)
728703
{
729-
if (arguments == null)
704+
if (paramIndex < paramCount)
730705
{
731-
arguments = new object[paramCount];
732-
}
706+
arguments ??= new object[paramCount];
733707

734-
try
735-
{
736-
arguments[paramIndex] = BindType(element, paramTypes[paramIndex]);
737-
paramIndex++;
708+
try
709+
{
710+
arguments[paramIndex] = BindType(ref reader, paramTypes[paramIndex]);
711+
}
712+
catch (Exception ex)
713+
{
714+
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);
715+
}
738716
}
739-
catch (Exception ex)
717+
else
740718
{
741-
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);
719+
// Skip extra arguments and throw error after reading them all
720+
reader.Skip();
742721
}
722+
reader.CheckRead();
723+
paramIndex++;
724+
}
725+
726+
if (paramIndex != paramCount)
727+
{
728+
throw new InvalidDataException($"Invocation provides {paramIndex} argument(s) but target expects {paramCount}.");
743729
}
744730

745731
return arguments ?? Array.Empty<object>();

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

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -205,9 +205,8 @@ public void ExtraItemsInMessageAreIgnored(string input)
205205
[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.")]
206206
[InlineData("{\"type\":1,\"invocationId\":\"42\",\"target\":\"foo\",\"arguments\":[1,\"\",{\"1\":1,\"2\":2}]}", "Invocation provides 3 argument(s) but target expects 2.")]
207207
[InlineData("{\"type\":1,\"arguments\":[1,\"\",{\"1\":1,\"2\":2}]},\"invocationId\":\"42\",\"target\":\"foo\"", "Invocation provides 3 argument(s) but target expects 2.")]
208-
// Both of these should be fixed by https://github.com/dotnet/corefx/issues/36901
209-
// [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.")]
210-
// [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.")]
208+
[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.")]
209+
[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.")]
211210
public void ArgumentBindingErrors(string input, string expectedMessage)
212211
{
213212
input = Frame(input);
@@ -219,6 +218,18 @@ public void ArgumentBindingErrors(string input, string expectedMessage)
219218
Assert.Equal(expectedMessage, bindingFailure.BindingFailure.SourceException.Message);
220219
}
221220

221+
[Theory]
222+
[InlineData("{\"type\":1,\"invocationId\":\"42\",\"target\":\"foo\",\"arguments\":[[}]}")]
223+
public void InvalidNestingWhileBindingTypesFails(string input)
224+
{
225+
input = Frame(input);
226+
227+
var binder = new TestBinder(paramTypes: new[] { typeof(int[]) }, returnType: null);
228+
var data = new ReadOnlySequence<byte>(Encoding.UTF8.GetBytes(input));
229+
var ex = Assert.Throws<InvalidDataException>(() => JsonHubProtocol.TryParseMessage(ref data, binder, out var message));
230+
Assert.Equal("Error reading JSON.", ex.Message);
231+
}
232+
222233
[Theory]
223234
[InlineData("{\"type\":1,\"invocationId\":\"42\",\"target\":\"foo\",\"arguments\":[\"2007-03-01T13:00:00Z\"]}")]
224235
[InlineData("{\"type\":1,\"invocationId\":\"42\",\"arguments\":[\"2007-03-01T13:00:00Z\"],\"target\":\"foo\"}")]

0 commit comments

Comments
 (0)