Throw InvalidDataException instead of FormatException in NegotationProtocol (#1246)

* Throw InvalidDataException instead of FormatException in NegotationProtocol
 - Modify NegotiationProtocol to throw InvalidDataException
 - Update NegotiationProtocolTests expectations
 - Remove test case for InlineData "Missing required property 'protocol'"
 - Update JsonHubProtocol & JsonUtils to throw InvalidDataException
 - Update corresponding test expectations
 - Add back removed test

Addresses #1203
This commit is contained in:
Josh Williams 2018-01-11 15:23:30 -06:00 committed by Andrew Stanton-Nurse
parent e3679cb3c1
commit 0311f9b415
5 changed files with 14 additions and 13 deletions

View File

@ -80,7 +80,7 @@ namespace Microsoft.AspNetCore.SignalR.Internal.Protocol
if (token == null || token.Type != JTokenType.Object) if (token == null || token.Type != JTokenType.Object)
{ {
throw new FormatException($"Unexpected JSON Token Type '{token?.Type}'. Expected a JSON Object."); throw new InvalidDataException($"Unexpected JSON Token Type '{token?.Type}'. Expected a JSON Object.");
} }
var json = (JObject)token; var json = (JObject)token;
@ -102,12 +102,12 @@ namespace Microsoft.AspNetCore.SignalR.Internal.Protocol
case HubProtocolConstants.PingMessageType: case HubProtocolConstants.PingMessageType:
return PingMessage.Instance; return PingMessage.Instance;
default: default:
throw new FormatException($"Unknown message type: {type}"); throw new InvalidDataException($"Unknown message type: {type}");
} }
} }
catch (JsonReaderException jrex) catch (JsonReaderException jrex)
{ {
throw new FormatException("Error reading JSON.", jrex); throw new InvalidDataException("Error reading JSON.", jrex);
} }
} }
} }
@ -278,7 +278,7 @@ namespace Microsoft.AspNetCore.SignalR.Internal.Protocol
var arguments = new object[args.Count]; var arguments = new object[args.Count];
if (paramTypes.Length != arguments.Length) if (paramTypes.Length != arguments.Length)
{ {
throw new FormatException($"Invocation provides {arguments.Length} argument(s) but target expects {paramTypes.Length}."); throw new InvalidDataException($"Invocation provides {arguments.Length} argument(s) but target expects {paramTypes.Length}.");
} }
try try
@ -293,7 +293,7 @@ namespace Microsoft.AspNetCore.SignalR.Internal.Protocol
} }
catch (Exception ex) catch (Exception ex)
{ {
throw new FormatException("Error binding arguments. Make sure that the types of the provided values match the types of the hub method being invoked.", 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);
} }
} }
@ -314,7 +314,7 @@ namespace Microsoft.AspNetCore.SignalR.Internal.Protocol
if (error != null && resultProp != null) if (error != null && resultProp != null)
{ {
throw new FormatException("The 'error' and 'result' properties are mutually exclusive."); throw new InvalidDataException("The 'error' and 'result' properties are mutually exclusive.");
} }
if (resultProp == null) if (resultProp == null)

View File

@ -2,6 +2,7 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using System; using System;
using System.IO;
using Newtonsoft.Json.Linq; using Newtonsoft.Json.Linq;
namespace Microsoft.AspNetCore.SignalR.Internal.Protocol namespace Microsoft.AspNetCore.SignalR.Internal.Protocol
@ -26,7 +27,7 @@ namespace Microsoft.AspNetCore.SignalR.Internal.Protocol
if (prop == null) if (prop == null)
{ {
throw new FormatException($"Missing required property '{property}'."); throw new InvalidDataException($"Missing required property '{property}'.");
} }
return GetValue<T>(property, expectedType, prop); return GetValue<T>(property, expectedType, prop);
@ -36,7 +37,7 @@ namespace Microsoft.AspNetCore.SignalR.Internal.Protocol
{ {
if (expectedType != JTokenType.None && prop.Type != expectedType) if (expectedType != JTokenType.None && prop.Type != expectedType)
{ {
throw new FormatException($"Expected '{property}' to be of type {expectedType}."); throw new InvalidDataException($"Expected '{property}' to be of type {expectedType}.");
} }
return prop.Value<T>(); return prop.Value<T>();
} }

View File

@ -33,7 +33,7 @@ namespace Microsoft.AspNetCore.SignalR.Internal.Protocol
{ {
if (!TextMessageParser.TryParseMessage(ref input, out var payload)) if (!TextMessageParser.TryParseMessage(ref input, out var payload))
{ {
throw new FormatException("Unable to parse payload as a negotiation message."); throw new InvalidDataException("Unable to parse payload as a negotiation message.");
} }
using (var memoryStream = new MemoryStream(payload.ToArray())) using (var memoryStream = new MemoryStream(payload.ToArray()))
@ -43,7 +43,7 @@ namespace Microsoft.AspNetCore.SignalR.Internal.Protocol
var token = JToken.ReadFrom(reader); var token = JToken.ReadFrom(reader);
if (token == null || token.Type != JTokenType.Object) if (token == null || token.Type != JTokenType.Object)
{ {
throw new FormatException($"Unexpected JSON Token Type '{token?.Type}'. Expected a JSON Object."); throw new InvalidDataException($"Unexpected JSON Token Type '{token?.Type}'. Expected a JSON Object.");
} }
var negotiationJObject = (JObject)token; var negotiationJObject = (JObject)token;

View File

@ -149,7 +149,7 @@ namespace Microsoft.AspNetCore.SignalR.Common.Tests.Internal.Protocol
var binder = new TestBinder(); var binder = new TestBinder();
var protocol = new JsonHubProtocol(); var protocol = new JsonHubProtocol();
var ex = Assert.Throws<FormatException>(() => protocol.TryParseMessages(Encoding.UTF8.GetBytes(input), binder, out var messages)); var ex = Assert.Throws<InvalidDataException>(() => protocol.TryParseMessages(Encoding.UTF8.GetBytes(input), binder, out var messages));
Assert.Equal(expectedMessage, ex.Message); Assert.Equal(expectedMessage, ex.Message);
} }
@ -165,7 +165,7 @@ namespace Microsoft.AspNetCore.SignalR.Common.Tests.Internal.Protocol
var binder = new TestBinder(paramTypes: new[] { typeof(int), typeof(string) }, returnType: typeof(bool)); var binder = new TestBinder(paramTypes: new[] { typeof(int), typeof(string) }, returnType: typeof(bool));
var protocol = new JsonHubProtocol(); var protocol = new JsonHubProtocol();
protocol.TryParseMessages(Encoding.UTF8.GetBytes(input), binder, out var messages); protocol.TryParseMessages(Encoding.UTF8.GetBytes(input), binder, out var messages);
var ex = Assert.Throws<FormatException>(() => ((HubMethodInvocationMessage)messages[0]).Arguments); var ex = Assert.Throws<InvalidDataException>(() => ((HubMethodInvocationMessage)messages[0]).Arguments);
Assert.Equal(expectedMessage, ex.Message); Assert.Equal(expectedMessage, ex.Message);
} }

View File

@ -36,7 +36,7 @@ namespace Microsoft.AspNetCore.SignalR.Common.Tests.Internal.Protocol
{ {
var message = Encoding.UTF8.GetBytes(payload); var message = Encoding.UTF8.GetBytes(payload);
var exception = Assert.Throws<FormatException>(() => var exception = Assert.Throws<InvalidDataException>(() =>
Assert.True(NegotiationProtocol.TryParseMessage(message, out var deserializedMessage))); Assert.True(NegotiationProtocol.TryParseMessage(message, out var deserializedMessage)));
Assert.Equal(expectedMessage, exception.Message); Assert.Equal(expectedMessage, exception.Message);