Skip to content

Commit 2133804

Browse files
AstnAstn
authored
Merge pull request #164 from Astn/aus-1003-reserved-names
AUS-1003: Refuse reserved method names in SMDServiceCollection
2 parents a015e3e + 4724600 commit 2133804

6 files changed

Lines changed: 280 additions & 6 deletions

File tree

‎AustinHarris.JsonRpcTestN/ReservedNameTests.cs‎

Lines changed: 235 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,235 @@
1+
using System;
2+
using System.Collections;
3+
using System.Collections.Generic;
4+
using System.Text;
5+
using AustinHarris.JsonRpc;
6+
using NUnit.Framework;
7+
8+
namespace AustinHarris.JsonRpcTestN
9+
{
10+
/// <summary>
11+
/// Reserved method names (<c>rpc.</c>-prefixed and <c>$/cancelRequest</c>) are refused by
12+
/// <see cref="SMDServiceCollection"/> itself, so every registration path refuses them.
13+
/// </summary>
14+
[TestFixture]
15+
public sealed class ReservedNameTests
16+
{
17+
private const string Session = "reserved-names";
18+
private static readonly string[] Reserved = { "rpc.x", "$/cancelRequest" };
19+
private static readonly string[] Allowed = { "$/progress", "rpcx", "Rpc.x", "x.rpc.y", "$/cancelrequest" };
20+
private static SMDServiceCollection Services => Handler.GetSessionHandler(Session).MetaData.Services;
21+
22+
[TearDown]
23+
public void Clean() => Handler.DestroySession(Session);
24+
25+
private static SMDService NewService(int result)
26+
{
27+
return new SMDService("POST", "JSON-RPC-2.0", new Dictionary<string, Type> { ["returns"] = typeof(int) }, new Dictionary<string, object>(), new Func<int>(() => result));
28+
}
29+
30+
private static SMDService Find(string name) => Services.Find(Encoding.UTF8.GetBytes(name));
31+
32+
private static void AssertReserved(string name, TestDelegate register)
33+
{
34+
var ex = Assert.Throws<ArgumentException>(register, name);
35+
StringAssert.StartsWith("'" + name + "' is a reserved JSON-RPC method name.", ex.Message);
36+
Assert.IsFalse(Services.ContainsKey(name), name);
37+
Assert.IsNull(Find(name), name);
38+
}
39+
40+
private static void AssertRegistered(string name)
41+
{
42+
Assert.IsTrue(Services.ContainsKey(name), name);
43+
Assert.IsNotNull(Find(name), name);
44+
}
45+
46+
private interface IPair
47+
{
48+
int First();
49+
int Second();
50+
}
51+
52+
private sealed class Pair : IPair
53+
{
54+
public int First() => 1;
55+
public int Second() => 2;
56+
}
57+
58+
private sealed class ReservedAlias
59+
{
60+
[JsonRpcMethod("rpc.x")]
61+
public int M() => 1;
62+
}
63+
64+
private sealed class ReservedCancelAlias
65+
{
66+
[JsonRpcMethod("$/cancelRequest")]
67+
public int M() => 1;
68+
}
69+
70+
private sealed class AllowedAliases
71+
{
72+
[JsonRpcMethod("$/progress")]
73+
[JsonRpcMethod("rpcx")]
74+
[JsonRpcMethod("Rpc.x")]
75+
[JsonRpcMethod("x.rpc.y")]
76+
[JsonRpcMethod("$/cancelrequest")]
77+
public int M() => 1;
78+
}
79+
80+
private sealed class NullKeyEntries : IReadOnlyDictionary<string, SMDService>
81+
{
82+
public int Count => 1;
83+
public IEnumerable<string> Keys => new string[] { null };
84+
public IEnumerable<SMDService> Values => new SMDService[] { null };
85+
public SMDService this[string key] => throw new NotImplementedException();
86+
public bool ContainsKey(string key) => false;
87+
public bool TryGetValue(string key, out SMDService value) { value = null; return false; }
88+
public IEnumerator<KeyValuePair<string, SMDService>> GetEnumerator()
89+
{
90+
yield return new KeyValuePair<string, SMDService>(null, null);
91+
}
92+
IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
93+
}
94+
95+
[Test]
96+
public void BindMethod_RefusesReserved_AcceptsTheRest()
97+
{
98+
foreach (var name in Reserved)
99+
AssertReserved(name, () => ServiceBinder.BindMethod(Session, name, () => 1));
100+
foreach (var name in Allowed)
101+
{
102+
ServiceBinder.BindMethod(Session, name, () => 7);
103+
Assert.AreEqual("{\"jsonrpc\":\"2.0\",\"result\":7,\"id\":1}",
104+
JsonRpcProcessor.ProcessSync(Session, "{\"method\":\"" + name + "\",\"id\":1}", null), name);
105+
}
106+
}
107+
108+
[Test]
109+
public void BindInterface_RefusesReserved_AcceptsTheRest()
110+
{
111+
foreach (var name in Reserved)
112+
{
113+
AssertReserved(name, () => ServiceBinder.BindInterface<IPair>(Session, new Pair(),
114+
new RpcInterfaceBindingOptions { NameRule = m => m.Leaf == "First" ? name : "second" }));
115+
Assert.AreEqual(0, Services.Count);
116+
}
117+
foreach (var name in Allowed)
118+
{
119+
ServiceBinder.BindInterface<IPair>(Session, new Pair(),
120+
new RpcInterfaceBindingOptions { NameRule = m => m.Leaf == "First" ? name : "second." + name });
121+
AssertRegistered(name);
122+
AssertRegistered("second." + name);
123+
}
124+
}
125+
126+
[Test]
127+
public void AttributeBinder_RefusesReservedAliases_AcceptsTheRest()
128+
{
129+
AssertReserved("rpc.x", () => ServiceBinder.BindService(Session, new ReservedAlias()));
130+
AssertReserved("$/cancelRequest", () => ServiceBinder.BindService(Session, new ReservedCancelAlias()));
131+
Assert.AreEqual(0, Services.Count);
132+
ServiceBinder.BindService(Session, new AllowedAliases());
133+
foreach (var name in Allowed) AssertRegistered(name);
134+
}
135+
136+
[Test]
137+
public void RegisterFuction_RefusesReserved_AcceptsTheRest()
138+
{
139+
var handler = Handler.GetSessionHandler(Session);
140+
#pragma warning disable CS0618
141+
foreach (var name in Reserved)
142+
AssertReserved(name, () => handler.RegisterFuction(name, new Dictionary<string, Type> { ["returns"] = typeof(int) }, null, new Func<int>(() => 1)));
143+
foreach (var name in Allowed)
144+
{
145+
handler.RegisterFuction(name, new Dictionary<string, Type> { ["returns"] = typeof(int) }, null, new Func<int>(() => 1));
146+
AssertRegistered(name);
147+
}
148+
#pragma warning restore CS0618
149+
}
150+
151+
[Test]
152+
public void DirectCollectionAdds_RefuseReserved_AcceptTheRest()
153+
{
154+
var services = Services;
155+
foreach (var name in Reserved)
156+
{
157+
AssertReserved(name, () => services.Add(name, NewService(1)));
158+
AssertReserved(name, () => services.Add(new KeyValuePair<string, SMDService>(name, NewService(1))));
159+
AssertReserved(name, () => services[name] = NewService(1));
160+
Assert.AreEqual("key", Assert.Throws<ArgumentException>(() => services.Add(name, NewService(1))).ParamName);
161+
Assert.AreEqual("key", Assert.Throws<ArgumentException>(() => services[name] = NewService(1)).ParamName);
162+
}
163+
Assert.AreEqual(0, services.Count);
164+
165+
foreach (var name in Allowed)
166+
{
167+
services.Add(name, NewService(1));
168+
AssertRegistered(name);
169+
Assert.IsTrue(services.Remove(name));
170+
services.Add(new KeyValuePair<string, SMDService>(name, NewService(2)));
171+
AssertRegistered(name);
172+
var replacement = NewService(3);
173+
services[name] = replacement;
174+
Assert.AreSame(replacement, Find(name));
175+
}
176+
177+
Assert.Throws<ArgumentNullException>(() => services.Add(null, NewService(1)));
178+
Assert.Throws<ArgumentNullException>(() => services[null] = NewService(1));
179+
}
180+
181+
[Test]
182+
public void Names_AreComparedAsGiven()
183+
{
184+
// no trimming and no case folding: these are ordinary names
185+
foreach (var name in new[] { " rpc.x", "RPC.x", "$/CancelRequest", "$/cancelRequest ", "rpc" })
186+
{
187+
Services.Add(name, NewService(1));
188+
AssertRegistered(name);
189+
}
190+
AssertReserved("rpc.", () => Services.Add("rpc.", NewService(1)));
191+
}
192+
193+
[Test]
194+
public void AddBatch_WithOneReservedEntry_LeavesTheCollectionUnchanged()
195+
{
196+
ServiceBinder.BindMethod(Session, "before", () => 1);
197+
var before = Find("before");
198+
int count = Services.Count;
199+
200+
var ex = Assert.Throws<ArgumentException>(() => ServiceBinder.BindInterface<IPair>(Session, new Pair(),
201+
new RpcInterfaceBindingOptions { NameRule = m => m.Leaf == "First" ? "ok" : "$/cancelRequest" }));
202+
Assert.AreEqual("entries", ex.ParamName);
203+
204+
Assert.AreEqual(count, Services.Count);
205+
Assert.AreSame(before, Find("before"));
206+
Assert.IsNull(Find("ok"));
207+
Assert.IsNull(Find("$/cancelRequest"));
208+
Assert.IsFalse(Services.ContainsKey("ok"));
209+
CollectionAssert.AreEqual(new[] { "before" }, Services.Keys);
210+
}
211+
212+
[Test]
213+
public void AddBatch_NullName_RetainsArgumentNullException()
214+
{
215+
var ex = Assert.Throws<ArgumentNullException>(() => Services.AddBatch(new NullKeyEntries()));
216+
Assert.AreEqual(0, Services.Count);
217+
}
218+
219+
[Test]
220+
public void AddReserved_AcceptsReservedOnce_AndKeepsTheDuplicateRule()
221+
{
222+
var first = NewService(1);
223+
Services.AddReserved("rpc.discover", first);
224+
Assert.AreSame(first, Find("rpc.discover"));
225+
Assert.AreSame(first, Services["rpc.discover"]);
226+
227+
Assert.Throws<ArgumentException>(() => Services.AddReserved("rpc.discover", NewService(2)));
228+
Assert.AreSame(first, Find("rpc.discover"), "the first registration stands");
229+
Assert.AreEqual(1, Services.Count);
230+
231+
Assert.Throws<ArgumentNullException>(() => Services.AddReserved(null, NewService(1)));
232+
Assert.Throws<ArgumentNullException>(() => Services.AddReserved("rpc.other", null));
233+
}
234+
}
235+
}

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ behaviour: a breaking change to either means a new major version.
3838
- The AspNetCore host binds every registered service, `JsonRpcService` subclasses included, to its effective session (the registration's session, then `JsonRpcOptions.SessionId`, then the default). It no longer skips a subclass on the default session.
3939
- The core package's description says "no JSON library dependency" instead of "no dependencies". The session registry uses the framework's `ConcurrentDictionary`; the `NonBlocking` package reference is gone, so the core has no dependencies on `net8.0` and `net10.0` (measured with `SessionRegistryBenchmarks`: unknown-id lookups and register/destroy cycles got faster, stable lookups and dispatch are unchanged).
4040
- `SMD.Services` is an `SMDServiceCollection`; every mutation through it updates the dispatch table at once. `SMD.Types` is a process-wide registry.
41+
- Registration refuses reserved method names (`rpc.`-prefixed and `$/cancelRequest`) on every path, including `BindMethod`, attribute binding and direct additions to `SMDServiceCollection`; `BindInterface` refused `rpc.` alone before.
4142
- The `jsonrpc` member is checked (`Config.VersionPolicy`, default `Lenient`): a missing member is accepted, `"jsonrpc":"1.0"` or a non-string value is `-32600`.
4243
- A parameter value the serializer cannot convert is `-32602` with structured data naming the parameter (it was `-32603`); `-32601` names the requested method in its data.
4344
- Named parameters are checked against the method's parameter list: an unknown or repeated name is `-32602`.

‎Json-Rpc/SMDService.cs‎

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,19 +109,36 @@ private static string TypeHash(Dictionary<string, object> jo)
109109
/// The services of one session keyed by JSON method name. A dictionary for callers; underneath, every
110110
/// mutation also replaces the lock-free UTF-8 dispatch table the request path resolves methods from, so
111111
/// an added, removed or replaced service is visible to the next request. Reads of the dictionary take a
112-
/// lock; the request path never does.
112+
/// lock; the request path never does. Names beginning with <c>rpc.</c> and the name <c>$/cancelRequest</c>
113+
/// are reserved: every public way of adding a service refuses them with an <see cref="ArgumentException"/>.
113114
/// </summary>
114115
public sealed class SMDServiceCollection : IDictionary<string, SMDService>, IReadOnlyDictionary<string, SMDService>
115116
{
116117
private Dictionary<string, SMDService> _services = new Dictionary<string, SMDService>();
117118
private readonly Utf8KeyTable<SMDService> _table = new Utf8KeyTable<SMDService>();
118119
private readonly object _sync = new object();
119120

121+
private const string ReservedPrefix = "rpc.";
122+
private const string CancelRequest = "$/cancelRequest";
123+
124+
/// <summary>
125+
/// Refuses the names the specification reserves (<c>rpc.</c>-prefixed, ordinal and case-sensitive) and
126+
/// <c>$/cancelRequest</c>. The name is compared as given: no trimming and no case folding.
127+
/// </summary>
128+
private static void ThrowIfReserved(string name, string paramName)
129+
{
130+
if (name == null) throw new ArgumentNullException(paramName);
131+
if (name.StartsWith(ReservedPrefix, StringComparison.Ordinal) || string.Equals(name, CancelRequest, StringComparison.Ordinal))
132+
throw new ArgumentException("'" + name + "' is a reserved JSON-RPC method name.", paramName);
133+
}
134+
120135
internal void AddBatch(IReadOnlyDictionary<string, SMDService> entries)
121136
{
122137
if (entries.Count == 0) return;
123138
lock (_sync)
124139
{
140+
// Every name is checked before anything is copied or added, so a batch with one reserved name adds nothing.
141+
foreach (var entry in entries) ThrowIfReserved(entry.Key, nameof(entries));
125142
var next = new Dictionary<string, SMDService>(_services);
126143
foreach (var entry in entries)
127144
{
@@ -173,6 +190,7 @@ public SMDService this[string key]
173190
{
174191
if (key == null) throw new ArgumentNullException(nameof(key));
175192
if (value == null) throw new ArgumentNullException(nameof(value));
193+
ThrowIfReserved(key, nameof(key));
176194
lock (_sync)
177195
{
178196
_services[key] = value;
@@ -222,11 +240,30 @@ public ICollection<SMDService> Values
222240
IEnumerable<SMDService> IReadOnlyDictionary<string, SMDService>.Values => Values;
223241

224242
public void Add(string key, SMDService value)
243+
{
244+
if (key == null) throw new ArgumentNullException(nameof(key));
245+
if (value == null) throw new ArgumentNullException(nameof(value));
246+
ThrowIfReserved(key, nameof(key));
247+
lock (_sync)
248+
{
249+
_services.Add(key, value);
250+
_table.Set(key, value);
251+
}
252+
}
253+
254+
/// <summary>
255+
/// Adds a service under a reserved name, for the library's own <c>rpc.discover</c> registration.
256+
/// It bypasses only the reserved-name check: an existing <paramref name="key"/> still throws
257+
/// <see cref="ArgumentException"/>. Internal, so the reserved check is the only public path; unused in 2.0.0.
258+
/// </summary>
259+
internal void AddReserved(string key, SMDService value)
225260
{
226261
if (key == null) throw new ArgumentNullException(nameof(key));
227262
if (value == null) throw new ArgumentNullException(nameof(value));
228263
lock (_sync)
229264
{
265+
if (_services.ContainsKey(key))
266+
throw new ArgumentException("JSON-RPC method '" + key + "' is already registered.", nameof(key));
230267
_services.Add(key, value);
231268
_table.Set(key, value);
232269
}

‎Json-Rpc/ServiceBinder.Interface.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@ public static RpcBinding BindInterface<TInterface>(TInterface implementation, Rp
2121
/// Only public instance methods declared by the selected interfaces are exported; names, attributes,
2222
/// and optional defaults come from those declarations, including explicit implementations.
2323
/// Recursive getters run once per mount at registration and may have side effects. A failure publishes
24-
/// nothing; getter side effects cannot be undone. Empty, reserved <c>rpc.</c>, duplicate, and occupied
25-
/// names are rejected. Generic methods and default interface bodies are unsupported.
24+
/// nothing; getter side effects cannot be undone. Empty, reserved (<c>rpc.</c>-prefixed or <c>$/cancelRequest</c>),
25+
/// duplicate, and occupied names are rejected. Generic methods and default interface bodies are unsupported.
2626
/// The returned handle owns the registrations, not the lifetime of the implementation objects.
2727
/// </summary>
2828
public static RpcBinding BindInterface<TInterface>(string sessionId, TInterface implementation,
@@ -118,8 +118,8 @@ private void AddMethod(MethodInfo method, object target, string[] path, string a
118118
var description = new RpcInterfaceMethod(method, path, leaf, defaultName);
119119
if (_include != null && !_include(description)) return;
120120
string name = _nameRule == null ? defaultName : _nameRule(description);
121-
if (string.IsNullOrWhiteSpace(name) || name.StartsWith("rpc.", StringComparison.Ordinal))
122-
throw new ArgumentException("Invalid or reserved JSON-RPC interface method name: '" + name + "'.");
121+
if (string.IsNullOrWhiteSpace(name))
122+
throw new ArgumentException("Invalid JSON-RPC interface method name: '" + name + "'.");
123123
if (Entries.ContainsKey(name)) throw new ArgumentException("Duplicate JSON-RPC interface method name '" + name + "'.");
124124
if (method.ContainsGenericParameters)
125125
throw new ArgumentException("Generic interface method '" + method.Name + "' is not supported.");

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ That is the whole in-process server. The rest of this page is about exposing met
177177

178178
## Defining methods
179179

180-
A *method* is a callable identified by the `method` member of a request; its implementation is a delegate, a `[JsonRpcMethod]` member of a class, or a member of a bound interface. `ServiceBinder` never asks for a `MethodInfo`; the same word names the -32601 "Method not found" error.
180+
A *method* is a callable identified by the `method` member of a request; its implementation is a delegate, a `[JsonRpcMethod]` member of a class, or a member of a bound interface. `ServiceBinder` never asks for a `MethodInfo`; the same word names the -32601 "Method not found" error. Names beginning with `rpc.` and the name `$/cancelRequest` are reserved and refused at registration.
181181

182182
### Classes
183183

0 commit comments

Comments
 (0)