From dc69dea706b9331b84c2e6e8dbcdab47ceb82da7 Mon Sep 17 00:00:00 2001 From: Ethan Celletti Date: Sun, 7 Jan 2018 20:44:58 -0800 Subject: [PATCH] Fixes (#44) Fixes #43 Fixes #35 And Misc fixes with detecting methods and base path detection --- .vscode/launch.json | 28 ++++++++ .vscode/tasks.json | 15 +++++ .../JsonConverters/RpcIdJsonConverter.cs | 27 +++----- .../Utilities/ExtensionsUtil.cs | 34 ++++++++++ .../Defaults/DefaultRpcInvoker.cs | 64 ++++++++++--------- src/EdjCase.JsonRpc.Router/RpcMethodInfo.cs | 33 +++++++++- src/EdjCase.JsonRpc.Router/RpcPath.cs | 43 ++++++++++++- src/EdjCase.JsonRpc.Router/RpcRouter.cs | 4 +- .../Utilities/ExtensionsUtil.cs | 12 ++++ .../Utilities/RpcUtil.cs | 59 +++++++++++++++++ .../Properties/launchSettings.json | 1 + .../RpcRoutes/RpcCommands.cs | 5 ++ .../RpcRoutes/RpcMath.cs | 8 ++- .../RpcRoutes/TestController.cs | 12 +++- test/EdjCase.JsonRpc.Router.Sample/Startup.cs | 9 ++- 15 files changed, 296 insertions(+), 58 deletions(-) create mode 100644 .vscode/launch.json create mode 100644 .vscode/tasks.json create mode 100644 src/EdjCase.JsonRpc.Core/Utilities/ExtensionsUtil.cs create mode 100644 src/EdjCase.JsonRpc.Router/Utilities/RpcUtil.cs diff --git a/.vscode/launch.json b/.vscode/launch.json new file mode 100644 index 0000000..afddc6d --- /dev/null +++ b/.vscode/launch.json @@ -0,0 +1,28 @@ +{ + // Use IntelliSense to find out which attributes exist for C# debugging + // Use hover for the description of the existing attributes + // For further information visit https://github.com/OmniSharp/omnisharp-vscode/blob/master/debugger-launchjson.md + "version": "0.2.0", + "configurations": [ + { + "name": "Router Tester", + "type": "coreclr", + "request": "launch", + "preLaunchTask": "build", + // If you have changed target frameworks, make sure to update the program path. + "program": "${workspaceFolder}/test/EdjCase.JsonRpc.Router.Sample/bin/Debug/netcoreapp2.0/EdjCase.JsonRpc.Router.Sample.dll", + "args": [], + "cwd": "${workspaceFolder}/test/EdjCase.JsonRpc.Router.Sample", + // For more information about the 'console' field, see https://github.com/OmniSharp/omnisharp-vscode/blob/master/debugger-launchjson.md#console-terminal-window + "console": "internalConsole", + "stopAtEntry": false, + "internalConsoleOptions": "openOnSessionStart" + }, + { + "name": ".NET Core Attach", + "type": "coreclr", + "request": "attach", + "processId": "${command:pickProcess}" + } + ] +} \ No newline at end of file diff --git a/.vscode/tasks.json b/.vscode/tasks.json new file mode 100644 index 0000000..df7719a --- /dev/null +++ b/.vscode/tasks.json @@ -0,0 +1,15 @@ +{ + "version": "2.0.0", + "tasks": [ + { + "label": "build", + "command": "dotnet", + "type": "process", + "args": [ + "build", + "${workspaceFolder}/test/EdjCase.JsonRpc.Client.Sample/EdjCase.JsonRpc.Client.Sample.csproj" + ], + "problemMatcher": "$msCompile" + } + ] +} \ No newline at end of file diff --git a/src/EdjCase.JsonRpc.Core/JsonConverters/RpcIdJsonConverter.cs b/src/EdjCase.JsonRpc.Core/JsonConverters/RpcIdJsonConverter.cs index 8378d1a..929eb29 100644 --- a/src/EdjCase.JsonRpc.Core/JsonConverters/RpcIdJsonConverter.cs +++ b/src/EdjCase.JsonRpc.Core/JsonConverters/RpcIdJsonConverter.cs @@ -1,5 +1,6 @@ using System; using Newtonsoft.Json; +using EdjCase.JsonRpc.Core.Utilities; namespace EdjCase.JsonRpc.Core.JsonConverters { @@ -16,8 +17,8 @@ public class RpcIdJsonConverter : JsonConverter /// Json serializer public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer) { - value = this.ValidateValue(value); - writer.WriteValue(value); + RpcId id = this.ValidateValue(value); + writer.WriteValue(id.Value); } /// @@ -45,11 +46,15 @@ private RpcId ValidateValue(object value) { return default; } + if(value is RpcId rpcId) + { + return rpcId; + } if(value is string stringValue) { return new RpcId(stringValue); } - if (this.IsNumericType(value.GetType())) + if (value.GetType().IsNumericType()) { return new RpcId(Convert.ToDouble(value)); } @@ -63,22 +68,8 @@ private RpcId ValidateValue(object value) /// True if the converter converts the specified type, otherwise False public override bool CanConvert(Type objectType) { - return objectType == typeof (string) || this.IsNumericType(objectType); + return objectType == typeof (string) || objectType.IsNumericType(); } - /// - /// Determines if the type is a number - /// - /// Type of the object - /// True if the type is a number, otherwise False - private bool IsNumericType(Type type) - { - return type == typeof (long) - || type == typeof (int) - || type == typeof (short) - || type == typeof (float) - || type == typeof (double) - || type == typeof (decimal); - } } } diff --git a/src/EdjCase.JsonRpc.Core/Utilities/ExtensionsUtil.cs b/src/EdjCase.JsonRpc.Core/Utilities/ExtensionsUtil.cs new file mode 100644 index 0000000..6c41772 --- /dev/null +++ b/src/EdjCase.JsonRpc.Core/Utilities/ExtensionsUtil.cs @@ -0,0 +1,34 @@ +using System; +using System.Collections.Generic; +using System.Text; + +namespace EdjCase.JsonRpc.Core.Utilities +{ + public static class TypeExtensions + { + /// + /// Determines if the type is a number + /// + /// Type of the object + /// Includes a check for whole number types. Defaults to true + /// True if the type is a number, otherwise False + public static bool IsNumericType(this Type type, bool includeInteger = true) + { + if (includeInteger) + { + return type == typeof(long) + || type == typeof(int) + || type == typeof(short) + || type == typeof(float) + || type == typeof(double) + || type == typeof(decimal); + } + else + { + return type == typeof(float) + || type == typeof(double) + || type == typeof(decimal); + } + } + } +} diff --git a/src/EdjCase.JsonRpc.Router/Defaults/DefaultRpcInvoker.cs b/src/EdjCase.JsonRpc.Router/Defaults/DefaultRpcInvoker.cs index 4d190e8..6a26595 100644 --- a/src/EdjCase.JsonRpc.Router/Defaults/DefaultRpcInvoker.cs +++ b/src/EdjCase.JsonRpc.Router/Defaults/DefaultRpcInvoker.cs @@ -59,7 +59,7 @@ private JsonSerializer GetJsonSerializer() /// Provides authorization policies for the authroziation service /// Optional logger for logging Rpc invocation /// Configuration data for the server - public DefaultRpcInvoker(IAuthorizationService authorizationService, IAuthorizationPolicyProvider policyProvider, + public DefaultRpcInvoker(IAuthorizationService authorizationService, IAuthorizationPolicyProvider policyProvider, ILogger logger, IOptions serverConfig) { this.authorizationService = authorizationService; @@ -131,7 +131,7 @@ public async Task InvokeRequestAsync(RpcRequest request, RpcPath pa { throw new RpcInvalidRequestException($"Request must be jsonrpc version '{JsonRpcContants.JsonRpcVersion}'"); } - + RpcMethodInfo rpcMethod = this.GetMatchingMethod(path, request, routeContext.RouteProvider, routeContext.RequestServices); bool isAuthorized = await this.IsAuthorizedAsync(rpcMethod.Method, routeContext); @@ -280,22 +280,34 @@ private RpcMethodInfo GetMatchingMethod(RpcPath path, RpcRequest request, IRpcRo var potentialMatches = new List(); foreach (MethodInfo method in methodsWithSameName) { - if (this.HasParameterSignature(method, request, out RpcMethodInfo rpcMethodInfo)) + (bool isMatch, RpcMethodInfo methodInfo) = this.HasParameterSignature(method, request); + if (isMatch) { - potentialMatches.Add(rpcMethodInfo); + potentialMatches.Add(methodInfo); } } if (potentialMatches.Count > 1) { - //Try to remove ambiguity with case sensitive check - potentialMatches = potentialMatches - .Where(m => string.Equals(m.Method.Name, request.Method, StringComparison.Ordinal)) + //Try to remove ambiguity with 'perfect matching' (usually optional params and types) + List exactMatches = potentialMatches + .Where(p => p.HasExactParameterMatch()) .ToList(); - if (potentialMatches.Count != 1) + if (exactMatches.Any()) + { + potentialMatches = exactMatches; + } + if (potentialMatches.Count > 1) { - this.logger?.LogError("More than one method matched the rpc request. Unable to invoke due to ambiguity."); - throw new RpcMethodNotFoundException(); + //Try to remove ambiguity with case sensitive check + potentialMatches = potentialMatches + .Where(m => string.Equals(m.Method.Name, request.Method, StringComparison.Ordinal)) + .ToList(); + if (potentialMatches.Count != 1) + { + this.logger?.LogError("More than one method matched the rpc request. Unable to invoke due to ambiguity."); + throw new RpcMethodNotFoundException(); + } } } @@ -389,7 +401,7 @@ private async Task InvokeAsync(RpcMethodInfo methodInfo, RpcPath path, I //Controller error handling RpcErrorFilterAttribute errorFilter = methodInfo.Method.DeclaringType.GetTypeInfo().GetCustomAttribute(); - if(errorFilter != null) + if (errorFilter != null) { OnExceptionResult result = errorFilter.OnException(routeInfo, ex.InnerException); if (!result.ThrowException) @@ -510,7 +522,7 @@ private object ConvertParameter(Type parameterType, object parameterValue) /// /// Array of parameters for the method /// True if the method signature matches the parameterList, otherwise False - private bool HasParameterSignature(MethodInfo method, RpcRequest rpcRequest, out RpcMethodInfo rpcMethodInfo) + private (bool Matches, RpcMethodInfo MethodInfo) HasParameterSignature(MethodInfo method, RpcRequest rpcRequest) { JToken[] orignialParameterList; if (rpcRequest.Parameters == null) @@ -527,8 +539,7 @@ private bool HasParameterSignature(MethodInfo method, RpcRequest rpcRequest, out bool canParse = this.TryParseParameterList(method, parameterMap, out orignialParameterList); if (!canParse) { - rpcMethodInfo = null; - return false; + return (false, null); } break; case JTokenType.Array: @@ -542,8 +553,7 @@ private bool HasParameterSignature(MethodInfo method, RpcRequest rpcRequest, out ParameterInfo[] parameterInfoList = method.GetParameters(); if (orignialParameterList.Length > parameterInfoList.Length) { - rpcMethodInfo = null; - return false; + return (false, null); } object[] correctedParameterList = new object[parameterInfoList.Length]; @@ -554,28 +564,26 @@ private bool HasParameterSignature(MethodInfo method, RpcRequest rpcRequest, out bool isMatch = this.ParameterMatches(parameterInfo, parameter, out object convertedParameter); if (!isMatch) { - rpcMethodInfo = null; - return false; + return (false, null); } correctedParameterList[i] = convertedParameter; } - - if(orignialParameterList.Length < parameterInfoList.Length) + + if (orignialParameterList.Length < parameterInfoList.Length) { //make a new array at the same length with padded 'missing' parameters (if optional) for (int i = orignialParameterList.Length; i < parameterInfoList.Length; i++) { if (!parameterInfoList[i].IsOptional) { - rpcMethodInfo = null; - return false; + return (false, null); } correctedParameterList[i] = Type.Missing; } } - rpcMethodInfo = new RpcMethodInfo(method, correctedParameterList, orignialParameterList); - return true; + var rpcMethodInfo = new RpcMethodInfo(method, correctedParameterList, orignialParameterList); + return (true, rpcMethodInfo); } /// @@ -665,18 +673,16 @@ private bool ParameterMatches(ParameterInfo parameterInfo, JToken value, out obj { case JTokenType.Array: { - //TODO should just assume they will work and have the end just fail if cant convert? JsonSerializer serializer = this.GetJsonSerializer(); JArray jArray = (JArray)value; - convertedValue = jArray.ToObject(parameterType, serializer); //Test conversion + convertedValue = jArray.ToObject(parameterType, serializer); return true; } case JTokenType.Object: { - //TODO should just assume they will work and have the end just fail if cant convert? JsonSerializer serializer = this.GetJsonSerializer(); JObject jObject = (JObject)value; - convertedValue = jObject.ToObject(parameterType, serializer); //Test conversion + convertedValue = jObject.ToObject(parameterType, serializer); return true; } default: @@ -691,7 +697,7 @@ private bool ParameterMatches(ParameterInfo parameterInfo, JToken value, out obj return false; } } - + /// /// Tries to parse the parameter map into an ordered parameter list diff --git a/src/EdjCase.JsonRpc.Router/RpcMethodInfo.cs b/src/EdjCase.JsonRpc.Router/RpcMethodInfo.cs index 8a81e11..5892a0b 100644 --- a/src/EdjCase.JsonRpc.Router/RpcMethodInfo.cs +++ b/src/EdjCase.JsonRpc.Router/RpcMethodInfo.cs @@ -1,5 +1,9 @@ -using System; +using EdjCase.JsonRpc.Core.Utilities; +using EdjCase.JsonRpc.Router.Utilities; +using Newtonsoft.Json.Linq; +using System; using System.Collections.Generic; +using System.Linq; using System.Reflection; using System.Text; using System.Threading.Tasks; @@ -10,12 +14,35 @@ public class RpcMethodInfo { public MethodInfo Method { get; } public object[] ConvertedParameters { get; } - public object[] RawParameters { get; } + public JToken[] RawParameters { get; } - public RpcMethodInfo(MethodInfo method, object[] convertedParameters, object[] rawParameters) + public RpcMethodInfo(MethodInfo method, object[] convertedParameters, JToken[] rawParameters) { this.Method = method; this.ConvertedParameters = convertedParameters; + this.RawParameters = rawParameters; + } + + public bool HasExactParameterMatch() + { + try + { + ParameterInfo[] parameters = this.Method.GetParameters(); + for (int i = 0; i < this.RawParameters.Length; i++) + { + JToken original = this.RawParameters[i]; + ParameterInfo parameter = parameters[i]; + if (!RpcUtil.TypesMatch(original, parameter.ParameterType)) + { + return false; + } + } + return true; + } + catch + { + return false; + } } } } diff --git a/src/EdjCase.JsonRpc.Router/RpcPath.cs b/src/EdjCase.JsonRpc.Router/RpcPath.cs index 6bb0e20..f144b47 100644 --- a/src/EdjCase.JsonRpc.Router/RpcPath.cs +++ b/src/EdjCase.JsonRpc.Router/RpcPath.cs @@ -1,4 +1,5 @@ -using System; +using EdjCase.JsonRpc.Core; +using System; using System.Linq; namespace EdjCase.JsonRpc.Router @@ -130,6 +131,46 @@ public static RpcPath Parse(string path, string basePath = null) return new RpcPath(path); } + /// + /// Removes the base path path from this path + /// + /// Base path to remove + /// A new path that is the full path without the base path + public RpcPath RemoveBasePath(RpcPath basePath) + { + if(!this.TryRemoveBasePath(basePath, out RpcPath path)) + { + throw new RpcParseException($"Count not remove path '{basePath}' from path '{this}'."); + } + return path; + } + + /// + /// Tries to remove the base path path from this path + /// + /// Base path to remove + /// True if removed the base path. Otherwise false + public bool TryRemoveBasePath(RpcPath basePath, out RpcPath path) + { + if (basePath == default) + { + path = this; + return true; + } + if (!this.StartsWith(basePath)) + { + path = default; + return false; + } + var newComponents = new string[this.components.Length - basePath.components.Length]; + if(newComponents.Length > 0) + { + Array.Copy(this.components, basePath.components.Length, newComponents, 0, 1); + } + path = new RpcPath(newComponents); + return true; + } + /// /// Merges the two paths to create a new Rpc path that is the combination of the two /// diff --git a/src/EdjCase.JsonRpc.Router/RpcRouter.cs b/src/EdjCase.JsonRpc.Router/RpcRouter.cs index 8b5edaf..c861f4d 100644 --- a/src/EdjCase.JsonRpc.Router/RpcRouter.cs +++ b/src/EdjCase.JsonRpc.Router/RpcRouter.cs @@ -76,7 +76,7 @@ public async Task RouteAsync(RouteContext context) { requestPath = RpcPath.Parse(context.HttpContext.Request.Path.Value); } - if (!requestPath.StartsWith(this.routeProvider.BaseRequestPath)) + if (!requestPath.TryRemoveBasePath(this.routeProvider.BaseRequestPath, out requestPath)) { this.logger?.LogTrace("Request did not match the base request path. Skipping rpc router."); return; @@ -113,6 +113,8 @@ public async Task RouteAsync(RouteContext context) return; } + context.HttpContext.Response.ContentType = "application/json"; + bool responseSet = false; string acceptEncoding = context.HttpContext.Request.Headers["Accept-Encoding"]; if (!string.IsNullOrWhiteSpace(acceptEncoding)) diff --git a/src/EdjCase.JsonRpc.Router/Utilities/ExtensionsUtil.cs b/src/EdjCase.JsonRpc.Router/Utilities/ExtensionsUtil.cs index 9c6ca14..19bafe0 100644 --- a/src/EdjCase.JsonRpc.Router/Utilities/ExtensionsUtil.cs +++ b/src/EdjCase.JsonRpc.Router/Utilities/ExtensionsUtil.cs @@ -5,6 +5,18 @@ namespace EdjCase.JsonRpc.Router.Utilities { + public static class TypeExtensions + { + /// + /// Determines if the type is nullable + /// + /// Type of the object + /// True if the type is nullable, otherwise False + public static bool IsNullableType(this Type type) + { + return !type.IsValueType || Nullable.GetUnderlyingType(type) != null; + } + } public static class RouteContextExtensions { diff --git a/src/EdjCase.JsonRpc.Router/Utilities/RpcUtil.cs b/src/EdjCase.JsonRpc.Router/Utilities/RpcUtil.cs new file mode 100644 index 0000000..54d1f4f --- /dev/null +++ b/src/EdjCase.JsonRpc.Router/Utilities/RpcUtil.cs @@ -0,0 +1,59 @@ +using EdjCase.JsonRpc.Core.Utilities; +using Newtonsoft.Json.Linq; +using System; +using System.Collections.Generic; +using System.Linq; + +namespace EdjCase.JsonRpc.Router.Utilities +{ + public class RpcUtil + { + public static bool TypesMatch(JToken token, Type type) + { + Type nullableType = Nullable.GetUnderlyingType(type); + if (nullableType != null) + { + type = nullableType; + } + switch (token.Type) + { + case JTokenType.Array: + JArray jArray = (JArray)token; + return type.IsArray + || type.GetInterfaces() + .Any(inter => inter.IsGenericType + && inter.GetGenericTypeDefinition() == typeof(IEnumerable<>) + && (!token.Any() + || RpcUtil.TypesMatch(jArray.First(), inter.GenericTypeArguments[0]))); + case JTokenType.Boolean: + return type == typeof(bool); + case JTokenType.Bytes: + return type == typeof(byte[]); + case JTokenType.Date: + return type == typeof(DateTime); + case JTokenType.Guid: + return type == typeof(Guid); + case JTokenType.Float: + return type.IsNumericType(includeInteger: false); + case JTokenType.Integer: + return type.IsNumericType(); + case JTokenType.Null: + case JTokenType.Undefined: + return nullableType != null || type.IsNullableType(); + case JTokenType.Object: + return type.IsAssignableFrom(typeof(object)); + case JTokenType.Uri: + return type == typeof(string) + || type == typeof(Uri); + case JTokenType.String: + return type == typeof(string); + case JTokenType.TimeSpan: + return type == typeof(TimeSpan) + || type == typeof(double) + || type == typeof(decimal); + default: + return false; + } + } + } +} diff --git a/test/EdjCase.JsonRpc.Router.Sample/Properties/launchSettings.json b/test/EdjCase.JsonRpc.Router.Sample/Properties/launchSettings.json index a48d278..e838bdd 100644 --- a/test/EdjCase.JsonRpc.Router.Sample/Properties/launchSettings.json +++ b/test/EdjCase.JsonRpc.Router.Sample/Properties/launchSettings.json @@ -19,6 +19,7 @@ "launchBrowser": false, "launchUrl": "http://localhost:62390", "environmentVariables": { + "ASPNETCORE_URLS": "http://localhost:62390", "ASPNETCORE_ENVIRONMENT": "Development" } } diff --git a/test/EdjCase.JsonRpc.Router.Sample/RpcRoutes/RpcCommands.cs b/test/EdjCase.JsonRpc.Router.Sample/RpcRoutes/RpcCommands.cs index d773b62..06c7ddd 100644 --- a/test/EdjCase.JsonRpc.Router.Sample/RpcRoutes/RpcCommands.cs +++ b/test/EdjCase.JsonRpc.Router.Sample/RpcRoutes/RpcCommands.cs @@ -23,5 +23,10 @@ public string Optional(string test = null) { return test; } + + public string Optional(int i, string test = null) + { + return i + test; + } } } diff --git a/test/EdjCase.JsonRpc.Router.Sample/RpcRoutes/RpcMath.cs b/test/EdjCase.JsonRpc.Router.Sample/RpcRoutes/RpcMath.cs index 4ff22cb..0160b3a 100644 --- a/test/EdjCase.JsonRpc.Router.Sample/RpcRoutes/RpcMath.cs +++ b/test/EdjCase.JsonRpc.Router.Sample/RpcRoutes/RpcMath.cs @@ -1,4 +1,5 @@ using Microsoft.AspNetCore.Authorization; +using System.Collections.Generic; using System.Threading.Tasks; namespace EdjCase.JsonRpc.Router.Sample.RpcRoutes @@ -16,7 +17,12 @@ public async Task AddAsync(int a, int b) } - public int Add(int[] a) + public int AddArray(int[] a) + { + return a[0] + a[1]; + } + + public int AddList(List a) { return a[0] + a[1]; } diff --git a/test/EdjCase.JsonRpc.Router.Sample/RpcRoutes/TestController.cs b/test/EdjCase.JsonRpc.Router.Sample/RpcRoutes/TestController.cs index ea0d0f3..c8f52af 100644 --- a/test/EdjCase.JsonRpc.Router.Sample/RpcRoutes/TestController.cs +++ b/test/EdjCase.JsonRpc.Router.Sample/RpcRoutes/TestController.cs @@ -7,9 +7,14 @@ namespace EdjCase.JsonRpc.Router.Sample.RpcRoutes { + public abstract class ControllerBase : RpcController + { + + } + [Authorize] [RpcRoute("Testz")] - public class TestController : RpcController + public class TestController : ControllerBase { public async Task Add(int a, int b) { @@ -34,8 +39,9 @@ public TestEnum Enum(TestEnum test) } } - public class TestEnum + public enum TestEnum { - public string Test { get; set; } + Test1, + Test2 } } diff --git a/test/EdjCase.JsonRpc.Router.Sample/Startup.cs b/test/EdjCase.JsonRpc.Router.Sample/Startup.cs index 02ab105..1ad7cf3 100644 --- a/test/EdjCase.JsonRpc.Router.Sample/Startup.cs +++ b/test/EdjCase.JsonRpc.Router.Sample/Startup.cs @@ -37,7 +37,7 @@ public void ConfigureServices(IServiceCollection services) { var claims = new List { - new Claim(ClaimTypes.NameIdentifier, Guid.NewGuid().ToString()) + new Claim(ClaimTypes.NameIdentifier, Guid.NewGuid().ToString()) }; var identity = new ClaimsIdentity(claims, "Basic"); var principal = new ClaimsPrincipal(identity); @@ -74,6 +74,12 @@ public void Configure(IApplicationBuilder app, IHostingEnvironment env, ILoggerF }); }); + app.UseJsonRpc(builder => + { + builder.BaseControllerType = typeof(ControllerBase); + builder.BaseRequestPath = "Auto"; + }); + } } @@ -83,7 +89,6 @@ public static void Main(string[] args) { var host = new WebHostBuilder() .UseKestrel() - .UseUrls("http://0.0.0.0:62390") .UseContentRoot(Directory.GetCurrentDirectory()) .UseIISIntegration() .UseStartup()