Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add JsonRpcIgnoreAttribute to block RPC access to particular methods #758

Merged
merged 1 commit into from
Jan 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions doc/recvrequest.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
20 changes: 20 additions & 0 deletions src/StreamJsonRpc/Reflection/JsonRpcIgnoreAttribute.cs
Original file line number Diff line number Diff line change
@@ -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
{
}
51 changes: 45 additions & 6 deletions src/StreamJsonRpc/Reflection/RpcTargetInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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))
{
Expand Down Expand Up @@ -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)
{
Expand All @@ -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));

Expand All @@ -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));
Expand Down
9 changes: 9 additions & 0 deletions src/StreamJsonRpc/Resources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/StreamJsonRpc/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
2 changes: 2 additions & 0 deletions src/StreamJsonRpc/netstandard2.0/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions src/StreamJsonRpc/netstandard2.1/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
101 changes: 101 additions & 0 deletions test/StreamJsonRpc.Tests/JsonRpcTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,12 @@ public JsonRpcTests(ITestOutputHelper logger)
this.clientRpc.StartListening();
}

public interface IServerWithIgnoredMethod
{
[JsonRpcIgnore]
void IgnoredMethod();
}

protected interface IControlledFlushHandler : IJsonRpcMessageHandler
{
/// <summary>
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
{
Expand Down Expand Up @@ -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()
Expand All @@ -3449,6 +3529,11 @@ internal void InternalMethodWithAttribute()
{
this.ServerMethodReached.Set();
}

[JsonRpcIgnore]
internal void InternalIgnoredMethod()
{
}
}
#pragma warning restore CA1801 // use all parameters

Expand Down Expand Up @@ -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
{
Expand Down