From 4724600755de03b27bb0159e770e3d558ea94b5d Mon Sep 17 00:00:00 2001 From: Austin Harris Date: Fri, 25 Sep 2026 17:56:58 -0600 Subject: [PATCH] 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. --- .../ReservedNameTests.cs | 235 ++++++++++++++++++ CHANGELOG.md | 1 + Json-Rpc/SMDService.cs | 39 ++- Json-Rpc/ServiceBinder.Interface.cs | 8 +- README.md | 2 +- docs/upgrading.md | 1 + 6 files changed, 280 insertions(+), 6 deletions(-) create mode 100644 AustinHarris.JsonRpcTestN/ReservedNameTests.cs diff --git a/AustinHarris.JsonRpcTestN/ReservedNameTests.cs b/AustinHarris.JsonRpcTestN/ReservedNameTests.cs new file mode 100644 index 0000000..1267194 --- /dev/null +++ b/AustinHarris.JsonRpcTestN/ReservedNameTests.cs @@ -0,0 +1,235 @@ +using System; +using System.Collections; +using System.Collections.Generic; +using System.Text; +using AustinHarris.JsonRpc; +using NUnit.Framework; + +namespace AustinHarris.JsonRpcTestN +{ + /// + /// Reserved method names (rpc.-prefixed and $/cancelRequest) are refused by + /// itself, so every registration path refuses them. + /// + [TestFixture] + public sealed class ReservedNameTests + { + private const string Session = "reserved-names"; + private static readonly string[] Reserved = { "rpc.x", "$/cancelRequest" }; + private static readonly string[] Allowed = { "$/progress", "rpcx", "Rpc.x", "x.rpc.y", "$/cancelrequest" }; + private static SMDServiceCollection Services => Handler.GetSessionHandler(Session).MetaData.Services; + + [TearDown] + public void Clean() => Handler.DestroySession(Session); + + private static SMDService NewService(int result) + { + return new SMDService("POST", "JSON-RPC-2.0", new Dictionary { ["returns"] = typeof(int) }, new Dictionary(), new Func(() => result)); + } + + private static SMDService Find(string name) => Services.Find(Encoding.UTF8.GetBytes(name)); + + private static void AssertReserved(string name, TestDelegate register) + { + var ex = Assert.Throws(register, name); + StringAssert.StartsWith("'" + name + "' is a reserved JSON-RPC method name.", ex.Message); + Assert.IsFalse(Services.ContainsKey(name), name); + Assert.IsNull(Find(name), name); + } + + private static void AssertRegistered(string name) + { + Assert.IsTrue(Services.ContainsKey(name), name); + Assert.IsNotNull(Find(name), name); + } + + private interface IPair + { + int First(); + int Second(); + } + + private sealed class Pair : IPair + { + public int First() => 1; + public int Second() => 2; + } + + private sealed class ReservedAlias + { + [JsonRpcMethod("rpc.x")] + public int M() => 1; + } + + private sealed class ReservedCancelAlias + { + [JsonRpcMethod("$/cancelRequest")] + public int M() => 1; + } + + private sealed class AllowedAliases + { + [JsonRpcMethod("$/progress")] + [JsonRpcMethod("rpcx")] + [JsonRpcMethod("Rpc.x")] + [JsonRpcMethod("x.rpc.y")] + [JsonRpcMethod("$/cancelrequest")] + public int M() => 1; + } + + private sealed class NullKeyEntries : IReadOnlyDictionary + { + public int Count => 1; + public IEnumerable Keys => new string[] { null }; + public IEnumerable Values => new SMDService[] { null }; + public SMDService this[string key] => throw new NotImplementedException(); + public bool ContainsKey(string key) => false; + public bool TryGetValue(string key, out SMDService value) { value = null; return false; } + public IEnumerator> GetEnumerator() + { + yield return new KeyValuePair(null, null); + } + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + } + + [Test] + public void BindMethod_RefusesReserved_AcceptsTheRest() + { + foreach (var name in Reserved) + AssertReserved(name, () => ServiceBinder.BindMethod(Session, name, () => 1)); + foreach (var name in Allowed) + { + ServiceBinder.BindMethod(Session, name, () => 7); + Assert.AreEqual("{\"jsonrpc\":\"2.0\",\"result\":7,\"id\":1}", + JsonRpcProcessor.ProcessSync(Session, "{\"method\":\"" + name + "\",\"id\":1}", null), name); + } + } + + [Test] + public void BindInterface_RefusesReserved_AcceptsTheRest() + { + foreach (var name in Reserved) + { + AssertReserved(name, () => ServiceBinder.BindInterface(Session, new Pair(), + new RpcInterfaceBindingOptions { NameRule = m => m.Leaf == "First" ? name : "second" })); + Assert.AreEqual(0, Services.Count); + } + foreach (var name in Allowed) + { + ServiceBinder.BindInterface(Session, new Pair(), + new RpcInterfaceBindingOptions { NameRule = m => m.Leaf == "First" ? name : "second." + name }); + AssertRegistered(name); + AssertRegistered("second." + name); + } + } + + [Test] + public void AttributeBinder_RefusesReservedAliases_AcceptsTheRest() + { + AssertReserved("rpc.x", () => ServiceBinder.BindService(Session, new ReservedAlias())); + AssertReserved("$/cancelRequest", () => ServiceBinder.BindService(Session, new ReservedCancelAlias())); + Assert.AreEqual(0, Services.Count); + ServiceBinder.BindService(Session, new AllowedAliases()); + foreach (var name in Allowed) AssertRegistered(name); + } + + [Test] + public void RegisterFuction_RefusesReserved_AcceptsTheRest() + { + var handler = Handler.GetSessionHandler(Session); +#pragma warning disable CS0618 + foreach (var name in Reserved) + AssertReserved(name, () => handler.RegisterFuction(name, new Dictionary { ["returns"] = typeof(int) }, null, new Func(() => 1))); + foreach (var name in Allowed) + { + handler.RegisterFuction(name, new Dictionary { ["returns"] = typeof(int) }, null, new Func(() => 1)); + AssertRegistered(name); + } +#pragma warning restore CS0618 + } + + [Test] + public void DirectCollectionAdds_RefuseReserved_AcceptTheRest() + { + var services = Services; + foreach (var name in Reserved) + { + AssertReserved(name, () => services.Add(name, NewService(1))); + AssertReserved(name, () => services.Add(new KeyValuePair(name, NewService(1)))); + AssertReserved(name, () => services[name] = NewService(1)); + Assert.AreEqual("key", Assert.Throws(() => services.Add(name, NewService(1))).ParamName); + Assert.AreEqual("key", Assert.Throws(() => services[name] = NewService(1)).ParamName); + } + Assert.AreEqual(0, services.Count); + + foreach (var name in Allowed) + { + services.Add(name, NewService(1)); + AssertRegistered(name); + Assert.IsTrue(services.Remove(name)); + services.Add(new KeyValuePair(name, NewService(2))); + AssertRegistered(name); + var replacement = NewService(3); + services[name] = replacement; + Assert.AreSame(replacement, Find(name)); + } + + Assert.Throws(() => services.Add(null, NewService(1))); + Assert.Throws(() => services[null] = NewService(1)); + } + + [Test] + public void Names_AreComparedAsGiven() + { + // no trimming and no case folding: these are ordinary names + foreach (var name in new[] { " rpc.x", "RPC.x", "$/CancelRequest", "$/cancelRequest ", "rpc" }) + { + Services.Add(name, NewService(1)); + AssertRegistered(name); + } + AssertReserved("rpc.", () => Services.Add("rpc.", NewService(1))); + } + + [Test] + public void AddBatch_WithOneReservedEntry_LeavesTheCollectionUnchanged() + { + ServiceBinder.BindMethod(Session, "before", () => 1); + var before = Find("before"); + int count = Services.Count; + + var ex = Assert.Throws(() => ServiceBinder.BindInterface(Session, new Pair(), + new RpcInterfaceBindingOptions { NameRule = m => m.Leaf == "First" ? "ok" : "$/cancelRequest" })); + Assert.AreEqual("entries", ex.ParamName); + + Assert.AreEqual(count, Services.Count); + Assert.AreSame(before, Find("before")); + Assert.IsNull(Find("ok")); + Assert.IsNull(Find("$/cancelRequest")); + Assert.IsFalse(Services.ContainsKey("ok")); + CollectionAssert.AreEqual(new[] { "before" }, Services.Keys); + } + + [Test] + public void AddBatch_NullName_RetainsArgumentNullException() + { + var ex = Assert.Throws(() => Services.AddBatch(new NullKeyEntries())); + Assert.AreEqual(0, Services.Count); + } + + [Test] + public void AddReserved_AcceptsReservedOnce_AndKeepsTheDuplicateRule() + { + var first = NewService(1); + Services.AddReserved("rpc.discover", first); + Assert.AreSame(first, Find("rpc.discover")); + Assert.AreSame(first, Services["rpc.discover"]); + + Assert.Throws(() => Services.AddReserved("rpc.discover", NewService(2))); + Assert.AreSame(first, Find("rpc.discover"), "the first registration stands"); + Assert.AreEqual(1, Services.Count); + + Assert.Throws(() => Services.AddReserved(null, NewService(1))); + Assert.Throws(() => Services.AddReserved("rpc.other", null)); + } + } +} diff --git a/CHANGELOG.md b/CHANGELOG.md index 09b34b3..4d3c266 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,7 @@ behaviour: a breaking change to either means a new major version. - 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. - 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). - `SMD.Services` is an `SMDServiceCollection`; every mutation through it updates the dispatch table at once. `SMD.Types` is a process-wide registry. +- 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. - The `jsonrpc` member is checked (`Config.VersionPolicy`, default `Lenient`): a missing member is accepted, `"jsonrpc":"1.0"` or a non-string value is `-32600`. - 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. - Named parameters are checked against the method's parameter list: an unknown or repeated name is `-32602`. diff --git a/Json-Rpc/SMDService.cs b/Json-Rpc/SMDService.cs index 5f07a27..cfe024d 100644 --- a/Json-Rpc/SMDService.cs +++ b/Json-Rpc/SMDService.cs @@ -109,7 +109,8 @@ private static string TypeHash(Dictionary jo) /// The services of one session keyed by JSON method name. A dictionary for callers; underneath, every /// mutation also replaces the lock-free UTF-8 dispatch table the request path resolves methods from, so /// an added, removed or replaced service is visible to the next request. Reads of the dictionary take a - /// lock; the request path never does. + /// lock; the request path never does. Names beginning with rpc. and the name $/cancelRequest + /// are reserved: every public way of adding a service refuses them with an . /// public sealed class SMDServiceCollection : IDictionary, IReadOnlyDictionary { @@ -117,11 +118,27 @@ public sealed class SMDServiceCollection : IDictionary, IRea private readonly Utf8KeyTable _table = new Utf8KeyTable(); private readonly object _sync = new object(); + private const string ReservedPrefix = "rpc."; + private const string CancelRequest = "$/cancelRequest"; + + /// + /// Refuses the names the specification reserves (rpc.-prefixed, ordinal and case-sensitive) and + /// $/cancelRequest. The name is compared as given: no trimming and no case folding. + /// + private static void ThrowIfReserved(string name, string paramName) + { + if (name == null) throw new ArgumentNullException(paramName); + if (name.StartsWith(ReservedPrefix, StringComparison.Ordinal) || string.Equals(name, CancelRequest, StringComparison.Ordinal)) + throw new ArgumentException("'" + name + "' is a reserved JSON-RPC method name.", paramName); + } + internal void AddBatch(IReadOnlyDictionary entries) { if (entries.Count == 0) return; lock (_sync) { + // Every name is checked before anything is copied or added, so a batch with one reserved name adds nothing. + foreach (var entry in entries) ThrowIfReserved(entry.Key, nameof(entries)); var next = new Dictionary(_services); foreach (var entry in entries) { @@ -173,6 +190,7 @@ public SMDService this[string key] { if (key == null) throw new ArgumentNullException(nameof(key)); if (value == null) throw new ArgumentNullException(nameof(value)); + ThrowIfReserved(key, nameof(key)); lock (_sync) { _services[key] = value; @@ -222,11 +240,30 @@ public ICollection Values IEnumerable IReadOnlyDictionary.Values => Values; public void Add(string key, SMDService value) + { + if (key == null) throw new ArgumentNullException(nameof(key)); + if (value == null) throw new ArgumentNullException(nameof(value)); + ThrowIfReserved(key, nameof(key)); + lock (_sync) + { + _services.Add(key, value); + _table.Set(key, value); + } + } + + /// + /// Adds a service under a reserved name, for the library's own rpc.discover registration. + /// It bypasses only the reserved-name check: an existing still throws + /// . Internal, so the reserved check is the only public path; unused in 2.0.0. + /// + internal void AddReserved(string key, SMDService value) { if (key == null) throw new ArgumentNullException(nameof(key)); if (value == null) throw new ArgumentNullException(nameof(value)); lock (_sync) { + if (_services.ContainsKey(key)) + throw new ArgumentException("JSON-RPC method '" + key + "' is already registered.", nameof(key)); _services.Add(key, value); _table.Set(key, value); } diff --git a/Json-Rpc/ServiceBinder.Interface.cs b/Json-Rpc/ServiceBinder.Interface.cs index 90c7ec3..7bf8209 100644 --- a/Json-Rpc/ServiceBinder.Interface.cs +++ b/Json-Rpc/ServiceBinder.Interface.cs @@ -21,8 +21,8 @@ public static RpcBinding BindInterface(TInterface implementation, Rp /// Only public instance methods declared by the selected interfaces are exported; names, attributes, /// and optional defaults come from those declarations, including explicit implementations. /// Recursive getters run once per mount at registration and may have side effects. A failure publishes - /// nothing; getter side effects cannot be undone. Empty, reserved rpc., duplicate, and occupied - /// names are rejected. Generic methods and default interface bodies are unsupported. + /// nothing; getter side effects cannot be undone. Empty, reserved (rpc.-prefixed or $/cancelRequest), + /// duplicate, and occupied names are rejected. Generic methods and default interface bodies are unsupported. /// The returned handle owns the registrations, not the lifetime of the implementation objects. /// public static RpcBinding BindInterface(string sessionId, TInterface implementation, @@ -118,8 +118,8 @@ private void AddMethod(MethodInfo method, object target, string[] path, string a var description = new RpcInterfaceMethod(method, path, leaf, defaultName); if (_include != null && !_include(description)) return; string name = _nameRule == null ? defaultName : _nameRule(description); - if (string.IsNullOrWhiteSpace(name) || name.StartsWith("rpc.", StringComparison.Ordinal)) - throw new ArgumentException("Invalid or reserved JSON-RPC interface method name: '" + name + "'."); + if (string.IsNullOrWhiteSpace(name)) + throw new ArgumentException("Invalid JSON-RPC interface method name: '" + name + "'."); if (Entries.ContainsKey(name)) throw new ArgumentException("Duplicate JSON-RPC interface method name '" + name + "'."); if (method.ContainsGenericParameters) throw new ArgumentException("Generic interface method '" + method.Name + "' is not supported."); diff --git a/README.md b/README.md index 51d8cd1..555bf2a 100644 --- a/README.md +++ b/README.md @@ -177,7 +177,7 @@ That is the whole in-process server. The rest of this page is about exposing met ## Defining methods -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. +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. ### Classes diff --git a/docs/upgrading.md b/docs/upgrading.md index 7bbd7d9..22c5353 100644 --- a/docs/upgrading.md +++ b/docs/upgrading.md @@ -11,6 +11,7 @@ Most 1.x services run unchanged. Read the first list before you build, and the s - **Overloads.** The default-session string overloads that take a serializer take it first: `Process(serializer, json, context)` and `ProcessSync(serializer, json, context)`. `ProcessSync(sessionId, json, context, serializer)` makes `context` required, so `ProcessSync(json, null)` still means the default session. `Process` and `ProcessAsync` do not: `Process(json, null)` no longer compiles (it is ambiguous with the `JsonRpcStateAsync` overload), and `ProcessAsync(json, null)` binds to the session overload with `json` as the session id and a null document, which throws `ArgumentNullException`. Write `Process(json)`, `Process(json, context: null)` or `ProcessAsync(json, context: null)`. - **DTOs.** `JsonRequest`, `JsonResponse` and `JsonRpcException` are plain DTOs without Json.NET attributes. `JsonRequest.Params` is the active serializer's object model, so cast to `JObject`/`JArray` only when the Json.NET serializer is active. - **SMD.** `SMD.Services` is an `SMDServiceCollection` (an `IDictionary`) instead of a `Dictionary`, and its setter is gone. Every mutation through it updates the dispatch table at once, so a removed method is unreachable immediately. `SMD.Types` is now `Dictionary>` and a process-wide registry (it was reset whenever a session was created). +- **Reserved names.** Names beginning with `rpc.` and the name `$/cancelRequest` are refused at registration on every path (`BindInterface` refused `rpc.` alone before). ## Changes clients will see on the wire