* clean up and optimize actor name validation. (#6919)

* * clean up and optimize actor name validation.

* Change ValidSymbols const to public (referenced in docs) and add unit test

* Fix API Approval list

---------

Co-authored-by: Gregorius Soedharmo <[email protected]>

Author: markusschaber

src/core/Akka.API.Tests/verify/CoreAPISpec.ApproveCore.DotNet.verified.txt

@@ -178,6 +178,7 @@ namespace Akka.Actor
     }
     public abstract class ActorPath : Akka.Util.ISurrogated, System.IComparable<Akka.Actor.ActorPath>, System.IEquatable<Akka.Actor.ActorPath>
     {
+        public const string ValidSymbols = "\"-_.*$+:@&=,!~\';()";
         protected ActorPath(Akka.Actor.Address address, string name) { }
         protected ActorPath(Akka.Actor.ActorPath parentPath, string name, long uid) { }
         public Akka.Actor.Address Address { get; }

src/core/Akka.API.Tests/verify/CoreAPISpec.ApproveCore.Net.verified.txt

@@ -178,6 +178,7 @@ namespace Akka.Actor
     }
     public abstract class ActorPath : Akka.Util.ISurrogated, System.IComparable<Akka.Actor.ActorPath>, System.IEquatable<Akka.Actor.ActorPath>
     {
+        public const string ValidSymbols = "\"-_.*$+:@&=,!~\';()";
         protected ActorPath(Akka.Actor.Address address, string name) { }
         protected ActorPath(Akka.Actor.ActorPath parentPath, string name, long uid) { }
         public Akka.Actor.Address Address { get; }

src/core/Akka.Tests/Actor/ActorPathSpec.cs

@@ -11,6 +11,7 @@
 using Akka.Actor;
 using Akka.TestKit;
 using Xunit;
+using FluentAssertions;
 
 namespace Akka.Tests.Actor
 {
@@ -274,6 +275,33 @@ public void Validate_element_parts(string element, bool matches)
             ActorPath.IsValidPathElement(element).ShouldBe(matches);
         }
 
+        [Fact]
+        public void ValidateElementPartsComprehensive()
+        {
+            // NOTE: $ is not a valid starting character
+            var valid = Enumerable.Range(0, 128).Select(i => (char)i).Where(c =>
+                c is >= 'a' and <= 'z' || 
+                c is >= 'A' and <= 'Z' || 
+                c is >= '0' and <= '9' ||
+                ActorPath.ValidSymbols.Contains(c)
+                && c is not '$'
+            ).ToArray();
+
+            foreach (var c in Enumerable.Range(0, 2048).Select(i => (char)i))
+            {
+                if(valid.Contains(c))
+                    ActorPath.IsValidPathElement(new string(new[] { c })).Should().BeTrue();
+                else
+                    ActorPath.IsValidPathElement(new string(new[] { c })).Should().BeFalse();
+            }
+
+            // $ after a valid character should be valid
+            foreach (var c in valid)
+            {
+                ActorPath.IsValidPathElement(new string(new[] { c, '$' })).Should().BeTrue();
+            }
+        }
+
         [Theory]
         [InlineData("$NamesMayNotNormallyStartWithDollarButShouldBeOkWHenEncoded")]
         [InlineData("NamesMayContain$Dollar")]

src/core/Akka/Actor/ActorCell.Children.cs

@@ -8,15 +8,11 @@
 using System;
 using System.Collections.Generic;
 using System.Collections.Immutable;
-using System.Text;
 using System.Runtime.CompilerServices;
 using System.Runtime.Serialization;
 using System.Threading;
 using Akka.Actor.Internal;
-using Akka.Dispatch.SysMsg;
-using Akka.Serialization;
 using Akka.Util;
-using Akka.Util.Internal;
 
 namespace Akka.Actor
 {
@@ -80,12 +76,12 @@ protected void StopFunctionRefs()
 
         /// <summary>
         /// Attaches a child to the current <see cref="ActorCell"/>.
-        /// 
+        ///
         /// This method is used in the process of starting actors.
         /// </summary>
         /// <param name="props">The <see cref="Props"/> this child actor will use.</param>
         /// <param name="isSystemService">If <c>true</c>, then this actor is a system actor and activates a special initialization path.</param>
-        /// <param name="name">The name of the actor being started. Can be <c>null</c>, and if it is we will automatically 
+        /// <param name="name">The name of the actor being started. Can be <c>null</c>, and if it is we will automatically
         /// generate a random name for this actor.</param>
         /// <exception cref="InvalidActorNameException">
         /// This exception is thrown if the given <paramref name="name"/> is an invalid actor name.
@@ -199,7 +195,7 @@ protected void UnreserveChild(string name)
         }
 
         /// <summary>
-        /// This should only be used privately or when creating the root actor. 
+        /// This should only be used privately or when creating the root actor.
         /// </summary>
         /// <param name="actor">TBD</param>
         /// <returns>TBD</returns>
@@ -307,8 +303,8 @@ private void ResumeChildren(Exception causedByFailure, IActorRef perpetrator)
         }
 
         /// <summary>
-        /// Tries to get the stats for the child with the specified name. The stats can be either <see cref="ChildNameReserved"/> 
-        /// indicating that only a name has been reserved for the child, or a <see cref="ChildRestartStats"/> for a child that 
+        /// Tries to get the stats for the child with the specified name. The stats can be either <see cref="ChildNameReserved"/>
+        /// indicating that only a name has been reserved for the child, or a <see cref="ChildRestartStats"/> for a child that
         /// has been initialized/created.
         /// </summary>
         /// <param name="name">TBD</param>
@@ -383,7 +379,7 @@ public IInternalActorRef GetSingleChild(string name)
             }
             return ActorRefs.Nobody;
         }
-        
+
         /// <summary>
         /// TBD
         /// </summary>
@@ -421,7 +417,7 @@ private static string CheckName(string name)
             if (name.Length == 0) throw new InvalidActorNameException("Actor name must not be empty.");
             if (!ActorPath.IsValidPathElement(name))
             {
-                throw new InvalidActorNameException($"Illegal actor name [{name}]. Actor paths MUST: not start with `$`, include only ASCII letters and can only contain these special characters: ${new string(ActorPath.ValidSymbols)}.");
+                throw new InvalidActorNameException($"Illegal actor name [{name}]. {ActorPath.ValidActorNameDescription}");
             }
             return name;
         }
@@ -437,7 +433,7 @@ private IInternalActorRef MakeChild(Props props, string name, bool async, bool s
                     if (oldInfo == null)
                         Serialization.Serialization.CurrentTransportInformation =
                             SystemImpl.Provider.SerializationInformation;
-                    
+
                     var ser = _systemImpl.Serialization;
                     if (props.Arguments != null)
                     {

src/core/Akka/Actor/ActorPath.cs

@@ -92,25 +92,36 @@ public override int GetHashCode()
             #endregion
         }
 
+        public const string ValidSymbols = "\"-_.*$+:@&=,!~';()";
+
         /// <summary>
-        /// INTERNAL API
+        /// A small bool array, indexed by the ASCII code (0..127), containing <c>true</c> for valid chars
+        /// and <c>false</c> for invalid characters.
         /// </summary>
-        internal static readonly char[] ValidSymbols = @"""-_.*$+:@&=,!~';""()".ToCharArray();
+        private static readonly bool[] ValidAscii = Enumerable.Range(0, 128).Select(c => (c >= 'a' && c <= 'z')
+                || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || ValidSymbols.Contains((char)c))
+            .ToArray();
+
+        /// <summary>
+        /// A human readable description of a valid actor name. This is used in several places to throw
+        /// exceptions when the caller supplies invalid actor names.
+        /// </summary>
+        internal const string ValidActorNameDescription=
+            $"Actor paths MUST: not start with `$`, not be empty, and only contain ASCII letters, digits and these special characters: `{ValidSymbols}`";
 
         /// <summary>
         /// Method that checks if actor name conforms to RFC 2396, http://www.ietf.org/rfc/rfc2396.txt
         /// Note that AKKA JVM does not allow parenthesis ( ) but, according to RFC 2396 those are allowed, and
         /// since we use URL Encode to create valid actor names, we must allow them.
         /// </summary>
-        /// <param name="s">TBD</param>
-        /// <returns>TBD</returns>
+        /// <param name="s">The string to verify for conformity</param>
+        /// <returns>True if the path element is valid</returns>
         public static bool IsValidPathElement(string s)
         {
             return !string.IsNullOrEmpty(s) && !s.StartsWith('$') && Validate(s);
         }
 
-        private static bool IsValidChar(char c) => (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') ||
-                                                   (c >= '0' && c <= '9') || ValidSymbols.Contains(c);
+        private static bool IsValidChar(char c) => c < 128 && ValidAscii[c];
 
         private static bool IsHexChar(char c) => (c >= 'a' && c <= 'f') || (c >= 'A' && c <= 'F') ||
                                                  (c >= '0' && c <= '9');
@@ -584,24 +595,24 @@ void AppendUidSpan(ref Span<char> writeable, int startPos, int sizeHint)
                     totalLength += p._name.Length + 1;
                     p = p._parent;
                 }
-                
+
                 // UID calculation
                 var uidSizeHint = 0;
                 if (uid != null)
                 {
                     // 1 extra character for the '#'
                     uidSizeHint = SpanHacks.Int64SizeInCharacters(uid.Value) + 1;
-                    totalLength += uidSizeHint; 
+                    totalLength += uidSizeHint;
                 }
 
-                // Concatenate segments (in reverse order) into buffer with '/' prefixes                
+                // Concatenate segments (in reverse order) into buffer with '/' prefixes
                 Span<char> buffer = totalLength < 1024 ? stackalloc char[totalLength] : new char[totalLength];
                 prefix.CopyTo(buffer);
 
                 var offset = buffer.Length - uidSizeHint;
                 // append UID span first
                 AppendUidSpan(ref buffer, offset, uidSizeHint-1); // -1 for the '#'
-                
+
                 p = this;
                 while (p._depth > 0)
                 {
@@ -752,11 +763,11 @@ private string ToStringWithAddress(Address address, bool includeUid)
                 // we never change address for IgnoreActorRef
                 return ToString();
             }
-            
+
             long? uid = null;
             if (includeUid && _uid != ActorCell.UndefinedUid)
                 uid = _uid;
-            
+
             if (_address.Host != null && _address.Port.HasValue)
                 return Join(_address.ToString().AsSpan(), uid);
 

src/core/Akka/Actor/Deployer.cs

@@ -97,7 +97,7 @@ void Add(IList<string> path, Deploy d)
                     if (!ActorPath.IsValidPathElement(t))
                     {
                         throw new IllegalActorNameException(
-                            $"Illegal actor name [{t}] in deployment [${d.Path}]. Actor paths MUST: not start with `$`, include only ASCII letters and can only contain these special characters: ${new string(ActorPath.ValidSymbols)}.");
+                            $"Illegal actor name [{t}] in deployment [${d.Path}]. {ActorPath.ValidActorNameDescription}");
                     }
                 }
                 if (!_deployments.CompareAndSet(w, w.Insert(path, d))) Add(path, d);