From 699d3be31b7fb2821532d4c3dd60e832584bc58c Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Fri, 21 Jan 2022 09:32:14 -0700 Subject: [PATCH] Add `JsonRpcIgnoreAttribute` to block RPC access to particular methods --- doc/recvrequest.md | 4 + .../Reflection/JsonRpcIgnoreAttribute.cs | 20 ++++ src/StreamJsonRpc/Reflection/RpcTargetInfo.cs | 51 +++++++-- src/StreamJsonRpc/Resources.Designer.cs | 9 ++ src/StreamJsonRpc/Resources.resx | 3 + .../netstandard2.0/PublicAPI.Unshipped.txt | 2 + .../netstandard2.1/PublicAPI.Unshipped.txt | 2 + test/StreamJsonRpc.Tests/JsonRpcTests.cs | 101 ++++++++++++++++++ 8 files changed, 186 insertions(+), 6 deletions(-) create mode 100644 src/StreamJsonRpc/Reflection/JsonRpcIgnoreAttribute.cs diff --git a/doc/recvrequest.md b/doc/recvrequest.md index 3a5a7d03..004f9078 100644 --- a/doc/recvrequest.md +++ b/doc/recvrequest.md @@ -80,6 +80,10 @@ rpc.AddLocalRpcTarget( rpc.StartListening(); ``` +### Blocking invocation of specific methods + +When a method on a target object would normally be exposed to the RPC client (either because it is `public` or because `JsonRpcTargetOptions.AllowNonPublicInvocation` has been set to `true`) and that method should *not* be exposed to RPC, apply the `[JsonRpcIgnore]` attribute to the method. + ### Server events When a server object defines public events, those events become notifications for the client. diff --git a/src/StreamJsonRpc/Reflection/JsonRpcIgnoreAttribute.cs b/src/StreamJsonRpc/Reflection/JsonRpcIgnoreAttribute.cs new file mode 100644 index 00000000..9c0b50d1 --- /dev/null +++ b/src/StreamJsonRpc/Reflection/JsonRpcIgnoreAttribute.cs @@ -0,0 +1,20 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +namespace StreamJsonRpc; + +/// +/// Attribute which identifies methods that should not be invokable over RPC. +/// +/// +/// When adding an RPC target object via or other APIs, +/// all public methods on the object default to being exposed to invocation by the client. +/// When is set, even more methods are exposed. +/// Applying this attribute to any method will ensure it can never be invoked directly by the RPC client. +/// If and are applied to the same method, +/// an exception will be thrown at the time of adding the object as an RPC target. +/// +[AttributeUsage(AttributeTargets.Method, AllowMultiple = false, Inherited = true)] +public class JsonRpcIgnoreAttribute : Attribute +{ +} diff --git a/src/StreamJsonRpc/Reflection/RpcTargetInfo.cs b/src/StreamJsonRpc/Reflection/RpcTargetInfo.cs index e9390922..7ea1ca24 100644 --- a/src/StreamJsonRpc/Reflection/RpcTargetInfo.cs +++ b/src/StreamJsonRpc/Reflection/RpcTargetInfo.cs @@ -5,13 +5,12 @@ namespace StreamJsonRpc.Reflection { using System; using System.Collections.Generic; - using System.Collections.Immutable; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; + using System.Globalization; using System.Linq; using System.Reflection; using System.Runtime.ExceptionServices; - using System.Text; using System.Threading; using System.Threading.Tasks; using Microsoft; @@ -389,6 +388,16 @@ private static IReadOnlyDictionary> GetRequestMeth continue; } + if (mapping.FindIgnoreAttribute(method) is object) + { + if (mapping.FindMethodAttribute(method) is object) + { + throw new ArgumentException(string.Format(CultureInfo.CurrentCulture, Resources.JsonRpcMethodAndIgnoreAttributesFound, method.Name)); + } + + continue; + } + var requestName = mapping.GetRpcMethodName(method); if (!requestMethodToDelegateMap.TryGetValue(requestName, out List? methodList)) @@ -423,7 +432,7 @@ private static IReadOnlyDictionary> GetRequestMeth requestMethodToClrMethodNameMap.Add(requestName, method.Name); } - JsonRpcMethodAttribute? attribute = mapping.FindAttribute(method); + JsonRpcMethodAttribute? attribute = mapping.FindMethodAttribute(method); if (attribute is null && (key.UseSingleObjectParameterDeserialization || key.ClientRequiresNamedArguments)) { @@ -494,6 +503,7 @@ internal class MethodNameMap { private readonly ReadOnlyMemory interfaceMaps; private readonly Dictionary methodAttributes = new Dictionary(); + private readonly Dictionary ignoreAttributes = new Dictionary(); internal MethodNameMap(TypeInfo typeInfo) { @@ -506,15 +516,15 @@ internal string GetRpcMethodName(MethodInfo method) { Requires.NotNull(method, nameof(method)); - return this.FindAttribute(method)?.Name ?? method.Name; + return this.FindMethodAttribute(method)?.Name ?? method.Name; } /// - /// Get the custom attribute, which may appear on the method itself or the interface definition of the method where applicable. + /// Get the , which may appear on the method itself or the interface definition of the method where applicable. /// /// The method to search for the attribute. /// The attribute, if found. - internal JsonRpcMethodAttribute? FindAttribute(MethodInfo method) + internal JsonRpcMethodAttribute? FindMethodAttribute(MethodInfo method) { Requires.NotNull(method, nameof(method)); @@ -538,6 +548,35 @@ internal string GetRpcMethodName(MethodInfo method) return attribute; } + /// + /// Get the , which may appear on the method itself or the interface definition of the method where applicable. + /// + /// The method to search for the attribute. + /// The attribute, if found. + internal JsonRpcIgnoreAttribute? FindIgnoreAttribute(MethodInfo method) + { + Requires.NotNull(method, nameof(method)); + + JsonRpcIgnoreAttribute? attribute; + lock (this.ignoreAttributes) + { + if (this.ignoreAttributes.TryGetValue(method, out attribute)) + { + return attribute; + } + } + + attribute = (JsonRpcIgnoreAttribute?)method.GetCustomAttribute(typeof(JsonRpcIgnoreAttribute)) + ?? (JsonRpcIgnoreAttribute?)this.FindMethodOnInterface(method)?.GetCustomAttribute(typeof(JsonRpcIgnoreAttribute)); + + lock (this.ignoreAttributes) + { + this.ignoreAttributes[method] = attribute; + } + + return attribute; + } + private MethodInfo? FindMethodOnInterface(MethodInfo methodImpl) { Requires.NotNull(methodImpl, nameof(methodImpl)); diff --git a/src/StreamJsonRpc/Resources.Designer.cs b/src/StreamJsonRpc/Resources.Designer.cs index 3668ea29..89d8998f 100644 --- a/src/StreamJsonRpc/Resources.Designer.cs +++ b/src/StreamJsonRpc/Resources.Designer.cs @@ -294,6 +294,15 @@ internal static string JsonRpcCannotBeNull { } } + /// + /// Looks up a localized string similar to Conflicting JsonRpcMethodAttribute and JsonRpcIgnoreAttribute found on the same method: {0}.. + /// + internal static string JsonRpcMethodAndIgnoreAttributesFound { + get { + return ResourceManager.GetString("JsonRpcMethodAndIgnoreAttributesFound", resourceCulture); + } + } + /// /// Looks up a localized string similar to This tracked object cannot be included in a notification. Only messages including an "id" property are supported.. /// diff --git a/src/StreamJsonRpc/Resources.resx b/src/StreamJsonRpc/Resources.resx index dbb334eb..b5732bfa 100644 --- a/src/StreamJsonRpc/Resources.resx +++ b/src/StreamJsonRpc/Resources.resx @@ -205,6 +205,9 @@ JSON RPC must not be null. + + Conflicting JsonRpcMethodAttribute and JsonRpcIgnoreAttribute found on the same method: {0}. + This tracked object cannot be included in a notification. Only messages including an "id" property are supported. diff --git a/src/StreamJsonRpc/netstandard2.0/PublicAPI.Unshipped.txt b/src/StreamJsonRpc/netstandard2.0/PublicAPI.Unshipped.txt index bb9b7f53..9763876b 100644 --- a/src/StreamJsonRpc/netstandard2.0/PublicAPI.Unshipped.txt +++ b/src/StreamJsonRpc/netstandard2.0/PublicAPI.Unshipped.txt @@ -7,6 +7,8 @@ StreamJsonRpc.ExceptionSettings.RecursionLimit.get -> int StreamJsonRpc.ExceptionSettings.RecursionLimit.init -> void StreamJsonRpc.JsonRpc.ExceptionOptions.get -> StreamJsonRpc.ExceptionSettings! StreamJsonRpc.JsonRpc.ExceptionOptions.set -> void +StreamJsonRpc.JsonRpcIgnoreAttribute +StreamJsonRpc.JsonRpcIgnoreAttribute.JsonRpcIgnoreAttribute() -> void StreamJsonRpc.JsonRpcMethodAttribute.ClientRequiresNamedArguments.get -> bool StreamJsonRpc.JsonRpcMethodAttribute.ClientRequiresNamedArguments.set -> void StreamJsonRpc.JsonRpcTargetOptions.ClientRequiresNamedArguments.get -> bool diff --git a/src/StreamJsonRpc/netstandard2.1/PublicAPI.Unshipped.txt b/src/StreamJsonRpc/netstandard2.1/PublicAPI.Unshipped.txt index bb9b7f53..9763876b 100644 --- a/src/StreamJsonRpc/netstandard2.1/PublicAPI.Unshipped.txt +++ b/src/StreamJsonRpc/netstandard2.1/PublicAPI.Unshipped.txt @@ -7,6 +7,8 @@ StreamJsonRpc.ExceptionSettings.RecursionLimit.get -> int StreamJsonRpc.ExceptionSettings.RecursionLimit.init -> void StreamJsonRpc.JsonRpc.ExceptionOptions.get -> StreamJsonRpc.ExceptionSettings! StreamJsonRpc.JsonRpc.ExceptionOptions.set -> void +StreamJsonRpc.JsonRpcIgnoreAttribute +StreamJsonRpc.JsonRpcIgnoreAttribute.JsonRpcIgnoreAttribute() -> void StreamJsonRpc.JsonRpcMethodAttribute.ClientRequiresNamedArguments.get -> bool StreamJsonRpc.JsonRpcMethodAttribute.ClientRequiresNamedArguments.set -> void StreamJsonRpc.JsonRpcTargetOptions.ClientRequiresNamedArguments.get -> bool diff --git a/test/StreamJsonRpc.Tests/JsonRpcTests.cs b/test/StreamJsonRpc.Tests/JsonRpcTests.cs index f291601b..2bdf9a98 100644 --- a/test/StreamJsonRpc.Tests/JsonRpcTests.cs +++ b/test/StreamJsonRpc.Tests/JsonRpcTests.cs @@ -62,6 +62,12 @@ public JsonRpcTests(ITestOutputHelper logger) this.clientRpc.StartListening(); } + public interface IServerWithIgnoredMethod + { + [JsonRpcIgnore] + void IgnoredMethod(); + } + protected interface IControlledFlushHandler : IJsonRpcMessageHandler { /// @@ -89,6 +95,9 @@ private interface IServer int InstanceMethodWithSingleObjectParameterButNoAttribute(XAndYFields fields); int Add_ExplicitInterfaceImplementation(int a, int b); + + [JsonRpcIgnore] + void InterfaceIgnoredMethod(); } private interface IServerDerived : IServer @@ -734,6 +743,68 @@ public async Task NonPublicMethods_InvokableOnlyUnderOption(bool allowNonPublicI } } + [Fact] + [Trait("JsonRpcIgnore", "")] + public async Task PublicIgnoredMethodCannotBeInvoked() + { + await Assert.ThrowsAsync(() => this.clientRpc.InvokeWithCancellationAsync(nameof(Server.PublicIgnoredMethod), cancellationToken: this.TimeoutToken)); + } + + [Fact] + [Trait("JsonRpcIgnore", "")] + public async Task PublicMethodIgnoredByInterfaceCannotBeInvoked() + { + await Assert.ThrowsAsync(() => this.clientRpc.InvokeWithCancellationAsync(nameof(Server.InterfaceIgnoredMethod), cancellationToken: this.TimeoutToken)); + } + + [Fact] + [Trait("JsonRpcIgnore", "")] + public async Task IgnoredMethodCanBeInvokedByClientWhenAddedExplicitly() + { + this.serverRpc.AllowModificationWhileListening = true; + this.serverRpc.AddLocalRpcMethod(typeof(Server).GetMethod(nameof(Server.PublicIgnoredMethod))!, this.server, null); + await this.clientRpc.InvokeWithCancellationAsync(nameof(Server.PublicIgnoredMethod), cancellationToken: this.TimeoutToken); + } + + [Fact] + [Trait("JsonRpcIgnore", "")] + public async Task InternalIgnoredMethodCannotBeInvoked() + { + var streams = Nerdbank.FullDuplexStream.CreateStreams(); + this.serverStream = streams.Item1; + this.clientStream = streams.Item2; + + this.serverRpc = new JsonRpc(this.serverStream); + this.clientRpc = new JsonRpc(this.clientStream); + + this.serverRpc.AddLocalRpcTarget(this.server, new JsonRpcTargetOptions { AllowNonPublicInvocation = true }); + + this.serverRpc.StartListening(); + this.clientRpc.StartListening(); + + await Assert.ThrowsAsync(() => this.clientRpc.InvokeWithCancellationAsync(nameof(Server.InternalIgnoredMethod), cancellationToken: this.TimeoutToken)); + } + + [Fact] + [Trait("JsonRpcIgnore", "")] + public void ConflictedIgnoreMethodThrows() + { + var streams = Nerdbank.FullDuplexStream.CreateStreams(); + this.serverStream = streams.Item1; + this.serverRpc = new JsonRpc(this.serverStream); + Assert.Throws(() => this.serverRpc.AddLocalRpcTarget(new ServerWithConflictingAttributes())); + } + + [Fact] + [Trait("JsonRpcIgnore", "")] + public void ConflictedIgnoreMethodViaInterfaceThrows() + { + var streams = Nerdbank.FullDuplexStream.CreateStreams(); + this.serverStream = streams.Item1; + this.serverRpc = new JsonRpc(this.serverStream); + Assert.Throws(() => this.serverRpc.AddLocalRpcTarget(new ServerWithConflictingAttributesViaInheritance())); + } + [Fact] public async Task CannotCallMethodWithOutParameter() { @@ -3437,6 +3508,15 @@ public void SendException(Exception? ex) public bool MethodOnDerived() => true; + [JsonRpcIgnore] + public void PublicIgnoredMethod() + { + } + + public void InterfaceIgnoredMethod() + { + } + int IServer.Add_ExplicitInterfaceImplementation(int a, int b) => a + b; internal void InternalMethod() @@ -3449,6 +3529,11 @@ internal void InternalMethodWithAttribute() { this.ServerMethodReached.Set(); } + + [JsonRpcIgnore] + internal void InternalIgnoredMethod() + { + } } #pragma warning restore CA1801 // use all parameters @@ -3530,6 +3615,22 @@ public int PlusTwo(int arg) } } + public class ServerWithConflictingAttributes + { + [JsonRpcMethod, JsonRpcIgnore] + public void BadMethod() + { + } + } + + public class ServerWithConflictingAttributesViaInheritance : IServerWithIgnoredMethod + { + [JsonRpcMethod] + public void IgnoredMethod() + { + } + } + [DataContract] public class Foo {