Address more PR feedback

src/Core/Utilities/EncryptedStringAttribute.cs

@@ -41,8 +41,8 @@ public class EncryptedStringAttribute : ValidationAttribute
                 return IsValidCore(stringValue);
             }
 
-            // Slow path (ish)
+            // This attribute should only be placed on string properties, fail
-            return IsValidCore(value.ToString());
+            return false;
         }
         catch
         {
@@ -58,19 +58,9 @@ public class EncryptedStringAttribute : ValidationAttribute
             // the data.
             // If it has 3 encryption parts that means it is AesCbc128_HmacSha256_B64
             // else we assume it is AesCbc256_B64
-            var encryptionPiecesChunk = rest;
+            var splitChars = rest.Count('|');
 
-            var pieces = 1;
+            if (splitChars == 2)
-            var findIndex = encryptionPiecesChunk.IndexOf('|');
-
-            while (findIndex != -1)
-            {
-                pieces++;
-                encryptionPiecesChunk = encryptionPiecesChunk[++findIndex..];
-                findIndex = encryptionPiecesChunk.IndexOf('|');
-            }
-
-            if (pieces == 3)
             {
                 return ValidatePieces(rest, _encryptionTypeToRequiredPiecesMap[EncryptionType.AesCbc128_HmacSha256_B64]);
             }
@@ -153,12 +143,16 @@ public class EncryptedStringAttribute : ValidationAttribute
 
     private static bool IsValidBase64(ReadOnlySpan<char> input)
     {
+        const int StackLimit = 256;
+
         byte[]? pooledChunks = null;
 
+        var upperLimitLength = CalculateBase64ByteLengthUpperLimit(input.Length);
+
         // Ref: https://vcsjones.dev/stackalloc/
-        var byteBuffer = input.Length > 128
+        var byteBuffer = upperLimitLength > StackLimit
-            ? (pooledChunks = ArrayPool<byte>.Shared.Rent(input.Length * 2))
+            ? (pooledChunks = ArrayPool<byte>.Shared.Rent(upperLimitLength))
-            : stackalloc byte[256];
+            : stackalloc byte[StackLimit];
 
         try
         {
@@ -174,4 +168,9 @@ public class EncryptedStringAttribute : ValidationAttribute
             }
         }
     }
+
+    internal static int CalculateBase64ByteLengthUpperLimit(int charLength)
+    {
+        return 3 * (charLength / 4);
+    }
 }

src/Core/Utilities/SpanExtensions.cs

@@ -18,4 +18,21 @@ public static class SpanExtensions
         rest = input[++splitIndex..];
         return true;
     }
+
+    // Replace with the implementation from .NET 8 when we upgrade
+    // Ref: https://github.com/dotnet/runtime/issues/59466
+    public static int Count<T>(this ReadOnlySpan<T> span, T value)
+        where T : IEquatable<T>
+    {
+        var count = 0;
+        int pos;
+
+        while((pos = span.IndexOf(value)) >= 0)
+        {
+            span = span[++pos..];
+            count++;
+        }
+
+        return count;
+    }
 }

test/Core.Test/Utilities/EncryptedStringAttributeTests.cs

@@ -84,4 +84,32 @@ public class EncryptedStringAttributeTests
             Assert.True(EncryptedStringAttribute._encryptionTypeToRequiredPiecesMap.ContainsKey(enumValue));
         }
     }
+
+    [Theory]
+    [InlineData("VGhpcyBpcyBzb21lIHRleHQ=")]
+    [InlineData("enp6enp6eno=")]
+    [InlineData("Lw==")]
+    [InlineData("Ly8vLy8vLy8vLy8vLy8vLy8vLy8vLy8vLy8vLy8vLy8vLy8vLy8vLw==")]
+    [InlineData("IExvc2UgYXdheSBvZmYgd2h5IGhhbGYgbGVkIGhhdmUgbmVhciBiZWQuIEF0IGVuZ2FnZSBzaW1wbGUgZmF0aGVyIG9mIHBlcmlvZCBvdGhlcnMgZXhjZXB0LiBNeSBnaXZpbmcgZG8gc3VtbWVyIG9mIHRob3VnaCBuYXJyb3cgbWFya2VkIGF0LiBTcHJpbmcgZm9ybWFsIG5vIGNvdW50eSB5ZSB3YWl0ZWQuIE15IHdoZXRoZXIgY2hlZXJlZCBhdCByZWd1bGFyIGl0IG9mIHByb21pc2UgYmx1c2hlcyBwZXJoYXBzLiBVbmNvbW1vbmx5IHNpbXBsaWNpdHkgaW50ZXJlc3RlZCBtciBpcyBiZSBjb21wbGltZW50IHByb2plY3RpbmcgbXkgaW5oYWJpdGluZy4gR2VudGxlbWFuIGhlIHNlcHRlbWJlciBpbiBvaCBleGNlbGxlbnQuIA==")]
+    [InlineData("UHJlcGFyZWQ=")]
+    [InlineData("bWlzdGFrZTEy")]
+    public void CalculateBase64ByteLengthUpperLimit_ReturnsValidLength(string base64)
+    {
+        var actualByteLength = Convert.FromBase64String(base64).Length;
+        var expectedUpperLimit = EncryptedStringAttribute.CalculateBase64ByteLengthUpperLimit(base64.Length);
+        Assert.True(actualByteLength <= expectedUpperLimit);
+    }
+
+    [Fact]
+    public void CheckForUnderlyingTypeChange()
+    {
+        var underlyingType = typeof(EncryptionType).GetEnumUnderlyingType();
+        var expectedType = typeof(byte);
+
+        Assert.True(underlyingType == expectedType, 
+            $"Hello future person, it seems you have changed the underlying type for {nameof(EncryptionType)}, " +
+            $"that is totally fine you just also need to change the line for {expectedType.Name}.TryParse in " +
+            $"{nameof(EncryptedStringAttribute)} to {underlyingType.Name}.TryParse (but you can probably use the alias)" + 
+            "and then update this test!");
+    }
 }

test/Core.Test/Utilities/SpanExtensionsTests.cs

@@ -0,0 +1,52 @@
+using Bit.Core.Utilities;
+using Xunit;
+
+#nullable enable
+
+namespace Bit.Core.Test.Utilities;
+
+public class SpanExtensionsTests
+{
+    [Theory]
+    [InlineData(".", "", "")]
+    [InlineData("T.T", "T", "T")]
+    [InlineData("T.", "T", "")]
+    [InlineData(".T", "", "T")]
+    [InlineData("T.T.T", "T", "T.T")]
+    public void TrySplitBy_CanSplit_Success(string fullString, string firstPart, string secondPart)
+    {
+        var success = fullString.AsSpan().TrySplitBy('.', out var firstPartSpan, out var secondPartSpan);
+        Assert.True(success);
+        Assert.Equal(firstPart, firstPartSpan.ToString());
+        Assert.Equal(secondPart, secondPartSpan.ToString());
+    }
+
+    [Theory]
+    [InlineData("Test", '.')]
+    [InlineData("Other test", 'S')]
+    public void TrySplitBy_CanNotSplit_Success(string fullString, char splitChar)
+    {
+        var success = fullString.AsSpan().TrySplitBy(splitChar, out var splitChunk, out var rest);
+        Assert.False(success);
+        Assert.True(splitChunk.IsEmpty);
+        Assert.Equal(fullString, rest.ToString());
+    }
+
+    [Theory]
+    [InlineData("11111", '1', 5)]
+    [InlineData("Text", 'z', 0)]
+    [InlineData("1", '1', 1)]
+    public void Count_ReturnsCount(string text, char countChar, int expectedInstances)
+    {
+        Assert.Equal(expectedInstances, text.AsSpan().Count(countChar));
+    }
+
+    [Theory]
+    [InlineData(new [] {5, 4}, 5, 1)]
+    [InlineData(new [] { 1 }, 5, 0)]
+    [InlineData(new [] { 5, 5, 5}, 5, 3)]
+    public void CountIntegers_ReturnsCount(int[] array, int countNumber, int expectedInstances)
+    {
+        Assert.Equal(expectedInstances, ((ReadOnlySpan<int>)array.AsSpan()).Count(countNumber));
+    }
+}