From 73acb51861f24f32ec1f2b3d2007486ca3ebbd42 Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Wed, 19 Jun 2024 11:33:13 -0700 Subject: [PATCH] Fix bug in char overload and other small updates --- .../System/Buffers/Text/Base64UrlDecoder.cs | 28 ++++++------- .../tests/Microsoft.Bcl.Memory.Tests.csproj | 2 +- .../Base64Url/Base64UrlDecoderUnitTests.cs | 42 +++++++++++++++++++ .../Text/Base64Url/Base64UrlDecoder.cs | 12 +++++- 4 files changed, 66 insertions(+), 18 deletions(-) diff --git a/src/libraries/Microsoft.Bcl.Memory/src/System/Buffers/Text/Base64UrlDecoder.cs b/src/libraries/Microsoft.Bcl.Memory/src/System/Buffers/Text/Base64UrlDecoder.cs index 51c7c651b50f38..ff9440880a9117 100644 --- a/src/libraries/Microsoft.Bcl.Memory/src/System/Buffers/Text/Base64UrlDecoder.cs +++ b/src/libraries/Microsoft.Bcl.Memory/src/System/Buffers/Text/Base64UrlDecoder.cs @@ -443,7 +443,6 @@ private static OperationStatus DecodeWithWhiteSpaceBlockwise(ReadOnlySpan } bool hasAnotherBlock = source.Length > 1; - bool localIsFinalBlock = !hasAnotherBlock; // If this block contains padding and there's another block, then only whitespace may follow for being valid. @@ -559,7 +558,7 @@ internal static unsafe OperationStatus DecodeFromUtf8InPlace(Span buffer, uint sourceIndex = 0; uint destIndex = 0; - if ((bufferLength & 3) == 1) // One byte cannot be decoded completely) + if ((bufferLength & 3) == 1) // One byte cannot be decoded completely { goto InvalidExit; } @@ -703,21 +702,18 @@ private static OperationStatus DecodeWithWhiteSpaceFromUtf8InPlace(Span so continue; } - if (bufferIdx != 4) + // For Base64Url 1 byte is not decodeable. + if (bufferIdx == 1) { - // For Base64Url 1 byte is not decodeable. - if (bufferIdx == 1) - { - status = OperationStatus.InvalidData; - break; - } - else // Fill empty slots in last block with padding + status = OperationStatus.InvalidData; + break; + } + else // Fill empty slots in last block with padding + { + while (bufferIdx < BlockSize) // Can happen only for last block { - while (bufferIdx < BlockSize) // Can happen only for last block - { - Debug.Assert(source.Length == sourceIndex); - buffer[bufferIdx++] = (byte)EncodingPadEqual; - } + Debug.Assert(source.Length == sourceIndex); + buffer[bufferIdx++] = (byte)EncodingPadEqual; } } @@ -1197,7 +1193,7 @@ private static OperationStatus DecodeWithWhiteSpaceBlockwise(ReadOnlySpan continue; } - bool hasAnotherBlock = source.Length >= BlockSize && bufferIdx == BlockSize; + bool hasAnotherBlock = source.Length > 1; bool localIsFinalBlock = !hasAnotherBlock; // If this block contains padding and there's another block, then only whitespace may follow for being valid. diff --git a/src/libraries/Microsoft.Bcl.Memory/tests/Microsoft.Bcl.Memory.Tests.csproj b/src/libraries/Microsoft.Bcl.Memory/tests/Microsoft.Bcl.Memory.Tests.csproj index c012b0b1b34d15..f3f7262755c19d 100644 --- a/src/libraries/Microsoft.Bcl.Memory/tests/Microsoft.Bcl.Memory.Tests.csproj +++ b/src/libraries/Microsoft.Bcl.Memory/tests/Microsoft.Bcl.Memory.Tests.csproj @@ -13,7 +13,7 @@ System\Memory\Base64\Base64TestBase.cs - System\Memory\Base64Url\Base64TestHelper.cs + System\Memory\Base64\Base64TestHelper.cs System\Memory\Base64Url\Base64UrlDecoderUnitTests.cs diff --git a/src/libraries/System.Memory/tests/Base64Url/Base64UrlDecoderUnitTests.cs b/src/libraries/System.Memory/tests/Base64Url/Base64UrlDecoderUnitTests.cs index 515bff7d761d85..1a321a7ba9544b 100644 --- a/src/libraries/System.Memory/tests/Base64Url/Base64UrlDecoderUnitTests.cs +++ b/src/libraries/System.Memory/tests/Base64Url/Base64UrlDecoderUnitTests.cs @@ -801,6 +801,29 @@ public void DecodingLessThan4BytesWithWhiteSpaces(byte[] utf8Bytes, byte decoded Assert.Equal(decoded, utf8Bytes[0]); } + [Theory] + [InlineData(new char[] { '\r', '\r', '-', '-' }, 251)] + [InlineData(new char[] { '\r', '_', '\r', '-' }, 255)] + [InlineData(new char[] { '_', '_', '\r', '\r' }, 255)] + [InlineData(new char[] { 'p', '\r', 'a', '\r' }, 165)] + [InlineData(new char[] { '\r', 'p', '\r', 'a', '\r' }, 165)] + [InlineData(new char[] { 'p', '\r', 'a', '\r', '=', '=' }, 165)] + public void DecodingLessThan4CharsWithWhiteSpaces(char[] utf8Bytes, byte decoded) + { + Assert.True(Base64Url.IsValid(utf8Bytes, out int decodedLength)); + Assert.Equal(1, decodedLength); + Span decodedSpan = new byte[decodedLength]; + OperationStatus status = Base64Url.DecodeFromChars(utf8Bytes, decodedSpan, out int bytesRead, out int bytesDecoded); + Assert.Equal(OperationStatus.Done, status); + Assert.Equal(utf8Bytes.Length, bytesRead); + Assert.Equal(decodedLength, bytesDecoded); + Assert.Equal(decoded, decodedSpan[0]); + decodedSpan.Clear(); + Assert.True(Base64Url.TryDecodeFromChars(utf8Bytes, decodedSpan, out bytesDecoded)); + Assert.Equal(decodedLength, bytesDecoded); + Assert.Equal(decoded, decodedSpan[0]); + } + [Theory] [InlineData(new byte[] { 0x4a, 0x74, 0xa, 0x4a, 0x4a, 0x74, 0xa, 0x4a }, new byte[] { 38, 210, 73, 180 })] [InlineData(new byte[] { 0xa, 0x2d, 0x56, 0xa, 0xa, 0xa, 0x2d, 0x4a, 0x4a, 0x4a, }, new byte[] { 249, 95, 137, 36 })] @@ -823,6 +846,25 @@ public void DecodingNotMultipleOf4WithWhiteSpace(byte[] utf8Bytes, byte[] decode Assert.True(utf8Bytes.AsSpan().Slice(0, bytesDecoded).SequenceEqual(decoded)); } + [Theory] + [InlineData(new char[] { 'J', 't', '\r', 'J', 'J', 't', '\r', 'J' }, new byte[] { 38, 210, 73, 180 })] + [InlineData(new char[] { '\r', '-', 'V', '\r', '\r', '\r', '-', 'J', 'J', 'J', }, new byte[] { 249, 95, 137, 36 })] + public void DecodingNotMultipleOf4CharsWithWhiteSpace(char[] utf8Bytes, byte[] decoded) + { + Assert.True(Base64Url.IsValid(utf8Bytes, out int decodedLength)); + Assert.Equal(4, decodedLength); + Span decodedSpan = new byte[decodedLength]; + OperationStatus status = Base64Url.DecodeFromChars(utf8Bytes, decodedSpan, out int bytesRead, out int bytesDecoded); + Assert.Equal(OperationStatus.Done, status); + Assert.Equal(utf8Bytes.Length, bytesRead); + Assert.Equal(decodedLength, bytesDecoded); + Assert.True(decodedSpan.SequenceEqual(decoded)); + decodedSpan.Clear(); + Assert.True(Base64Url.TryDecodeFromChars(utf8Bytes, decodedSpan, out bytesDecoded)); + Assert.Equal(decodedLength, bytesDecoded); + Assert.True(decodedSpan.SequenceEqual(decoded)); + } + [Theory] [MemberData(nameof(BasicDecodingWithExtraWhitespaceShouldBeCountedInConsumedBytes_MemberData))] public void BasicDecodingWithExtraWhitespaceShouldBeCountedInConsumedBytes(string inputString, int expectedConsumed, int expectedWritten) diff --git a/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Base64Url/Base64UrlDecoder.cs b/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Base64Url/Base64UrlDecoder.cs index 6e7d97f79426c5..937a88d5f6f08f 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Base64Url/Base64UrlDecoder.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Base64Url/Base64UrlDecoder.cs @@ -221,7 +221,17 @@ private static OperationStatus DecodeWithWhiteSpaceBlockwise(Rea continue; } - bool hasAnotherBlock = source.Length >= BlockSize && bufferIdx == BlockSize; + bool hasAnotherBlock; + + if (typeof(TBase64Decoder) == typeof(Base64DecoderByte)) + { + hasAnotherBlock = source.Length >= BlockSize; + } + else + { + hasAnotherBlock = source.Length > 1; + } + bool localIsFinalBlock = !hasAnotherBlock; // If this block contains padding and there's another block, then only whitespace may follow for being valid.