Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

* clean up and optimize actor name validation. #6919

Merged
merged 3 commits into from
Sep 18, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions src/core/Akka.Tests/Actor/ActorPathSpec.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using Akka.Actor;
using Akka.TestKit;
using Xunit;
using FluentAssertions;

namespace Akka.Tests.Actor
{
Expand Down Expand Up @@ -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")]
Expand Down
20 changes: 8 additions & 12 deletions src/core/Akka/Actor/ActorCell.Children.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -383,7 +379,7 @@ public IInternalActorRef GetSingleChild(string name)
}
return ActorRefs.Nobody;
}

/// <summary>
/// TBD
/// </summary>
Expand Down Expand Up @@ -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;
}
Expand All @@ -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)
{
Expand Down
35 changes: 23 additions & 12 deletions src/core/Akka/Actor/ActorPath.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion src/core/Akka/Actor/Deployer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down