Skip to content

Commit 4724600

Browse files
AstnAstn
committed
AUS-1003: Refuse reserved method names in SMDServiceCollection
Names beginning with rpc. and the name $/cancelRequest are refused by one check in SMDServiceCollection (Add, the indexer setter and AddBatch before any entry is copied), which every registration path reaches: BindMethod, the attribute binder and RegisterFuction through AddService, BindInterface through AddBatch. BindInterface drops its own rpc. test. An internal AddReserved keeps the duplicate rule for the library's later rpc.discover registration. README, CHANGELOG and docs/upgrading.md carry the change.
1 parent a015e3e commit 4724600

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)