AUS-1003: Refuse reserved method names in SMDServiceCollection

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
+{
+    /// <summary>
+    /// Reserved method names (<c>rpc.</c>-prefixed and <c>$/cancelRequest</c>) are refused by
+    /// <see cref="SMDServiceCollection"/> itself, so every registration path refuses them.
+    /// </summary>
+    [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<string, Type> { ["returns"] = typeof(int) }, new Dictionary<string, object>(), new Func<int>(() => 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<ArgumentException>(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<string, SMDService>
+        {
+            public int Count => 1;
+            public IEnumerable<string> Keys => new string[] { null };
+            public IEnumerable<SMDService> 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<KeyValuePair<string, SMDService>> GetEnumerator()
+            {
+                yield return new KeyValuePair<string, SMDService>(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<IPair>(Session, new Pair(),
+                    new RpcInterfaceBindingOptions { NameRule = m => m.Leaf == "First" ? name : "second" }));
+                Assert.AreEqual(0, Services.Count);
+            }
+            foreach (var name in Allowed)
+            {
+                ServiceBinder.BindInterface<IPair>(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<string, Type> { ["returns"] = typeof(int) }, null, new Func<int>(() => 1)));
+            foreach (var name in Allowed)
+            {
+                handler.RegisterFuction(name, new Dictionary<string, Type> { ["returns"] = typeof(int) }, null, new Func<int>(() => 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<string, SMDService>(name, NewService(1))));
+                AssertReserved(name, () => services[name] = NewService(1));
+                Assert.AreEqual("key", Assert.Throws<ArgumentException>(() => services.Add(name, NewService(1))).ParamName);
+                Assert.AreEqual("key", Assert.Throws<ArgumentException>(() => 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<string, SMDService>(name, NewService(2)));
+                AssertRegistered(name);
+                var replacement = NewService(3);
+                services[name] = replacement;
+                Assert.AreSame(replacement, Find(name));
+            }
+
+            Assert.Throws<ArgumentNullException>(() => services.Add(null, NewService(1)));
+            Assert.Throws<ArgumentNullException>(() => 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<ArgumentException>(() => ServiceBinder.BindInterface<IPair>(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<ArgumentNullException>(() => 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<ArgumentException>(() => Services.AddReserved("rpc.discover", NewService(2)));
+            Assert.AreSame(first, Find("rpc.discover"), "the first registration stands");
+            Assert.AreEqual(1, Services.Count);
+
+            Assert.Throws<ArgumentNullException>(() => Services.AddReserved(null, NewService(1)));
+            Assert.Throws<ArgumentNullException>(() => Services.AddReserved("rpc.other", null));
+        }
+    }
+}

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`.

Json-Rpc/SMDService.cs

@@ -109,19 +109,36 @@ private static string TypeHash(Dictionary<string, object> 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 <c>rpc.</c> and the name <c>$/cancelRequest</c>
+    /// are reserved: every public way of adding a service refuses them with an <see cref="ArgumentException"/>.
     /// </summary>
     public sealed class SMDServiceCollection : IDictionary<string, SMDService>, IReadOnlyDictionary<string, SMDService>
     {
         private Dictionary<string, SMDService> _services = new Dictionary<string, SMDService>();
         private readonly Utf8KeyTable<SMDService> _table = new Utf8KeyTable<SMDService>();
         private readonly object _sync = new object();
 
+        private const string ReservedPrefix = "rpc.";
+        private const string CancelRequest = "$/cancelRequest";
+
+        /// <summary>
+        /// Refuses the names the specification reserves (<c>rpc.</c>-prefixed, ordinal and case-sensitive) and
+        /// <c>$/cancelRequest</c>. The name is compared as given: no trimming and no case folding.
+        /// </summary>
+        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<string, SMDService> 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<string, SMDService>(_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<SMDService> Values
         IEnumerable<SMDService> IReadOnlyDictionary<string, SMDService>.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);
+            }
+        }
+
+        /// <summary>
+        /// Adds a service under a reserved name, for the library's own <c>rpc.discover</c> registration.
+        /// It bypasses only the reserved-name check: an existing <paramref name="key"/> still throws
+        /// <see cref="ArgumentException"/>. Internal, so the reserved check is the only public path; unused in 2.0.0.
+        /// </summary>
+        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);
             }

Json-Rpc/ServiceBinder.Interface.cs

@@ -21,8 +21,8 @@ public static RpcBinding BindInterface<TInterface>(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 <c>rpc.</c>, duplicate, and occupied
-        /// names are rejected. Generic methods and default interface bodies are unsupported.
+        /// nothing; getter side effects cannot be undone. Empty, reserved (<c>rpc.</c>-prefixed or <c>$/cancelRequest</c>),
+        /// 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.
         /// </summary>
         public static RpcBinding BindInterface<TInterface>(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.");

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