From 48b3f1864283916f98d1261647ec9bd5bef1bca2 Mon Sep 17 00:00:00 2001 From: David Fowler Date: Thu, 13 Sep 2018 22:11:17 -0700 Subject: [PATCH] Improve the handshake request parsing errors (#2953) - Print out the raw handshake payload as utf8 text if it fails to parse as JSON or if we're missing properties. This should help flesh out potentially buggy clients. --- .../Protocol/HandshakeProtocol.cs | 14 +++++++++++--- .../Internal/Protocol/HandshakeProtocolTests.cs | 7 ++++--- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.AspNetCore.SignalR.Common/Protocol/HandshakeProtocol.cs b/src/Microsoft.AspNetCore.SignalR.Common/Protocol/HandshakeProtocol.cs index 89e72f3ece..3c5294f286 100644 --- a/src/Microsoft.AspNetCore.SignalR.Common/Protocol/HandshakeProtocol.cs +++ b/src/Microsoft.AspNetCore.SignalR.Common/Protocol/HandshakeProtocol.cs @@ -4,6 +4,7 @@ using System; using System.Buffers; using System.IO; +using System.Text; using Microsoft.AspNetCore.Internal; using Microsoft.AspNetCore.SignalR.Internal; using Newtonsoft.Json; @@ -214,17 +215,17 @@ namespace Microsoft.AspNetCore.SignalR.Protocol completed = true; break; default: - throw new InvalidDataException($"Unexpected token '{reader.TokenType}' when reading handshake request JSON."); + throw new InvalidDataException($"Unexpected token '{reader.TokenType}' when reading handshake request JSON. Message content: {GetPayloadAsString()}"); } } if (protocol == null) { - throw new InvalidDataException($"Missing required property '{ProtocolPropertyName}'."); + throw new InvalidDataException($"Missing required property '{ProtocolPropertyName}'. Message content: {GetPayloadAsString()}"); } if (protocolVersion == null) { - throw new InvalidDataException($"Missing required property '{ProtocolVersionPropertyName}'."); + throw new InvalidDataException($"Missing required property '{ProtocolVersionPropertyName}'. Message content: {GetPayloadAsString()}"); } requestMessage = new HandshakeRequestMessage(protocol, protocolVersion.Value); @@ -235,6 +236,13 @@ namespace Microsoft.AspNetCore.SignalR.Protocol Utf8BufferTextReader.Return(textReader); } + // For error messages, we want to print the payload as text + string GetPayloadAsString() + { + // REVIEW: Should we show hex for binary charaters? + return Encoding.UTF8.GetString(payload.ToArray()); + } + return true; } } diff --git a/test/Microsoft.AspNetCore.SignalR.Common.Tests/Internal/Protocol/HandshakeProtocolTests.cs b/test/Microsoft.AspNetCore.SignalR.Common.Tests/Internal/Protocol/HandshakeProtocolTests.cs index c19b88e6c3..4529664bea 100644 --- a/test/Microsoft.AspNetCore.SignalR.Common.Tests/Internal/Protocol/HandshakeProtocolTests.cs +++ b/test/Microsoft.AspNetCore.SignalR.Common.Tests/Internal/Protocol/HandshakeProtocolTests.cs @@ -50,10 +50,11 @@ namespace Microsoft.AspNetCore.SignalR.Common.Tests.Internal.Protocol [InlineData("42\u001e", "Unexpected JSON Token Type 'Integer'. Expected a JSON Object.")] [InlineData("\"42\"\u001e", "Unexpected JSON Token Type 'String'. Expected a JSON Object.")] [InlineData("null\u001e", "Unexpected JSON Token Type 'Null'. Expected a JSON Object.")] - [InlineData("{}\u001e", "Missing required property 'protocol'.")] + [InlineData("{}\u001e", "Missing required property 'protocol'. Message content: {}")] [InlineData("[]\u001e", "Unexpected JSON Token Type 'Array'. Expected a JSON Object.")] - [InlineData("{\"protocol\":\"json\"}\u001e", "Missing required property 'version'.")] - [InlineData("{\"version\":1}\u001e", "Missing required property 'protocol'.")] + [InlineData("{\"protocol\":\"json\"}\u001e", "Missing required property 'version'. Message content: {\"protocol\":\"json\"}")] + [InlineData("{\"version\":1}\u001e", "Missing required property 'protocol'. Message content: {\"version\":1}")] + [InlineData("{\"type\":4,\"invocationId\":\"42\",\"target\":\"foo\",\"arguments\":{}}\u001e", "Missing required property 'protocol'. Message content: {\"type\":4,\"invocationId\":\"42\",\"target\":\"foo\",\"arguments\":{}}")] [InlineData("{\"version\":\"123\"}\u001e", "Expected 'version' to be of type Integer.")] [InlineData("{\"protocol\":null,\"version\":123}\u001e", "Expected 'protocol' to be of type String.")] public void ParsingHandshakeRequestMessageThrowsForInvalidMessages(string payload, string expectedMessage)