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