Merge pull request #758 from AArnott/JsonRpcIgnore

Add `JsonRpcIgnoreAttribute` to block RPC access to particular methods

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.

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;
+
+/// <summary>
+/// Attribute which identifies methods that should <em>not</em> be invokable over RPC.
+/// </summary>
+/// <remarks>
+/// <para>When adding an RPC target object via <see cref="JsonRpc.AddLocalRpcTarget(object)"/> or other APIs,
+/// all public methods on the object default to being exposed to invocation by the client.
+/// When <see cref="JsonRpcTargetOptions.AllowNonPublicInvocation"/> 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.</para>
+/// <para>If <see cref="JsonRpcMethodAttribute"/> and <see cref="JsonRpcIgnoreAttribute"/> are applied to the same method,
+/// an exception will be thrown at the time of adding the object as an RPC target.</para>
+/// </remarks>
+[AttributeUsage(AttributeTargets.Method, AllowMultiple = false, Inherited = true)]
+public class JsonRpcIgnoreAttribute : Attribute
+{
+}

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<string, List<MethodSignature>> 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<MethodSignature>? methodList))
@@ -423,7 +432,7 @@ private static IReadOnlyDictionary<string, List<MethodSignature>> 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<InterfaceMapping> interfaceMaps;
             private readonly Dictionary<MethodInfo, JsonRpcMethodAttribute?> methodAttributes = new Dictionary<MethodInfo, JsonRpcMethodAttribute?>();
+            private readonly Dictionary<MethodInfo, JsonRpcIgnoreAttribute?> ignoreAttributes = new Dictionary<MethodInfo, JsonRpcIgnoreAttribute?>();
 
             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;
             }
 
             /// <summary>
-            /// Get the custom attribute, which may appear on the method itself or the interface definition of the method where applicable.
+            /// Get the <see cref="JsonRpcMethodAttribute"/>, which may appear on the method itself or the interface definition of the method where applicable.
             /// </summary>
             /// <param name="method">The method to search for the attribute.</param>
             /// <returns>The attribute, if found.</returns>
-            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;
             }
 
+            /// <summary>
+            /// Get the <see cref="JsonRpcIgnoreAttribute"/>, which may appear on the method itself or the interface definition of the method where applicable.
+            /// </summary>
+            /// <param name="method">The method to search for the attribute.</param>
+            /// <returns>The attribute, if found.</returns>
+            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));

src/StreamJsonRpc/Resources.resx

@@ -205,6 +205,9 @@
   <data name="JsonRpcCannotBeNull" xml:space="preserve">
     <value>JSON RPC must not be null.</value>
   </data>
+  <data name="JsonRpcMethodAndIgnoreAttributesFound" xml:space="preserve">
+    <value>Conflicting JsonRpcMethodAttribute and JsonRpcIgnoreAttribute found on the same method: {0}.</value>
+  </data>
   <data name="MarshaledObjectInNotificationError" xml:space="preserve">
     <value>This tracked object cannot be included in a notification. Only messages including an "id" property are supported.</value>
   </data>

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

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

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
     {
         /// <summary>
@@ -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<RemoteMethodNotFoundException>(() => this.clientRpc.InvokeWithCancellationAsync(nameof(Server.PublicIgnoredMethod), cancellationToken: this.TimeoutToken));
+    }
+
+    [Fact]
+    [Trait("JsonRpcIgnore", "")]
+    public async Task PublicMethodIgnoredByInterfaceCannotBeInvoked()
+    {
+        await Assert.ThrowsAsync<RemoteMethodNotFoundException>(() => 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<RemoteMethodNotFoundException>(() => 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<ArgumentException>(() => 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<ArgumentException>(() => 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
     {