From 7763dae2002e95b51deaa830cdad8bdcbb83e729 Mon Sep 17 00:00:00 2001 From: BrennanConroy Date: Wed, 24 Mar 2021 10:34:28 -0700 Subject: [PATCH 1/2] Remove unsafe from BytesOrdinalEqualsStringAndAscii --- .../ServerInfrastructure/StringUtilities.cs | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/Shared/ServerInfrastructure/StringUtilities.cs b/src/Shared/ServerInfrastructure/StringUtilities.cs index 8b7d27b4e7ab..2e9c4cee8936 100644 --- a/src/Shared/ServerInfrastructure/StringUtilities.cs +++ b/src/Shared/ServerInfrastructure/StringUtilities.cs @@ -389,7 +389,7 @@ out Unsafe.AsRef>(output), } [MethodImpl(MethodImplOptions.AggressiveOptimization)] - public unsafe static bool BytesOrdinalEqualsStringAndAscii(string previousValue, ReadOnlySpan newValue) + public static bool BytesOrdinalEqualsStringAndAscii(string previousValue, ReadOnlySpan newValue) { // previousValue is a previously materialized string which *must* have already passed validation. Debug.Assert(IsValidHeaderString(previousValue)); @@ -412,8 +412,8 @@ public unsafe static bool BytesOrdinalEqualsStringAndAscii(string previousValue, // This isn't problematic as we know the maximum length is max string length (from test above) // which is a signed value so half the size of the unsigned pointer value so we can safely add // a Vector.Count to it without overflowing. - var count = (IntPtr)newValue.Length; - var offset = (IntPtr)0; + var count = (nint)newValue.Length; + var offset = (nint)0; // Get references to the first byte in the span, and the first char in the string. ref var bytes = ref MemoryMarshal.GetReference(newValue); @@ -422,12 +422,12 @@ public unsafe static bool BytesOrdinalEqualsStringAndAscii(string previousValue, do { // If Vector not-accelerated or remaining less than vector size - if (!Vector.IsHardwareAccelerated || (byte*)(offset + Vector.Count) > (byte*)count) + if (!Vector.IsHardwareAccelerated || (offset + Vector.Count) > count) { if (IntPtr.Size == 8) // Use Intrinsic switch for branch elimination { // 64-bit: Loop longs by default - while ((byte*)(offset + sizeof(long)) <= (byte*)count) + while ((offset + sizeof(long)) <= count) { if (!WidenFourAsciiBytesToUtf16AndCompareToChars( ref Unsafe.Add(ref str, offset), @@ -441,7 +441,7 @@ ref Unsafe.Add(ref str, offset + 4), offset += sizeof(long); } - if ((byte*)(offset + sizeof(int)) <= (byte*)count) + if ((offset + sizeof(int)) <= count) { if (!WidenFourAsciiBytesToUtf16AndCompareToChars( ref Unsafe.Add(ref str, offset), @@ -456,7 +456,7 @@ ref Unsafe.Add(ref str, offset), else { // 32-bit: Loop ints by default - while ((byte*)(offset + sizeof(int)) <= (byte*)count) + while ((offset + sizeof(int)) <= count) { if (!WidenFourAsciiBytesToUtf16AndCompareToChars( ref Unsafe.Add(ref str, offset), @@ -468,7 +468,7 @@ ref Unsafe.Add(ref str, offset), offset += sizeof(int); } } - if ((byte*)(offset + sizeof(short)) <= (byte*)count) + if ((offset + sizeof(short)) <= count) { if (!WidenTwoAsciiBytesToUtf16AndCompareToChars( ref Unsafe.Add(ref str, offset), @@ -479,7 +479,7 @@ ref Unsafe.Add(ref str, offset), offset += sizeof(short); } - if ((byte*)offset < (byte*)count) + if (offset < count) { var ch = (char)Unsafe.Add(ref bytes, offset); if (((ch & 0x80) != 0) || Unsafe.Add(ref str, offset) != ch) @@ -527,11 +527,11 @@ ref Unsafe.Add(ref str, offset), } offset += Vector.Count; - } while ((byte*)(offset + Vector.Count) <= (byte*)count); + } while ((offset + Vector.Count) <= count); // Vector path done, loop back to do non-Vector // If is a exact multiple of vector size, bail now - } while ((byte*)offset < (byte*)count); + } while (offset < count); // If we get here (input is exactly a multiple of Vector length) then there are no inequalities via widening; // so the input bytes are both ascii and a match to the string if it was converted via Encoding.ASCII.GetString(...) @@ -651,7 +651,7 @@ private static bool AllBytesInUInt16AreAscii(ushort value) return ((value & 0x8080u) == 0); } - private unsafe static bool IsValidHeaderString(string value) + private static bool IsValidHeaderString(string value) { // Method for Debug.Assert to ensure BytesOrdinalEqualsStringAndAscii // is not called with an unvalidated string comparitor. From 0a977f4e6e976f4e95a70bdae51ae1bdacf79997 Mon Sep 17 00:00:00 2001 From: BrennanConroy Date: Wed, 24 Mar 2021 11:01:08 -0700 Subject: [PATCH 2/2] assert message --- src/Servers/Kestrel/Core/test/AsciiDecoding.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Servers/Kestrel/Core/test/AsciiDecoding.cs b/src/Servers/Kestrel/Core/test/AsciiDecoding.cs index 128b79fdd034..32faf184c5e1 100644 --- a/src/Servers/Kestrel/Core/test/AsciiDecoding.cs +++ b/src/Servers/Kestrel/Core/test/AsciiDecoding.cs @@ -60,7 +60,7 @@ private void ExceptionThrownForZeroOrNonAscii(byte b) { var byteRange = Enumerable.Range(1, length).Select(x => (byte)x).ToArray(); byteRange[position] = b; - + Assert.Throws(() => new Span(byteRange).GetAsciiStringNonNullCharacters()); } } @@ -147,7 +147,7 @@ static void Test(Span asciiBytes) // Change byte back for next iteration, ensure is equal again asciiBytes[i] = b; - Assert.True(StringUtilities.BytesOrdinalEqualsStringAndAscii(s, asciiBytes)); + Assert.True(StringUtilities.BytesOrdinalEqualsStringAndAscii(s, asciiBytes), s); } } }