From 452857b2fa04bff239d0a98aeb1d14c25dd692f7 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 17 Nov 2024 12:31:24 +0100 Subject: [PATCH] Remove unsafe quirks from System.Text.Encodings.Web --- .../Text/Encodings/Web/DefaultHtmlEncoder.cs | 20 +- .../System/Text/Encodings/Web/SpanUtility.cs | 224 +----------------- .../tests/SpanUtilityTests.cs | 114 --------- 3 files changed, 13 insertions(+), 345 deletions(-) diff --git a/src/libraries/System.Text.Encodings.Web/src/System/Text/Encodings/Web/DefaultHtmlEncoder.cs b/src/libraries/System.Text.Encodings.Web/src/System/Text/Encodings/Web/DefaultHtmlEncoder.cs index 5ab8c388d1bc1..2691d8f327011 100644 --- a/src/libraries/System.Text.Encodings.Web/src/System/Text/Encodings/Web/DefaultHtmlEncoder.cs +++ b/src/libraries/System.Text.Encodings.Web/src/System/Text/Encodings/Web/DefaultHtmlEncoder.cs @@ -68,22 +68,22 @@ internal override int EncodeUtf8(Rune value, Span destination) { if (value.Value == '<') { - if (!SpanUtility.TryWriteBytes(destination, (byte)'&', (byte)'l', (byte)'t', (byte)';')) { goto OutOfSpace; } + if (!"<"u8.TryCopyTo(destination)) { goto OutOfSpace; } return 4; } else if (value.Value == '>') { - if (!SpanUtility.TryWriteBytes(destination, (byte)'&', (byte)'g', (byte)'t', (byte)';')) { goto OutOfSpace; } + if (!">"u8.TryCopyTo(destination)) { goto OutOfSpace; } return 4; } else if (value.Value == '&') { - if (!SpanUtility.TryWriteBytes(destination, (byte)'&', (byte)'a', (byte)'m', (byte)'p', (byte)';')) { goto OutOfSpace; } + if (!"&"u8.TryCopyTo(destination)) { goto OutOfSpace; } return 5; } else if (value.Value == '\"') { - if (!SpanUtility.TryWriteBytes(destination, (byte)'&', (byte)'q', (byte)'u', (byte)'o', (byte)'t', (byte)';')) { goto OutOfSpace; } + if (!"""u8.TryCopyTo(destination)) { goto OutOfSpace; } return 6; } else @@ -109,7 +109,7 @@ static int TryEncodeScalarAsHex(object @this, uint scalarValue, Span desti if (!SpanUtility.IsValidIndex(destination, idxOfSemicolon)) { goto OutOfSpaceInner; } destination[idxOfSemicolon] = (byte)';'; - if (!SpanUtility.TryWriteBytes(destination, (byte)'&', (byte)'#', (byte)'x', (byte)'0')) + if (!"�"u8.TryCopyTo(destination)) { Debug.Fail("We should've had enough room to write 4 bytes."); } @@ -134,22 +134,22 @@ internal override int EncodeUtf16(Rune value, Span destination) { if (value.Value == '<') { - if (!SpanUtility.TryWriteChars(destination, '&', 'l', 't', ';')) { goto OutOfSpace; } + if (!"<".TryCopyTo(destination)) { goto OutOfSpace; } return 4; } else if (value.Value == '>') { - if (!SpanUtility.TryWriteChars(destination, '&', 'g', 't', ';')) { goto OutOfSpace; } + if (!">".TryCopyTo(destination)) { goto OutOfSpace; } return 4; } else if (value.Value == '&') { - if (!SpanUtility.TryWriteChars(destination, '&', 'a', 'm', 'p', ';')) { goto OutOfSpace; } + if (!"&".TryCopyTo(destination)) { goto OutOfSpace; } return 5; } else if (value.Value == '\"') { - if (!SpanUtility.TryWriteChars(destination, '&', 'q', 'u', 'o', 't', ';')) { goto OutOfSpace; } + if (!""".TryCopyTo(destination)) { goto OutOfSpace; } return 6; } else @@ -186,7 +186,7 @@ static int TryEncodeScalarAsHex(object @this, uint scalarValue, Span desti // It's more efficient to write 4 chars at a time instead of 1 char. // The '0' at the end will be overwritten. - if (!SpanUtility.TryWriteChars(destination, '&', '#', 'x', '0')) + if ("�".TryCopyTo(destination)) { Debug.Fail("We should've had enough room to write 4 chars."); } diff --git a/src/libraries/System.Text.Encodings.Web/src/System/Text/Encodings/Web/SpanUtility.cs b/src/libraries/System.Text.Encodings.Web/src/System/Text/Encodings/Web/SpanUtility.cs index 62dedbea3572c..108094e1b27f3 100644 --- a/src/libraries/System.Text.Encodings.Web/src/System/Text/Encodings/Web/SpanUtility.cs +++ b/src/libraries/System.Text.Encodings.Web/src/System/Text/Encodings/Web/SpanUtility.cs @@ -24,200 +24,6 @@ public static bool IsValidIndex(Span span, int index) return ((uint)index < (uint)span.Length) ? true : false; } - /// - /// Tries writing four bytes to the span. If success, returns true. If the span is not large - /// enough to hold four bytes, leaves the span unchanged and returns false. - /// - /// - /// Parameters are intended to be constant values. - /// - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static bool TryWriteBytes(Span span, byte a, byte b, byte c, byte d) - { - if (span.Length >= 4) - { - uint abcd32; - if (BitConverter.IsLittleEndian) - { - abcd32 = ((uint)d << 24) | ((uint)c << 16) | ((uint)b << 8) | a; - } - else - { - abcd32 = ((uint)a << 24) | ((uint)b << 16) | ((uint)c << 8) | d; - } - Unsafe.WriteUnaligned(ref MemoryMarshal.GetReference(span), abcd32); - return true; - } - else - { - return false; - } - } - - /// - /// Tries writing five bytes to the span. If success, returns true. If the span is not large - /// enough to hold five bytes, leaves the span unchanged and returns false. - /// - /// - /// Parameters are intended to be constant values. - /// - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static bool TryWriteBytes(Span span, byte a, byte b, byte c, byte d, byte e) - { - if (span.Length >= 5) - { - uint abcd32; - if (BitConverter.IsLittleEndian) - { - abcd32 = ((uint)d << 24) | ((uint)c << 16) | ((uint)b << 8) | a; - } - else - { - abcd32 = ((uint)a << 24) | ((uint)b << 16) | ((uint)c << 8) | d; - } - ref byte rDest = ref MemoryMarshal.GetReference(span); - Unsafe.WriteUnaligned(ref rDest, abcd32); - Unsafe.Add(ref rDest, 4) = e; - return true; - } - else - { - return false; - } - } - - /// - /// Tries writing six bytes to the span. If success, returns true. If the span is not large - /// enough to hold six bytes, leaves the span unchanged and returns false. - /// - /// - /// Parameters are intended to be constant values. - /// - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static bool TryWriteBytes(Span span, byte a, byte b, byte c, byte d, byte e, byte f) - { - if (span.Length >= 6) - { - uint abcd32; - uint ef16; - if (BitConverter.IsLittleEndian) - { - abcd32 = ((uint)d << 24) | ((uint)c << 16) | ((uint)b << 8) | a; - ef16 = ((uint)f << 8) | e; - } - else - { - abcd32 = ((uint)a << 24) | ((uint)b << 16) | ((uint)c << 8) | d; - ef16 = ((uint)e << 8) | f; - } - ref byte rDest = ref MemoryMarshal.GetReference(span); - Unsafe.WriteUnaligned(ref rDest, abcd32); - Unsafe.WriteUnaligned(ref Unsafe.Add(ref rDest, 4), (ushort)ef16); - return true; - } - else - { - return false; - } - } - - /// - /// Tries writing four chars to the span. If success, returns true. If the span is not large - /// enough to hold four chars, leaves the span unchanged and returns false. - /// - /// - /// Parameters are intended to be constant values. - /// - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static bool TryWriteChars(Span span, char a, char b, char c, char d) - { - if (span.Length >= 4) - { - ulong abcd64; - if (BitConverter.IsLittleEndian) - { - abcd64 = ((ulong)d << 48) | ((ulong)c << 32) | ((ulong)b << 16) | a; - } - else - { - abcd64 = ((ulong)a << 48) | ((ulong)b << 32) | ((ulong)c << 16) | d; - } - Unsafe.WriteUnaligned(ref Unsafe.As(ref MemoryMarshal.GetReference(span)), abcd64); - return true; - } - else - { - return false; - } - } - - /// - /// Tries writing five chars to the span. If success, returns true. If the span is not large - /// enough to hold five chars, leaves the span unchanged and returns false. - /// - /// - /// Parameters are intended to be constant values. - /// - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static bool TryWriteChars(Span span, char a, char b, char c, char d, char e) - { - if (span.Length >= 5) - { - ulong abcd64; - if (BitConverter.IsLittleEndian) - { - abcd64 = ((ulong)d << 48) | ((ulong)c << 32) | ((ulong)b << 16) | a; - } - else - { - abcd64 = ((ulong)a << 48) | ((ulong)b << 32) | ((ulong)c << 16) | d; - } - ref char rDest = ref MemoryMarshal.GetReference(span); - Unsafe.WriteUnaligned(ref Unsafe.As(ref rDest), abcd64); - Unsafe.Add(ref rDest, 4) = e; - return true; - } - else - { - return false; - } - } - - /// - /// Tries writing six chars to the span. If success, returns true. If the span is not large - /// enough to hold six chars, leaves the span unchanged and returns false. - /// - /// - /// Parameters are intended to be constant values. - /// - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static bool TryWriteChars(Span span, char a, char b, char c, char d, char e, char f) - { - if (span.Length >= 6) - { - ulong abcd64; - uint ef32; - if (BitConverter.IsLittleEndian) - { - abcd64 = ((ulong)d << 48) | ((ulong)c << 32) | ((ulong)b << 16) | a; - ef32 = ((uint)f << 16) | e; - } - else - { - abcd64 = ((ulong)a << 48) | ((ulong)b << 32) | ((ulong)c << 16) | d; - ef32 = ((uint)e << 16) | f; - } - ref byte rDest = ref Unsafe.As(ref MemoryMarshal.GetReference(span)); - Unsafe.WriteUnaligned(ref rDest, abcd64); - Unsafe.WriteUnaligned(ref Unsafe.AddByteOffset(ref rDest, (nint)sizeof(ulong)), ef32); - return true; - } - else - { - return false; - } - } - /// /// Tries writing a 64-bit value as little endian to the span. If success, returns true. If /// the span is not large enough to hold 8 bytes, leaves the span unchanged and returns false. @@ -225,35 +31,11 @@ public static bool TryWriteChars(Span span, char a, char b, char c, char d [MethodImpl(MethodImplOptions.AggressiveInlining)] public static bool TryWriteUInt64LittleEndian(Span span, int offset, ulong value) { - if (AreValidIndexAndLength(span.Length, offset, sizeof(ulong))) - { - if (!BitConverter.IsLittleEndian) - { - value = BinaryPrimitives.ReverseEndianness(value); - } - Unsafe.WriteUnaligned(ref Unsafe.Add(ref MemoryMarshal.GetReference(span), (nint)(uint)offset), value); - return true; - } - else - { - return false; - } - } - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static bool AreValidIndexAndLength(int spanRealLength, int requestedOffset, int requestedLength) - { - // Logic here is copied from Span.Slice. - if (IntPtr.Size == 4) - { - if ((uint)requestedOffset > (uint)spanRealLength) { return false; } - if ((uint)requestedLength > (uint)(spanRealLength - requestedOffset)) { return false; } - } - else + if (!BitConverter.IsLittleEndian) { - if ((ulong)(uint)spanRealLength < (ulong)(uint)requestedOffset + (ulong)(uint)requestedLength) { return false; } + value = BinaryPrimitives.ReverseEndianness(value); } - return true; + return MemoryMarshal.TryWrite(span.Slice(offset), value); } } } diff --git a/src/libraries/System.Text.Encodings.Web/tests/SpanUtilityTests.cs b/src/libraries/System.Text.Encodings.Web/tests/SpanUtilityTests.cs index f97779aaa6135..2ab7aeb37b031 100644 --- a/src/libraries/System.Text.Encodings.Web/tests/SpanUtilityTests.cs +++ b/src/libraries/System.Text.Encodings.Web/tests/SpanUtilityTests.cs @@ -38,120 +38,6 @@ public void IsValidIndex_Span(string inputData, int index, bool expectedValue) Assert.Equal(expectedValue, SpanUtility.IsValidIndex(span, index)); } - [Fact] - public void TryWriteFourBytes() - { - Span span = stackalloc byte[0]; - Assert.False(SpanUtility.TryWriteBytes(span, 10, 20, 30, 40)); - - span = stackalloc byte[3] { 100, 101, 102 }; - Assert.False(SpanUtility.TryWriteBytes(span, 10, 20, 30, 40)); - Assert.Equal(new byte[] { 100, 101, 102 }, span.ToArray()); - - span = stackalloc byte[4] { 100, 101, 102, 103 }; - Assert.True(SpanUtility.TryWriteBytes(span, 10, 20, 30, 40)); - Assert.Equal(new byte[] { 10, 20, 30, 40 }, span.ToArray()); - - span = stackalloc byte[5] { 100, 101, 102, 103, 104 }; - Assert.True(SpanUtility.TryWriteBytes(span, 10, 20, 30, 40)); - Assert.Equal(new byte[] { 10, 20, 30, 40, 104 }, span.ToArray()); - } - - [Fact] - public void TryWriteFiveBytes() - { - Span span = stackalloc byte[0]; - Assert.False(SpanUtility.TryWriteBytes(span, 10, 20, 30, 40, 50)); - - span = stackalloc byte[4] { 100, 101, 102, 103 }; - Assert.False(SpanUtility.TryWriteBytes(span, 10, 20, 30, 40, 50)); - Assert.Equal(new byte[] { 100, 101, 102, 103 }, span.ToArray()); - - span = stackalloc byte[5] { 100, 101, 102, 103, 104 }; - Assert.True(SpanUtility.TryWriteBytes(span, 10, 20, 30, 40, 50)); - Assert.Equal(new byte[] { 10, 20, 30, 40, 50 }, span.ToArray()); - - span = stackalloc byte[6] { 100, 101, 102, 103, 104, 105 }; - Assert.True(SpanUtility.TryWriteBytes(span, 10, 20, 30, 40, 50)); - Assert.Equal(new byte[] { 10, 20, 30, 40, 50, 105 }, span.ToArray()); - } - - [Fact] - public void TryWriteSixBytes() - { - Span span = stackalloc byte[0]; - Assert.False(SpanUtility.TryWriteBytes(span, 10, 20, 30, 40, 50, 60)); - - span = stackalloc byte[5] { 100, 101, 102, 103, 104 }; - Assert.False(SpanUtility.TryWriteBytes(span, 10, 20, 30, 40, 50, 60)); - Assert.Equal(new byte[] { 100, 101, 102, 103, 104 }, span.ToArray()); - - span = stackalloc byte[6] { 100, 101, 102, 103, 104, 105 }; - Assert.True(SpanUtility.TryWriteBytes(span, 10, 20, 30, 40, 50, 60)); - Assert.Equal(new byte[] { 10, 20, 30, 40, 50, 60 }, span.ToArray()); - - span = stackalloc byte[7] { 100, 101, 102, 103, 104, 105, 106 }; - Assert.True(SpanUtility.TryWriteBytes(span, 10, 20, 30, 40, 50, 60)); - Assert.Equal(new byte[] { 10, 20, 30, 40, 50, 60, 106 }, span.ToArray()); - } - - [Fact] - public void TryWriteFourChars() - { - Span span = stackalloc char[0]; - Assert.False(SpanUtility.TryWriteChars(span, 'a', 'b', 'c', 'd')); - - span = stackalloc char[3] { '0', '1', '2' }; - Assert.False(SpanUtility.TryWriteChars(span, 'a', 'b', 'c', 'd')); - Assert.Equal(new char[] { '0', '1', '2' }, span.ToArray()); - - span = stackalloc char[4] { '0', '1', '2', '3' }; - Assert.True(SpanUtility.TryWriteChars(span, 'a', 'b', 'c', 'd')); - Assert.Equal(new char[] { 'a', 'b', 'c', 'd' }, span.ToArray()); - - span = stackalloc char[5] { '0', '1', '2', '3', '4' }; - Assert.True(SpanUtility.TryWriteChars(span, 'a', 'b', 'c', 'd')); - Assert.Equal(new char[] { 'a', 'b', 'c', 'd', '4' }, span.ToArray()); - } - - [Fact] - public void TryWriteFiveChars() - { - Span span = stackalloc char[0]; - Assert.False(SpanUtility.TryWriteChars(span, 'a', 'b', 'c', 'd', 'e')); - - span = stackalloc char[4] { '0', '1', '2', '3' }; - Assert.False(SpanUtility.TryWriteChars(span, 'a', 'b', 'c', 'd', 'e')); - Assert.Equal(new char[] { '0', '1', '2', '3' }, span.ToArray()); - - span = stackalloc char[5] { '0', '1', '2', '3', '4' }; - Assert.True(SpanUtility.TryWriteChars(span, 'a', 'b', 'c', 'd', 'e')); - Assert.Equal(new char[] { 'a', 'b', 'c', 'd', 'e' }, span.ToArray()); - - span = stackalloc char[6] { '0', '1', '2', '3', '4', '5' }; - Assert.True(SpanUtility.TryWriteChars(span, 'a', 'b', 'c', 'd', 'e')); - Assert.Equal(new char[] { 'a', 'b', 'c', 'd', 'e', '5' }, span.ToArray()); - } - - [Fact] - public void TryWriteSixChars() - { - Span span = stackalloc char[0]; - Assert.False(SpanUtility.TryWriteChars(span, 'a', 'b', 'c', 'd', 'e', 'f')); - - span = stackalloc char[5] { '0', '1', '2', '3', '4' }; - Assert.False(SpanUtility.TryWriteChars(span, 'a', 'b', 'c', 'd', 'e', 'f')); - Assert.Equal(new char[] { '0', '1', '2', '3', '4' }, span.ToArray()); - - span = stackalloc char[6] { '0', '1', '2', '3', '4', '5' }; - Assert.True(SpanUtility.TryWriteChars(span, 'a', 'b', 'c', 'd', 'e', 'f')); - Assert.Equal(new char[] { 'a', 'b', 'c', 'd', 'e', 'f' }, span.ToArray()); - - span = stackalloc char[7] { '0', '1', '2', '3', '4', '5', '6' }; - Assert.True(SpanUtility.TryWriteChars(span, 'a', 'b', 'c', 'd', 'e', 'f')); - Assert.Equal(new char[] { 'a', 'b', 'c', 'd', 'e', 'f', '6' }, span.ToArray()); - } - [Theory] [InlineData(0, 0)] [InlineData(0, 1)]