From 825874fd561a6d6da31e00d1a10e6d7876573b31 Mon Sep 17 00:00:00 2001 From: Justin Baur <19896123+justindbaur@users.noreply.github.com> Date: Wed, 21 Sep 2022 14:54:10 -0400 Subject: [PATCH] Address PR feedback pt.1 --- src/Core/Utilities/EncryptedStringAttribute.cs | 14 +++++++------- src/Core/Utilities/SpanExtensions.cs | 2 +- .../Utilities/EncryptedStringAttributeTests.cs | 4 ++-- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/Core/Utilities/EncryptedStringAttribute.cs b/src/Core/Utilities/EncryptedStringAttribute.cs index f31aa39692..9be93a6a6d 100644 --- a/src/Core/Utilities/EncryptedStringAttribute.cs +++ b/src/Core/Utilities/EncryptedStringAttribute.cs @@ -11,7 +11,7 @@ namespace Bit.Core.Utilities; /// public class EncryptedStringAttribute : ValidationAttribute { - internal static readonly Dictionary _encryptionTypeMap = new() + internal static readonly Dictionary _encryptionTypeToRequiredPiecesMap = new() { [EncryptionType.AesCbc256_B64] = 2, // iv|ct [EncryptionType.AesCbc128_HmacSha256_B64] = 3, // iv|ct|mac @@ -54,7 +54,7 @@ public class EncryptedStringAttribute : ValidationAttribute { if (!value.TrySplitBy('.', out var headerChunk, out var rest)) { - // We coundn't find a header part, this is the slow path, because we have to do two loops over + // We couldn't find a header part, this is the slow path, because we have to do two loops over // the data. // If it has 3 encryption parts that means it is AesCbc128_HmacSha256_B64 // else we assume it is AesCbc256_B64 @@ -72,11 +72,11 @@ public class EncryptedStringAttribute : ValidationAttribute if (pieces == 3) { - return ValidatePieces(rest, _encryptionTypeMap[EncryptionType.AesCbc128_HmacSha256_B64]); + return ValidatePieces(rest, _encryptionTypeToRequiredPiecesMap[EncryptionType.AesCbc128_HmacSha256_B64]); } else { - return ValidatePieces(rest, _encryptionTypeMap[EncryptionType.AesCbc256_B64]); + return ValidatePieces(rest, _encryptionTypeToRequiredPiecesMap[EncryptionType.AesCbc256_B64]); } } @@ -94,7 +94,7 @@ public class EncryptedStringAttribute : ValidationAttribute // Since this value came from Enum.TryParse we know it is an enumerated object and we can therefore // just access the dictionary - return ValidatePieces(rest, _encryptionTypeMap[encryptionType]); + return ValidatePieces(rest, _encryptionTypeToRequiredPiecesMap[encryptionType]); } // Simply cast the number to the enum, this could be a value that doesn't actually have a backing enum @@ -102,7 +102,7 @@ public class EncryptedStringAttribute : ValidationAttribute // numbers will be filtered out there. encryptionType = (EncryptionType)encryptionTypeNumber; - if (!_encryptionTypeMap.TryGetValue(encryptionType, out var encryptionPieces)) + if (!_encryptionTypeToRequiredPiecesMap.TryGetValue(encryptionType, out var encryptionPieces)) { // Could not find a configuration map for the given header piece. This is an invalid string return false; @@ -170,7 +170,7 @@ public class EncryptedStringAttribute : ValidationAttribute // Check if we rented the pool and if so, return it. if (pooledChunks != null) { - ArrayPool.Shared.Return(pooledChunks); + ArrayPool.Shared.Return(pooledChunks, true); } } } diff --git a/src/Core/Utilities/SpanExtensions.cs b/src/Core/Utilities/SpanExtensions.cs index cc17a51568..16e767079c 100644 --- a/src/Core/Utilities/SpanExtensions.cs +++ b/src/Core/Utilities/SpanExtensions.cs @@ -7,7 +7,7 @@ public static class SpanExtensions { var splitIndex = input.IndexOf(splitChar); - if (splitIndex < 1) + if (splitIndex == -1) { chunk = default; rest = input; diff --git a/test/Core.Test/Utilities/EncryptedStringAttributeTests.cs b/test/Core.Test/Utilities/EncryptedStringAttributeTests.cs index 8d534650ab..beba4194c5 100644 --- a/test/Core.Test/Utilities/EncryptedStringAttributeTests.cs +++ b/test/Core.Test/Utilities/EncryptedStringAttributeTests.cs @@ -75,13 +75,13 @@ public class EncryptedStringAttributeTests public void EncryptionTypeMap_HasEntry_ForEachEnumValue() { var enumValues = Enum.GetValues(); - Assert.Equal(enumValues.Length, EncryptedStringAttribute._encryptionTypeMap.Count); + Assert.Equal(enumValues.Length, EncryptedStringAttribute._encryptionTypeToRequiredPiecesMap.Count); foreach (var enumValue in enumValues) { // Go a step further and ensure that the map contains a value for each value instead of just casting // a random number for one of the keys. - Assert.True(EncryptedStringAttribute._encryptionTypeMap.ContainsKey(enumValue)); + Assert.True(EncryptedStringAttribute._encryptionTypeToRequiredPiecesMap.ContainsKey(enumValue)); } } }