From b792fcb4efcfb32daab4370b6d45d85fd7f70f08 Mon Sep 17 00:00:00 2001 From: David Fowler Date: Sun, 18 Mar 2018 15:16:03 -0700 Subject: [PATCH] Reduce the per message read allocations (#1635) - Introduced Utf8BufferTextReader that writes buffers directly into the char[] allocated by JSON.NET when reading via the JsonReader. - Use IArrayPool implementation over ArrayPool when reading incomming messages. - Replaced JToken parsing with manual parsing using JsonTextReader. - Added tests for parsing incoming JSON messages with out of order properties. - Make access to message headers lazy - Changed IHubProtocol.TryParseMessage to be ReadOnlyMemory instead of ReadOnlySpan --- .../DefaultHubDispatcherBenchmark.cs | 2 +- .../MessageParserBenchmark.cs | 4 +- client-ts/FunctionalTests/package-lock.json | 28 +- .../Formatters/BinaryMessageParser.cs | 6 +- .../Internal/Formatters/TextMessageParser.cs | 4 +- .../Internal/Protocol/HubInvocationMessage.cs | 10 +- .../Internal/Protocol/IHubProtocol.cs | 2 +- .../Internal/Protocol/JsonArrayPool.cs | 28 ++ .../Internal/Protocol/JsonHubProtocol.cs | 473 +++++++++++++----- .../Internal/Protocol/JsonUtils.cs | 58 +++ .../Internal/Protocol/NegotiationProtocol.cs | 27 +- .../Internal/Protocol/Utf8BufferTextReader.cs | 54 ++ .../Protocol/MessagePackHubProtocol.cs | 11 +- .../HubConnectionTests.cs | 2 +- .../Formatters/BinaryMessageFormatterTests.cs | 2 +- .../Formatters/BinaryMessageParserTests.cs | 12 +- .../Formatters/TextMessageParserTests.cs | 8 +- .../Internal/Protocol/HubMessageHelpers.cs | 6 + .../Internal/Protocol/JsonHubProtocolTests.cs | 25 + .../Protocol/MessagePackHubProtocolTests.cs | 4 +- 20 files changed, 578 insertions(+), 188 deletions(-) create mode 100644 src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/JsonArrayPool.cs create mode 100644 src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/Utf8BufferTextReader.cs diff --git a/benchmarks/Microsoft.AspNetCore.SignalR.Microbenchmarks/DefaultHubDispatcherBenchmark.cs b/benchmarks/Microsoft.AspNetCore.SignalR.Microbenchmarks/DefaultHubDispatcherBenchmark.cs index ca1ef0f128..341a1959de 100644 --- a/benchmarks/Microsoft.AspNetCore.SignalR.Microbenchmarks/DefaultHubDispatcherBenchmark.cs +++ b/benchmarks/Microsoft.AspNetCore.SignalR.Microbenchmarks/DefaultHubDispatcherBenchmark.cs @@ -52,7 +52,7 @@ namespace Microsoft.AspNetCore.SignalR.Microbenchmarks public string Name { get; } public TransferFormat TransferFormat { get; } - public bool TryParseMessages(ReadOnlySpan input, IInvocationBinder binder, IList messages) + public bool TryParseMessages(ReadOnlyMemory input, IInvocationBinder binder, IList messages) { return false; } diff --git a/benchmarks/Microsoft.AspNetCore.SignalR.Microbenchmarks/MessageParserBenchmark.cs b/benchmarks/Microsoft.AspNetCore.SignalR.Microbenchmarks/MessageParserBenchmark.cs index a854ae7659..f05fd178da 100644 --- a/benchmarks/Microsoft.AspNetCore.SignalR.Microbenchmarks/MessageParserBenchmark.cs +++ b/benchmarks/Microsoft.AspNetCore.SignalR.Microbenchmarks/MessageParserBenchmark.cs @@ -40,7 +40,7 @@ namespace Microsoft.AspNetCore.SignalR.Microbenchmarks [Benchmark] public void SingleBinaryMessage() { - ReadOnlySpan buffer = _binaryInput; + ReadOnlyMemory buffer = _binaryInput; if (!BinaryMessageParser.TryParseMessage(ref buffer, out _)) { throw new InvalidOperationException("Failed to parse"); @@ -50,7 +50,7 @@ namespace Microsoft.AspNetCore.SignalR.Microbenchmarks [Benchmark] public void SingleTextMessage() { - ReadOnlySpan buffer = _textInput; + ReadOnlyMemory buffer = _textInput; if (!TextMessageParser.TryParseMessage(ref buffer, out _)) { throw new InvalidOperationException("Failed to parse"); diff --git a/client-ts/FunctionalTests/package-lock.json b/client-ts/FunctionalTests/package-lock.json index ed6372b193..190e3dc831 100644 --- a/client-ts/FunctionalTests/package-lock.json +++ b/client-ts/FunctionalTests/package-lock.json @@ -7,13 +7,13 @@ "@std/esm": { "version": "0.18.0", "resolved": "https://registry.npmjs.org/@std/esm/-/esm-0.18.0.tgz", - "integrity": "sha512-oeHSSVp/WxC08ngpKgyYR4LcI0+EBwZiJcB58jvIqyJnOGxudSkxTgAQKsVfpNsMXfOoILgu9PWhuzIZ8GQEjw==", + "integrity": "sha1-4hK1Zcdl+Tsp7FIqSToZLOeuK6Y=", "dev": true }, "@types/debug": { "version": "0.0.30", "resolved": "https://registry.npmjs.org/@types/debug/-/debug-0.0.30.tgz", - "integrity": "sha512-orGL5LXERPYsLov6CWs3Fh6203+dXzJkR7OnddIr2514Hsecwc8xRpzCapshBbKFImCsvS/mk6+FWiN5LyZJAQ==", + "integrity": "sha1-3B5A9687nIFQE6eGDmJS9jUqhN8=", "dev": true }, "@types/node": { @@ -31,7 +31,7 @@ "@types/strip-json-comments": { "version": "0.0.30", "resolved": "https://registry.npmjs.org/@types/strip-json-comments/-/strip-json-comments-0.0.30.tgz", - "integrity": "sha512-7NQmHra/JILCd1QqpSzl8+mJRc8ZHz3uDm8YV1Ks9IhK0epEiTw8aIErbvH9PI+6XbqhyIQy3462nEsn7UVzjQ==", + "integrity": "sha1-mqMMBNshKpoGSdaub9UKzMQHSKE=", "dev": true }, "ansi-styles": { @@ -69,7 +69,7 @@ "color-convert": { "version": "1.9.1", "resolved": "https://registry.npmjs.org/color-convert/-/color-convert-1.9.1.tgz", - "integrity": "sha512-mjGanIiwQJskCC18rPR6OmrZ6fm2Lc7PeGFYwCmy5J34wC6F1PzdGL6xeMfmgicfYcNLGuVFA3WzXtIDCQSZxQ==", + "integrity": "sha1-wSYRB66y8pTr/+ye2eytUppgl+0=", "dev": true, "requires": { "color-name": "1.1.3" @@ -90,7 +90,7 @@ "debug": { "version": "3.1.0", "resolved": "https://registry.npmjs.org/debug/-/debug-3.1.0.tgz", - "integrity": "sha512-OX8XqP7/1a9cqkxYw2yXss15f26NKWBpDXQd0/uK/KPqdQhxbPa994hnzjcE2VqQpDslf55723cKPUOGSmMY3g==", + "integrity": "sha1-W7WgZyYotkFJVmuhaBnmFRjGcmE=", "dev": true, "requires": { "ms": "2.0.0" @@ -300,19 +300,19 @@ "safe-buffer": { "version": "5.1.1", "resolved": "https://registry.npmjs.org/safe-buffer/-/safe-buffer-5.1.1.tgz", - "integrity": "sha512-kKvNJn6Mm93gAczWVJg7wH+wGYWNrDHdWvpUmHyEsgCtIwwo3bqPtV4tR5tuPaUhTOo/kvhVwd8XwwOllGYkbg==", + "integrity": "sha1-iTMSr2myEj3vcfV4iQAWce6yyFM=", "dev": true }, "source-map": { "version": "0.6.1", "resolved": "https://registry.npmjs.org/source-map/-/source-map-0.6.1.tgz", - "integrity": "sha512-UjgapumWlbMhkBgzT7Ykc5YXUT46F0iKu8SGXq0bcwP5dz/h0Plj6enJqjz1Zbq2l5WaqYnrVbwWOWMyF3F47g==", + "integrity": "sha1-dHIq8y6WFOnCh6jQu95IteLxomM=", "dev": true }, "source-map-support": { "version": "0.5.3", "resolved": "https://registry.npmjs.org/source-map-support/-/source-map-support-0.5.3.tgz", - "integrity": "sha512-eKkTgWYeBOQqFGXRfKabMFdnWepo51vWqEdoeikaEPFiJC7MCU5j2h4+6Q8npkZTeLGbSyecZvRxiSoWl3rh+w==", + "integrity": "sha1-Kz1f/ymM+k0a/X1DUtVp6aAVjnY=", "dev": true, "requires": { "source-map": "0.6.1" @@ -321,7 +321,7 @@ "split": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/split/-/split-1.0.1.tgz", - "integrity": "sha512-mTyOoPbrivtXnwnIxZRFYRrPNtEFKlpB2fvjSnCQUiAA6qAZzqwna5envK4uk6OIeP17CsdF3rSBGYVBsU0Tkg==", + "integrity": "sha1-YFvZvjA6pZ+zX5Ip++oN3snqB9k=", "dev": true, "requires": { "through": "2.3.8" @@ -410,7 +410,7 @@ "process-nextick-args": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/process-nextick-args/-/process-nextick-args-2.0.0.tgz", - "integrity": "sha512-MtEC1TqN0EU5nephaJ4rAtThHtC86dNN9qCuEhtshvpVBkAW5ZO7BASN9REnF9eoXGcRub+pFuKEpOHE+HbEMw==", + "integrity": "sha1-o31zL0JxtKsa0HDTVQjoKQeI/6o=", "dev": true }, "readable-stream": { @@ -431,7 +431,7 @@ "string_decoder": { "version": "1.0.3", "resolved": "https://registry.npmjs.org/string_decoder/-/string_decoder-1.0.3.tgz", - "integrity": "sha512-4AH6Z5fzNNBcH+6XDMfA/BTt87skxqJlO0lAh3Dker5zThcAxG6mKz+iGu308UKoPPQ8Dcqx/4JhujzltRa+hQ==", + "integrity": "sha1-D8Z9fBQYJd6UKC3VNr7GubzoYKs=", "dev": true, "requires": { "safe-buffer": "5.1.1" @@ -473,7 +473,7 @@ "tap-teamcity": { "version": "3.0.2", "resolved": "https://registry.npmjs.org/tap-teamcity/-/tap-teamcity-3.0.2.tgz", - "integrity": "sha512-FI26a4CGNx9LWx2vRh3fLNrel1GJm5smBVJl2tzabTwGrmX9d+KHWP2O9xdKgMtH5IOBpN6goy9Yh4P2NRaoQw==", + "integrity": "sha1-727fs5PvMX09DcOZHugkurlIw54=", "dev": true, "requires": { "@std/esm": "0.18.0", @@ -603,7 +603,7 @@ "ts-node": { "version": "4.1.0", "resolved": "https://registry.npmjs.org/ts-node/-/ts-node-4.1.0.tgz", - "integrity": "sha512-xcZH12oVg9PShKhy3UHyDmuDLV3y7iKwX25aMVPt1SIXSuAfWkFiGPEkg+th8R4YKW/QCxDoW7lJdb15lx6QWg==", + "integrity": "sha1-NtlSnHuQu5kzBsQIzQf3dD3iBxI=", "dev": true, "requires": { "arrify": "1.0.1", @@ -621,7 +621,7 @@ "tsconfig": { "version": "7.0.0", "resolved": "https://registry.npmjs.org/tsconfig/-/tsconfig-7.0.0.tgz", - "integrity": "sha512-vZXmzPrL+EmC4T/4rVlT2jNVMWCi/O4DIiSj3UHg1OE5kCKbk4mfrXc6dZksLgRM/TZlKnousKH9bbTazUWRRw==", + "integrity": "sha1-hFOIdaTcIW5cSlQys6Tew9VOkbc=", "dev": true, "requires": { "@types/strip-bom": "3.0.0", diff --git a/src/Microsoft.AspNetCore.SignalR.Common/Internal/Formatters/BinaryMessageParser.cs b/src/Microsoft.AspNetCore.SignalR.Common/Internal/Formatters/BinaryMessageParser.cs index cfeb9a2756..4d80c937b7 100644 --- a/src/Microsoft.AspNetCore.SignalR.Common/Internal/Formatters/BinaryMessageParser.cs +++ b/src/Microsoft.AspNetCore.SignalR.Common/Internal/Formatters/BinaryMessageParser.cs @@ -9,7 +9,7 @@ namespace Microsoft.AspNetCore.SignalR.Internal.Formatters { private const int MaxLengthPrefixSize = 5; - public static bool TryParseMessage(ref ReadOnlySpan buffer, out ReadOnlySpan payload) + public static bool TryParseMessage(ref ReadOnlyMemory buffer, out ReadOnlyMemory payload) { if (buffer.IsEmpty) { @@ -33,10 +33,12 @@ namespace Microsoft.AspNetCore.SignalR.Internal.Formatters var numBytes = 0; var lengthPrefixBuffer = buffer.Slice(0, Math.Min(MaxLengthPrefixSize, buffer.Length)); + var span = lengthPrefixBuffer.Span; + byte byteRead; do { - byteRead = lengthPrefixBuffer[numBytes]; + byteRead = span[numBytes]; length = length | (((uint)(byteRead & 0x7f)) << (numBytes * 7)); numBytes++; } diff --git a/src/Microsoft.AspNetCore.SignalR.Common/Internal/Formatters/TextMessageParser.cs b/src/Microsoft.AspNetCore.SignalR.Common/Internal/Formatters/TextMessageParser.cs index efd79e3586..1cdfe687a6 100644 --- a/src/Microsoft.AspNetCore.SignalR.Common/Internal/Formatters/TextMessageParser.cs +++ b/src/Microsoft.AspNetCore.SignalR.Common/Internal/Formatters/TextMessageParser.cs @@ -7,9 +7,9 @@ namespace Microsoft.AspNetCore.SignalR.Internal.Formatters { public static class TextMessageParser { - public static bool TryParseMessage(ref ReadOnlySpan buffer, out ReadOnlySpan payload) + public static bool TryParseMessage(ref ReadOnlyMemory buffer, out ReadOnlyMemory payload) { - var index = buffer.IndexOf(TextMessageFormatter.RecordSeparator); + var index = buffer.Span.IndexOf(TextMessageFormatter.RecordSeparator); if (index == -1) { payload = default; diff --git a/src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/HubInvocationMessage.cs b/src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/HubInvocationMessage.cs index 1d9ee3c73a..830fdbfc57 100644 --- a/src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/HubInvocationMessage.cs +++ b/src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/HubInvocationMessage.cs @@ -7,15 +7,7 @@ namespace Microsoft.AspNetCore.SignalR.Internal.Protocol { public abstract class HubInvocationMessage : HubMessage { - private Dictionary _headers; - - public IDictionary Headers - { - get - { - return _headers ?? (_headers = new Dictionary()); - } - } + public IDictionary Headers { get; set; } public string InvocationId { get; } diff --git a/src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/IHubProtocol.cs b/src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/IHubProtocol.cs index f5651bfab0..b47f5dcf30 100644 --- a/src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/IHubProtocol.cs +++ b/src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/IHubProtocol.cs @@ -14,7 +14,7 @@ namespace Microsoft.AspNetCore.SignalR.Internal.Protocol TransferFormat TransferFormat { get; } - bool TryParseMessages(ReadOnlySpan input, IInvocationBinder binder, IList messages); + bool TryParseMessages(ReadOnlyMemory input, IInvocationBinder binder, IList messages); void WriteMessage(HubMessage message, Stream output); } diff --git a/src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/JsonArrayPool.cs b/src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/JsonArrayPool.cs new file mode 100644 index 0000000000..057f7a5e44 --- /dev/null +++ b/src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/JsonArrayPool.cs @@ -0,0 +1,28 @@ +using System; +using System.Buffers; +using Newtonsoft.Json; + +namespace Microsoft.AspNetCore.SignalR.Internal.Protocol +{ + internal class JsonArrayPool : IArrayPool + { + private readonly ArrayPool _inner; + + internal static readonly JsonArrayPool Shared = new JsonArrayPool(ArrayPool.Shared); + + public JsonArrayPool(ArrayPool inner) + { + _inner = inner; + } + + public T[] Rent(int minimumLength) + { + return _inner.Rent(minimumLength); + } + + public void Return(T[] array) + { + _inner.Return(array); + } + } +} diff --git a/src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/JsonHubProtocol.cs b/src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/JsonHubProtocol.cs index d9c49a5d7c..cc964a9940 100644 --- a/src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/JsonHubProtocol.cs +++ b/src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/JsonHubProtocol.cs @@ -47,15 +47,12 @@ namespace Microsoft.AspNetCore.SignalR.Internal.Protocol public TransferFormat TransferFormat => TransferFormat.Text; - public bool TryParseMessages(ReadOnlySpan input, IInvocationBinder binder, IList messages) + public bool TryParseMessages(ReadOnlyMemory input, IInvocationBinder binder, IList messages) { while (TextMessageParser.TryParseMessage(ref input, out var payload)) { - // TODO: Need a span-native JSON parser! - using (var memoryStream = new MemoryStream(payload.ToArray())) - { - messages.Add(ParseMessage(memoryStream, binder)); - } + var textReader = new Utf8BufferTextReader(payload); + messages.Add(ParseMessage(textReader, binder)); } return messages.Count > 0; @@ -67,69 +64,264 @@ namespace Microsoft.AspNetCore.SignalR.Internal.Protocol TextMessageFormatter.WriteRecordSeparator(output); } - private HubMessage ParseMessage(Stream input, IInvocationBinder binder) + private HubMessage ParseMessage(TextReader textReader, IInvocationBinder binder) { - using (var reader = new JsonTextReader(new StreamReader(input))) + try { - try - { - // PERF: Could probably use the JsonTextReader directly for better perf and fewer allocations - var token = JToken.ReadFrom(reader); + // We parse using the JsonTextReader directly but this has a problem. Some of our properties are dependent on other properties + // and since reading the json might be unordered, we need to store the parsed content as JToken to re-parse when true types are known. + // if we're lucky and the state we need to directly parse is available, then we'll use it. - if (token == null || token.Type != JTokenType.Object) + int? type = null; + string invocationId = null; + string target = null; + string error = null; + var hasItem = false; + object item = null; + JToken itemToken = null; + var hasResult = false; + object result = null; + JToken resultToken = null; + bool hasArguments = false; + object[] arguments = null; + JArray argumentsToken = null; + ExceptionDispatchInfo argumentBindingException = null; + Dictionary headers = null; + var completed = false; + + using (var reader = new JsonTextReader(textReader)) + { + reader.ArrayPool = JsonArrayPool.Shared; + + JsonUtils.CheckRead(reader); + + // We're always parsing a JSON object + if (reader.TokenType != JsonToken.StartObject) { - throw new InvalidDataException($"Unexpected JSON Token Type '{token?.Type}'. Expected a JSON Object."); + throw new InvalidDataException($"Unexpected JSON Token Type '{JsonUtils.GetTokenString(reader.TokenType)}'. Expected a JSON Object."); } - var json = (JObject)token; - - // Determine the type of the message - var type = JsonUtils.GetRequiredProperty(json, TypePropertyName, JTokenType.Integer); - - switch (type) + do { - case HubProtocolConstants.InvocationMessageType: - return BindInvocationMessage(json, binder); - case HubProtocolConstants.StreamInvocationMessageType: - return BindStreamInvocationMessage(json, binder); - case HubProtocolConstants.StreamItemMessageType: - return BindStreamItemMessage(json, binder); - case HubProtocolConstants.CompletionMessageType: - return BindCompletionMessage(json, binder); - case HubProtocolConstants.CancelInvocationMessageType: - return BindCancelInvocationMessage(json); - case HubProtocolConstants.PingMessageType: - return PingMessage.Instance; - default: - throw new InvalidDataException($"Unknown message type: {type}"); + switch (reader.TokenType) + { + case JsonToken.PropertyName: + string memberName = reader.Value.ToString(); + + switch (memberName) + { + case TypePropertyName: + var messageType = JsonUtils.ReadAsInt32(reader, TypePropertyName); + + if (messageType == null) + { + throw new InvalidDataException($"Missing required property '{TypePropertyName}'."); + } + + type = messageType.Value; + break; + case InvocationIdPropertyName: + invocationId = JsonUtils.ReadAsString(reader, InvocationIdPropertyName); + break; + case TargetPropertyName: + target = JsonUtils.ReadAsString(reader, TargetPropertyName); + break; + case ErrorPropertyName: + error = JsonUtils.ReadAsString(reader, ErrorPropertyName); + break; + case ResultPropertyName: + JsonUtils.CheckRead(reader); + + hasResult = true; + + if (string.IsNullOrEmpty(invocationId)) + { + // If we don't have an invocation id then we need to store it as a JToken so we can parse it later + resultToken = JToken.Load(reader); + } + else + { + // If we have an invocation id already we can parse the end result + var returnType = binder.GetReturnType(invocationId); + result = PayloadSerializer.Deserialize(reader, returnType); + } + break; + case ItemPropertyName: + JsonUtils.CheckRead(reader); + + hasItem = true; + + if (string.IsNullOrEmpty(invocationId)) + { + // If we don't have an invocation id then we need to store it as a JToken so we can parse it later + itemToken = JToken.Load(reader); + } + else + { + var returnType = binder.GetReturnType(invocationId); + item = PayloadSerializer.Deserialize(reader, returnType); + } + break; + case ArgumentsPropertyName: + JsonUtils.CheckRead(reader); + + if (reader.TokenType != JsonToken.StartArray) + { + throw new InvalidDataException($"Expected '{ArgumentsPropertyName}' to be of type {JTokenType.Array}."); + } + + hasArguments = true; + + if (string.IsNullOrEmpty(target)) + { + // We don't know the method name yet so just parse an array of generic JArray + argumentsToken = JArray.Load(reader); + } + else + { + try + { + var paramTypes = binder.GetParameterTypes(target); + arguments = BindArguments(reader, paramTypes); + } + catch (Exception ex) + { + argumentBindingException = ExceptionDispatchInfo.Capture(ex); + } + } + break; + case HeadersPropertyName: + JsonUtils.CheckRead(reader); + headers = ReadHeaders(reader); + break; + default: + // Skip read the property name + JsonUtils.CheckRead(reader); + // Skip the value for this property + reader.Skip(); + break; + } + break; + case JsonToken.EndObject: + completed = true; + break; + default: + break; + } } + while (!completed && JsonUtils.CheckRead(reader)); } - catch (JsonReaderException jrex) + + HubMessage message = null; + + switch (type) { - throw new InvalidDataException("Error reading JSON.", jrex); + case HubProtocolConstants.InvocationMessageType: + { + if (argumentsToken != null) + { + try + { + var paramTypes = binder.GetParameterTypes(target); + arguments = BindArguments(argumentsToken, paramTypes); + } + catch (Exception ex) + { + argumentBindingException = ExceptionDispatchInfo.Capture(ex); + } + } + + message = BindInvocationMessage(invocationId, target, argumentBindingException, arguments, hasArguments, binder); + } + break; + case HubProtocolConstants.StreamInvocationMessageType: + { + if (argumentsToken != null) + { + try + { + var paramTypes = binder.GetParameterTypes(target); + arguments = BindArguments(argumentsToken, paramTypes); + } + catch (Exception ex) + { + argumentBindingException = ExceptionDispatchInfo.Capture(ex); + } + } + + message = BindStreamInvocationMessage(invocationId, target, argumentBindingException, arguments, hasArguments, binder); + } + break; + case HubProtocolConstants.StreamItemMessageType: + if (itemToken != null) + { + var returnType = binder.GetReturnType(invocationId); + item = itemToken.ToObject(returnType, PayloadSerializer); + } + + message = BindStreamItemMessage(invocationId, item, hasItem, binder); + break; + case HubProtocolConstants.CompletionMessageType: + if (resultToken != null) + { + var returnType = binder.GetReturnType(invocationId); + result = resultToken.ToObject(returnType, PayloadSerializer); + } + + message = BindCompletionMessage(invocationId, error, result, hasResult, binder); + break; + case HubProtocolConstants.CancelInvocationMessageType: + message = BindCancelInvocationMessage(invocationId); + break; + case HubProtocolConstants.PingMessageType: + return PingMessage.Instance; + case null: + throw new InvalidDataException($"Missing required property '{TypePropertyName}'."); + default: + throw new InvalidDataException($"Unknown message type: {type}"); } + + return ApplyHeaders(message, headers); + } + catch (JsonReaderException jrex) + { + throw new InvalidDataException("Error reading JSON.", jrex); } } - private void ReadHeaders(JObject json, IDictionary headers) + private Dictionary ReadHeaders(JsonTextReader reader) { - var headersProp = json[HeadersPropertyName]; - if (headersProp != null) + var headers = new Dictionary(); + + if (reader.TokenType != JsonToken.StartObject) { - if (headersProp.Type != JTokenType.Object) + throw new InvalidDataException($"Expected '{HeadersPropertyName}' to be of type {JTokenType.Object}."); + } + + while (reader.Read()) + { + switch (reader.TokenType) { - throw new InvalidDataException($"Expected '{HeadersPropertyName}' to be of type {JTokenType.Object}."); - } - var headersObj = headersProp.Value(); - foreach (var prop in headersObj) - { - if (prop.Value.Type != JTokenType.String) - { - throw new InvalidDataException($"Expected header '{prop.Key}' to be of type {JTokenType.String}."); - } - headers[prop.Key] = prop.Value.Value(); + case JsonToken.PropertyName: + string propertyName = reader.Value.ToString(); + + JsonUtils.CheckRead(reader); + + if (reader.TokenType != JsonToken.String) + { + throw new InvalidDataException($"Expected header '{propertyName}' to be of type {JTokenType.String}."); + } + + headers[propertyName] = reader.Value?.ToString(); + break; + case JsonToken.Comment: + break; + case JsonToken.EndObject: + return headers; } } + + throw new JsonReaderException("Unexpected end when reading message headers"); } private void WriteMessageCore(HubMessage message, Stream stream) @@ -176,7 +368,7 @@ namespace Microsoft.AspNetCore.SignalR.Internal.Protocol private void WriteHeaders(JsonTextWriter writer, HubInvocationMessage message) { - if (message.Headers.Count > 0) + if (message.Headers != null && message.Headers.Count > 0) { writer.WritePropertyName(HeadersPropertyName); writer.WriteStartObject(); @@ -260,95 +452,122 @@ namespace Microsoft.AspNetCore.SignalR.Internal.Protocol writer.WriteValue(type); } - private InvocationMessage BindInvocationMessage(JObject json, IInvocationBinder binder) + private HubMessage BindCancelInvocationMessage(string invocationId) { - var invocationId = JsonUtils.GetOptionalProperty(json, InvocationIdPropertyName, JTokenType.String); - var target = JsonUtils.GetRequiredProperty(json, TargetPropertyName, JTokenType.String); - - var args = JsonUtils.GetRequiredProperty(json, ArgumentsPropertyName, JTokenType.Array); - - var paramTypes = binder.GetParameterTypes(target); - - InvocationMessage message; - try + if (string.IsNullOrEmpty(invocationId)) { - var arguments = BindArguments(args, paramTypes); - message = new InvocationMessage(invocationId, target, argumentBindingException: null, arguments: arguments); + throw new InvalidDataException($"Missing required property '{InvocationIdPropertyName}'."); } - catch (Exception ex) - { - message = new InvocationMessage(invocationId, target, ExceptionDispatchInfo.Capture(ex)); - } - ReadHeaders(json, message.Headers); - return message; + + return new CancelInvocationMessage(invocationId); } - private StreamInvocationMessage BindStreamInvocationMessage(JObject json, IInvocationBinder binder) + private HubMessage BindCompletionMessage(string invocationId, string error, object result, bool hasResult, IInvocationBinder binder) { - var invocationId = JsonUtils.GetRequiredProperty(json, InvocationIdPropertyName, JTokenType.String); - var target = JsonUtils.GetRequiredProperty(json, TargetPropertyName, JTokenType.String); - - var args = JsonUtils.GetRequiredProperty(json, ArgumentsPropertyName, JTokenType.Array); - - var paramTypes = binder.GetParameterTypes(target); - - StreamInvocationMessage message; - try + if (string.IsNullOrEmpty(invocationId)) { - var arguments = BindArguments(args, paramTypes); - message = new StreamInvocationMessage(invocationId, target, argumentBindingException: null, arguments: arguments); + throw new InvalidDataException($"Missing required property '{InvocationIdPropertyName}'."); } - catch (Exception ex) - { - message = new StreamInvocationMessage(invocationId, target, ExceptionDispatchInfo.Capture(ex)); - } - ReadHeaders(json, message.Headers); - return message; - } - private StreamItemMessage BindStreamItemMessage(JObject json, IInvocationBinder binder) - { - var invocationId = JsonUtils.GetRequiredProperty(json, InvocationIdPropertyName, JTokenType.String); - var result = JsonUtils.GetRequiredProperty(json, ItemPropertyName); - - var returnType = binder.GetReturnType(invocationId); - var message = new StreamItemMessage(invocationId, result?.ToObject(returnType, PayloadSerializer)); - ReadHeaders(json, message.Headers); - return message; - } - - private CompletionMessage BindCompletionMessage(JObject json, IInvocationBinder binder) - { - var invocationId = JsonUtils.GetRequiredProperty(json, InvocationIdPropertyName, JTokenType.String); - var error = JsonUtils.GetOptionalProperty(json, ErrorPropertyName, JTokenType.String); - var resultProp = json.Property(ResultPropertyName); - - if (error != null && resultProp != null) + if (error != null && hasResult) { throw new InvalidDataException("The 'error' and 'result' properties are mutually exclusive."); } - CompletionMessage message; - if (resultProp == null) + if (hasResult) { - message = new CompletionMessage(invocationId, error, result: null, hasResult: false); + return new CompletionMessage(invocationId, error, result, hasResult: true); } - else - { - var returnType = binder.GetReturnType(invocationId); - var payload = resultProp.Value?.ToObject(returnType, PayloadSerializer); - message = new CompletionMessage(invocationId, error, result: payload, hasResult: true); - } - ReadHeaders(json, message.Headers); - return message; + + return new CompletionMessage(invocationId, error, result: null, hasResult: false); } - private CancelInvocationMessage BindCancelInvocationMessage(JObject json) + private HubMessage BindStreamItemMessage(string invocationId, object item, bool hasItem, IInvocationBinder binder) { - var invocationId = JsonUtils.GetRequiredProperty(json, InvocationIdPropertyName, JTokenType.String); - var message = new CancelInvocationMessage(invocationId); - ReadHeaders(json, message.Headers); - return message; + if (string.IsNullOrEmpty(invocationId)) + { + throw new InvalidDataException($"Missing required property '{InvocationIdPropertyName}'."); + } + + if (!hasItem) + { + throw new InvalidDataException($"Missing required property '{ItemPropertyName}'."); + } + + return new StreamItemMessage(invocationId, item); + } + + private HubMessage BindStreamInvocationMessage(string invocationId, string target, ExceptionDispatchInfo argumentBindingException, object[] arguments, bool hasArguments, IInvocationBinder binder) + { + if (string.IsNullOrEmpty(invocationId)) + { + throw new InvalidDataException($"Missing required property '{InvocationIdPropertyName}'."); + } + + if (!hasArguments) + { + throw new InvalidDataException($"Missing required property '{ArgumentsPropertyName}'."); + } + + if (string.IsNullOrEmpty(target)) + { + throw new InvalidDataException($"Missing required property '{TargetPropertyName}'."); + } + + return new StreamInvocationMessage(invocationId, target, argumentBindingException: argumentBindingException, arguments: arguments); + } + + private HubMessage BindInvocationMessage(string invocationId, string target, ExceptionDispatchInfo argumentBindingException, object[] arguments, bool hasArguments, IInvocationBinder binder) + { + if (string.IsNullOrEmpty(target)) + { + throw new InvalidDataException($"Missing required property '{TargetPropertyName}'."); + } + + if (!hasArguments) + { + throw new InvalidDataException($"Missing required property '{ArgumentsPropertyName}'."); + } + + return new InvocationMessage(invocationId, target, argumentBindingException: argumentBindingException, arguments: arguments); + } + + private object[] BindArguments(JsonTextReader reader, IReadOnlyList paramTypes) + { + var arguments = new object[paramTypes.Count]; + var paramIndex = 0; + var argumentsCount = 0; + + while (reader.Read()) + { + if (reader.TokenType == JsonToken.EndArray) + { + if (argumentsCount != paramTypes.Count) + { + throw new InvalidDataException($"Invocation provides {argumentsCount} argument(s) but target expects {paramTypes.Count}."); + } + + return arguments; + } + + try + { + if (paramIndex < paramTypes.Count) + { + // Set all known arguments + arguments[paramIndex] = PayloadSerializer.Deserialize(reader, paramTypes[paramIndex]); + } + + argumentsCount++; + 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); + } + } + + throw new JsonReaderException("Unexpected end when reading JSON"); } private object[] BindArguments(JArray args, IReadOnlyList paramTypes) @@ -375,6 +594,16 @@ namespace Microsoft.AspNetCore.SignalR.Internal.Protocol } } + private HubMessage ApplyHeaders(HubMessage message, Dictionary headers) + { + if (headers != null && message is HubInvocationMessage invocationMessage) + { + invocationMessage.Headers = headers; + } + + return message; + } + internal static JsonSerializerSettings CreateDefaultSerializerSettings() { return new JsonSerializerSettings { ContractResolver = new CamelCasePropertyNamesContractResolver() }; diff --git a/src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/JsonUtils.cs b/src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/JsonUtils.cs index bbcd3d1461..7c7eb37fae 100644 --- a/src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/JsonUtils.cs +++ b/src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/JsonUtils.cs @@ -3,6 +3,7 @@ using System; using System.IO; +using Newtonsoft.Json; using Newtonsoft.Json.Linq; namespace Microsoft.AspNetCore.SignalR.Internal.Protocol @@ -41,5 +42,62 @@ namespace Microsoft.AspNetCore.SignalR.Internal.Protocol } return prop.Value(); } + + public static string GetTokenString(JsonToken tokenType) + { + switch (tokenType) + { + case JsonToken.None: + break; + case JsonToken.StartObject: + return JTokenType.Object.ToString(); + case JsonToken.StartArray: + return JTokenType.Array.ToString(); + case JsonToken.PropertyName: + return JTokenType.Property.ToString(); + default: + break; + } + return tokenType.ToString(); + } + + public static int? ReadAsInt32(JsonTextReader reader, string propertyName) + { + reader.Read(); + + if (reader.TokenType != JsonToken.Integer) + { + throw new InvalidDataException($"Expected '{propertyName}' to be of type {JTokenType.Integer}."); + } + + if (reader.Value == null) + { + return null; + } + + return Convert.ToInt32(reader.Value); + } + + public static string ReadAsString(JsonTextReader reader, string propertyName) + { + reader.Read(); + + if (reader.TokenType != JsonToken.String) + { + throw new InvalidDataException($"Expected '{propertyName}' to be of type {JTokenType.String}."); + } + + return reader.Value?.ToString(); + } + + public static bool CheckRead(JsonTextReader reader) + { + if (!reader.Read()) + { + throw new JsonReaderException("Unexpected end when reading JSON"); + } + + return true; + } } } diff --git a/src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/NegotiationProtocol.cs b/src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/NegotiationProtocol.cs index 948c06bd91..2a0453d600 100644 --- a/src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/NegotiationProtocol.cs +++ b/src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/NegotiationProtocol.cs @@ -3,7 +3,6 @@ using System; using System.Buffers; -using System.Collections; using System.IO; using System.Text; using Microsoft.AspNetCore.SignalR.Internal.Formatters; @@ -31,27 +30,27 @@ namespace Microsoft.AspNetCore.SignalR.Internal.Protocol TextMessageFormatter.WriteRecordSeparator(output); } - public static bool TryParseMessage(ReadOnlySpan input, out NegotiationMessage negotiationMessage) + public static bool TryParseMessage(ReadOnlyMemory input, out NegotiationMessage negotiationMessage) { if (!TextMessageParser.TryParseMessage(ref input, out var payload)) { throw new InvalidDataException("Unable to parse payload as a negotiation message."); } - using (var memoryStream = new MemoryStream(payload.ToArray())) + var textReader = new Utf8BufferTextReader(payload); + using (var reader = new JsonTextReader(textReader)) { - using (var reader = new JsonTextReader(new StreamReader(memoryStream))) - { - var token = JToken.ReadFrom(reader); - if (token == null || token.Type != JTokenType.Object) - { - throw new InvalidDataException($"Unexpected JSON Token Type '{token?.Type}'. Expected a JSON Object."); - } + reader.ArrayPool = JsonArrayPool.Shared; - var negotiationJObject = (JObject)token; - var protocol = JsonUtils.GetRequiredProperty(negotiationJObject, ProtocolPropertyName); - negotiationMessage = new NegotiationMessage(protocol); + var token = JToken.ReadFrom(reader); + if (token == null || token.Type != JTokenType.Object) + { + throw new InvalidDataException($"Unexpected JSON Token Type '{token?.Type}'. Expected a JSON Object."); } + + var negotiationJObject = (JObject)token; + var protocol = JsonUtils.GetRequiredProperty(negotiationJObject, ProtocolPropertyName); + negotiationMessage = new NegotiationMessage(protocol); } return true; } @@ -75,7 +74,7 @@ namespace Microsoft.AspNetCore.SignalR.Internal.Protocol var memory = buffer.IsSingleSegment ? buffer.First : buffer.ToArray(); - return TryParseMessage(memory.Span, out negotiationMessage); + return TryParseMessage(memory, out negotiationMessage); } } } diff --git a/src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/Utf8BufferTextReader.cs b/src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/Utf8BufferTextReader.cs new file mode 100644 index 0000000000..22509c02d4 --- /dev/null +++ b/src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/Utf8BufferTextReader.cs @@ -0,0 +1,54 @@ +using System; +using System.IO; +using System.Runtime.InteropServices; +using System.Text; + +namespace Microsoft.AspNetCore.SignalR.Internal.Protocol +{ + internal class Utf8BufferTextReader : TextReader + { + private ReadOnlyMemory _utf8Buffer; + + public Utf8BufferTextReader(ReadOnlyMemory utf8Buffer) + { + _utf8Buffer = utf8Buffer; + } + + public override int Read(char[] buffer, int index, int count) + { + if (_utf8Buffer.IsEmpty) + { + return 0; + } + + var source = _utf8Buffer.Span; + var destination = new Span(buffer, index, count); + var destinationBytesCount = Encoding.UTF8.GetByteCount(buffer, index, count); + + // We have then the destination + if (source.Length > destinationBytesCount) + { + source = source.Slice(0, destinationBytesCount); + + _utf8Buffer = _utf8Buffer.Slice(destinationBytesCount); + } + else + { + _utf8Buffer = ReadOnlyMemory.Empty; + } + +#if NETCOREAPP2_1 + return Encoding.UTF8.GetChars(source, destination); +#else + unsafe + { + fixed (char* destinationChars = &MemoryMarshal.GetReference(destination)) + fixed (byte* sourceBytes = &MemoryMarshal.GetReference(source)) + { + return Encoding.UTF8.GetChars(sourceBytes, source.Length, destinationChars, destination.Length); + } + } +#endif + } + } +} diff --git a/src/Microsoft.AspNetCore.SignalR.Protocols.MsgPack/Internal/Protocol/MessagePackHubProtocol.cs b/src/Microsoft.AspNetCore.SignalR.Protocols.MsgPack/Internal/Protocol/MessagePackHubProtocol.cs index f3aafe53ac..10862adf92 100644 --- a/src/Microsoft.AspNetCore.SignalR.Protocols.MsgPack/Internal/Protocol/MessagePackHubProtocol.cs +++ b/src/Microsoft.AspNetCore.SignalR.Protocols.MsgPack/Internal/Protocol/MessagePackHubProtocol.cs @@ -37,7 +37,7 @@ namespace Microsoft.AspNetCore.SignalR.Internal.Protocol SerializationContext = options.Value.SerializationContext; } - public bool TryParseMessages(ReadOnlySpan input, IInvocationBinder binder, IList messages) + public bool TryParseMessages(ReadOnlyMemory input, IInvocationBinder binder, IList messages) { while (BinaryMessageParser.TryParseMessage(ref input, out var payload)) { @@ -213,14 +213,11 @@ namespace Microsoft.AspNetCore.SignalR.Internal.Protocol } } - private static T ApplyHeaders(IDictionary source, T destination) where T: HubInvocationMessage + private static T ApplyHeaders(IDictionary source, T destination) where T : HubInvocationMessage { - if(source != null && source.Count > 0) + if (source != null && source.Count > 0) { - foreach(var header in source) - { - destination.Headers[header.Key] = header.Value; - } + destination.Headers = source; } return destination; diff --git a/test/Microsoft.AspNetCore.SignalR.Client.Tests/HubConnectionTests.cs b/test/Microsoft.AspNetCore.SignalR.Client.Tests/HubConnectionTests.cs index 6d3428d120..03e4fcf1db 100644 --- a/test/Microsoft.AspNetCore.SignalR.Client.Tests/HubConnectionTests.cs +++ b/test/Microsoft.AspNetCore.SignalR.Client.Tests/HubConnectionTests.cs @@ -255,7 +255,7 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests public TransferFormat TransferFormat => TransferFormat.Binary; - public bool TryParseMessages(ReadOnlySpan input, IInvocationBinder binder, IList messages) + public bool TryParseMessages(ReadOnlyMemory input, IInvocationBinder binder, IList messages) { ParseCalls += 1; if (_error != null) diff --git a/test/Microsoft.AspNetCore.SignalR.Common.Tests/Internal/Formatters/BinaryMessageFormatterTests.cs b/test/Microsoft.AspNetCore.SignalR.Common.Tests/Internal/Formatters/BinaryMessageFormatterTests.cs index f650173399..3186bfa045 100644 --- a/test/Microsoft.AspNetCore.SignalR.Common.Tests/Internal/Formatters/BinaryMessageFormatterTests.cs +++ b/test/Microsoft.AspNetCore.SignalR.Common.Tests/Internal/Formatters/BinaryMessageFormatterTests.cs @@ -113,7 +113,7 @@ namespace Microsoft.AspNetCore.Sockets.Tests.Internal.Formatters { BinaryMessageFormatter.WriteLengthPrefix(payload.Length, ms); ms.Write(payload, 0, payload.Length); - var buffer = new ReadOnlySpan(ms.ToArray()); + var buffer = new ReadOnlyMemory(ms.ToArray()); Assert.True(BinaryMessageParser.TryParseMessage(ref buffer, out var roundtripped)); Assert.Equal(payload, roundtripped.ToArray()); } diff --git a/test/Microsoft.AspNetCore.SignalR.Common.Tests/Internal/Formatters/BinaryMessageParserTests.cs b/test/Microsoft.AspNetCore.SignalR.Common.Tests/Internal/Formatters/BinaryMessageParserTests.cs index 49e426292c..385be0d6e0 100644 --- a/test/Microsoft.AspNetCore.SignalR.Common.Tests/Internal/Formatters/BinaryMessageParserTests.cs +++ b/test/Microsoft.AspNetCore.SignalR.Common.Tests/Internal/Formatters/BinaryMessageParserTests.cs @@ -17,7 +17,7 @@ namespace Microsoft.AspNetCore.SignalR.Common.Tests.Internal.Formatters [InlineData(new byte[] { 0x0B, 0x41, 0x0A, 0x52, 0x0D, 0x43, 0x0D, 0x0A, 0x3B, 0x44, 0x45, 0x46 }, "A\nR\rC\r\n;DEF")] public void ReadMessage(byte[] encoded, string payload) { - ReadOnlySpan span = encoded; + ReadOnlyMemory span = encoded; Assert.True(BinaryMessageParser.TryParseMessage(ref span, out var message)); Assert.Equal(0, span.Length); @@ -52,7 +52,7 @@ namespace Microsoft.AspNetCore.SignalR.Common.Tests.Internal.Formatters })] public void ReadBinaryMessage(byte[] encoded, byte[] payload) { - ReadOnlySpan< byte> span = encoded; + ReadOnlyMemory< byte> span = encoded; Assert.True(BinaryMessageParser.TryParseMessage(ref span, out var message)); Assert.Equal(0, span.Length); Assert.Equal(payload, message.ToArray()); @@ -66,7 +66,7 @@ namespace Microsoft.AspNetCore.SignalR.Common.Tests.Internal.Formatters { var ex = Assert.Throws(() => { - var buffer = new ReadOnlySpan(payload); + var buffer = new ReadOnlyMemory(payload); BinaryMessageParser.TryParseMessage(ref buffer, out var message); }); Assert.Equal("Messages over 2GB in size are not supported.", ex.Message); @@ -79,7 +79,7 @@ namespace Microsoft.AspNetCore.SignalR.Common.Tests.Internal.Formatters [InlineData(new byte[] { 0x80 })] // size is cut public void BinaryMessageParserReturnsFalseForPartialPayloads(byte[] payload) { - var buffer = new ReadOnlySpan(payload); + var buffer = new ReadOnlyMemory(payload); Assert.False(BinaryMessageParser.TryParseMessage(ref buffer, out var message)); } @@ -94,7 +94,7 @@ namespace Microsoft.AspNetCore.SignalR.Common.Tests.Internal.Formatters /* body: */ 0x48, 0x65, 0x6C, 0x6C, 0x6F, 0x2C, 0x0D, 0x0A, 0x57, 0x6F, 0x72, 0x6C, 0x64, 0x21, }; - ReadOnlySpan buffer = encoded; + ReadOnlyMemory buffer = encoded; var messages = new List(); while (BinaryMessageParser.TryParseMessage(ref buffer, out var message)) { @@ -113,7 +113,7 @@ namespace Microsoft.AspNetCore.SignalR.Common.Tests.Internal.Formatters [InlineData(new byte[] { 0x09, 0x00, 0x00 })] // Not enough data for payload public void ReadIncompleteMessages(byte[] encoded) { - ReadOnlySpan buffer = encoded; + ReadOnlyMemory buffer = encoded; Assert.False(BinaryMessageParser.TryParseMessage(ref buffer, out var message)); Assert.Equal(encoded.Length, buffer.Length); } diff --git a/test/Microsoft.AspNetCore.SignalR.Common.Tests/Internal/Formatters/TextMessageParserTests.cs b/test/Microsoft.AspNetCore.SignalR.Common.Tests/Internal/Formatters/TextMessageParserTests.cs index 2376895d23..9dbc7b2866 100644 --- a/test/Microsoft.AspNetCore.SignalR.Common.Tests/Internal/Formatters/TextMessageParserTests.cs +++ b/test/Microsoft.AspNetCore.SignalR.Common.Tests/Internal/Formatters/TextMessageParserTests.cs @@ -13,7 +13,7 @@ namespace Microsoft.AspNetCore.SignalR.Common.Tests.Internal.Formatters [Fact] public void ReadMessage() { - var message = new ReadOnlySpan(Encoding.UTF8.GetBytes("ABC\u001e")); + var message = new ReadOnlyMemory(Encoding.UTF8.GetBytes("ABC\u001e")); Assert.True(TextMessageParser.TryParseMessage(ref message, out var payload)); Assert.Equal("ABC", Encoding.UTF8.GetString(payload.ToArray())); @@ -23,14 +23,14 @@ namespace Microsoft.AspNetCore.SignalR.Common.Tests.Internal.Formatters [Fact] public void TryReadingIncompleteMessage() { - var message = new ReadOnlySpan(Encoding.UTF8.GetBytes("ABC")); + var message = new ReadOnlyMemory(Encoding.UTF8.GetBytes("ABC")); Assert.False(TextMessageParser.TryParseMessage(ref message, out var payload)); } [Fact] public void TryReadingMultipleMessages() { - var message = new ReadOnlySpan(Encoding.UTF8.GetBytes("ABC\u001eXYZ\u001e")); + var message = new ReadOnlyMemory(Encoding.UTF8.GetBytes("ABC\u001eXYZ\u001e")); Assert.True(TextMessageParser.TryParseMessage(ref message, out var payload)); Assert.Equal("ABC", Encoding.UTF8.GetString(payload.ToArray())); Assert.True(TextMessageParser.TryParseMessage(ref message, out payload)); @@ -40,7 +40,7 @@ namespace Microsoft.AspNetCore.SignalR.Common.Tests.Internal.Formatters [Fact] public void IncompleteTrailingMessage() { - var message = new ReadOnlySpan(Encoding.UTF8.GetBytes("ABC\u001eXYZ\u001e123")); + var message = new ReadOnlyMemory(Encoding.UTF8.GetBytes("ABC\u001eXYZ\u001e123")); Assert.True(TextMessageParser.TryParseMessage(ref message, out var payload)); Assert.Equal("ABC", Encoding.UTF8.GetString(payload.ToArray())); Assert.True(TextMessageParser.TryParseMessage(ref message, out payload)); diff --git a/test/Microsoft.AspNetCore.SignalR.Common.Tests/Internal/Protocol/HubMessageHelpers.cs b/test/Microsoft.AspNetCore.SignalR.Common.Tests/Internal/Protocol/HubMessageHelpers.cs index bfbdf874f9..c3ba421d4c 100644 --- a/test/Microsoft.AspNetCore.SignalR.Common.Tests/Internal/Protocol/HubMessageHelpers.cs +++ b/test/Microsoft.AspNetCore.SignalR.Common.Tests/Internal/Protocol/HubMessageHelpers.cs @@ -13,8 +13,14 @@ namespace Microsoft.AspNetCore.SignalR.Common.Tests.Internal.Protocol { foreach (var header in headers) { + if (hubMessage.Headers == null) + { + hubMessage.Headers = new Dictionary(); + } + hubMessage.Headers[header.Key] = header.Value; } + return hubMessage; } } diff --git a/test/Microsoft.AspNetCore.SignalR.Common.Tests/Internal/Protocol/JsonHubProtocolTests.cs b/test/Microsoft.AspNetCore.SignalR.Common.Tests/Internal/Protocol/JsonHubProtocolTests.cs index 220ccafc9f..200d4cd515 100644 --- a/test/Microsoft.AspNetCore.SignalR.Common.Tests/Internal/Protocol/JsonHubProtocolTests.cs +++ b/test/Microsoft.AspNetCore.SignalR.Common.Tests/Internal/Protocol/JsonHubProtocolTests.cs @@ -82,6 +82,14 @@ namespace Microsoft.AspNetCore.SignalR.Common.Tests.Internal.Protocol new object[] { PingMessage.Instance, true, NullValueHandling.Ignore, "{\"type\":6}" }, }; + public static IEnumerable OutOfOrderJsonTestData => new[] + { + new object[] { "{ \"arguments\": [1,2], \"type\":1, \"target\": \"Method\" }", new InvocationMessage("Method", argumentBindingException: null, 1, 2) }, + new object[] { "{ \"type\":4, \"arguments\": [1,2], \"target\": \"Method\", \"invocationId\": \"3\" }", new StreamInvocationMessage("3", "Method", argumentBindingException: null, 1, 2) }, + new object[] { "{ \"type\":3, \"result\": 10, \"invocationId\": \"15\" }", new CompletionMessage("15", null, 10, hasResult: true) }, + new object[] { "{ \"item\": \"foo\", \"invocationId\": \"1a\", \"type\":2 }", new StreamItemMessage("1a", "foo") } + }; + [Theory] [MemberData(nameof(ProtocolTestData))] public void WriteMessage(HubMessage message, bool camelCase, NullValueHandling nullValueHandling, string expectedOutput) @@ -169,6 +177,7 @@ namespace Microsoft.AspNetCore.SignalR.Common.Tests.Internal.Protocol [InlineData("{'type':'foo'}", "Expected 'type' to be of type Integer.")] [InlineData("{'type':3,'invocationId':'42','error':'foo','result':true}", "The 'error' and 'result' properties are mutually exclusive.")] + [InlineData("{'type':3,'invocationId':'42','result':true", "Error reading JSON.")] public void InvalidMessages(string input, string expectedMessage) { input = Frame(input); @@ -180,9 +189,25 @@ namespace Microsoft.AspNetCore.SignalR.Common.Tests.Internal.Protocol Assert.Equal(expectedMessage, ex.Message); } + [Theory] + [MemberData(nameof(OutOfOrderJsonTestData))] + public void ParseOutOfOrderJson(string input, HubMessage expectedMessage) + { + input = Frame(input); + + var binder = new TestBinder(expectedMessage); + var protocol = new JsonHubProtocol(); + var messages = new List(); + protocol.TryParseMessages(Encoding.UTF8.GetBytes(input), binder, messages); + + Assert.Equal(expectedMessage, messages[0], TestHubMessageEqualityComparer.Instance); + } + [Theory] [InlineData("{'type':1,'invocationId':'42','target':'foo','arguments':[]}", "Invocation provides 0 argument(s) but target expects 2.")] + [InlineData("{'type':1,'arguments':[], 'invocationId':'42','target':'foo'}", "Invocation provides 0 argument(s) but target expects 2.")] [InlineData("{'type':1,'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','arguments':[ 'abc', 'xyz'], 'target':'foo'}", "Error binding arguments. Make sure that the types of the provided values match the types of the hub method being invoked.")] [InlineData("{'type':4,'invocationId':'42','target':'foo','arguments':[]}", "Invocation provides 0 argument(s) but target expects 2.")] [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.")] public void ArgumentBindingErrors(string input, string expectedMessage) diff --git a/test/Microsoft.AspNetCore.SignalR.Common.Tests/Internal/Protocol/MessagePackHubProtocolTests.cs b/test/Microsoft.AspNetCore.SignalR.Common.Tests/Internal/Protocol/MessagePackHubProtocolTests.cs index 4d77154c8f..dee4d528df 100644 --- a/test/Microsoft.AspNetCore.SignalR.Common.Tests/Internal/Protocol/MessagePackHubProtocolTests.cs +++ b/test/Microsoft.AspNetCore.SignalR.Common.Tests/Internal/Protocol/MessagePackHubProtocolTests.cs @@ -303,7 +303,7 @@ namespace Microsoft.AspNetCore.SignalR.Common.Tests.Internal.Protocol AssertMessages(testData.Encoded, bytes); // Unframe the message to check the binary encoding - ReadOnlySpan byteSpan = bytes.AsSpan(); + ReadOnlyMemory byteSpan = bytes; Assert.True(BinaryMessageParser.TryParseMessage(ref byteSpan, out var unframed)); // Check the baseline binary encoding, use Assert.True in order to configure the error message @@ -413,7 +413,7 @@ namespace Microsoft.AspNetCore.SignalR.Common.Tests.Internal.Protocol AssertMessages(Array(HubProtocolConstants.CompletionMessageType, Map(), "0", 3, Array(42)), result); } - private static void AssertMessages(MessagePackObject expectedOutput, ReadOnlySpan bytes) + private static void AssertMessages(MessagePackObject expectedOutput, ReadOnlyMemory bytes) { Assert.True(BinaryMessageParser.TryParseMessage(ref bytes, out var message)); var obj = Unpack(message.ToArray());