Fix bug in char overload and other small updates

dotnet · Jun 19, 2024 · 73acb51 · 73acb51
1 parent c1c586b
commit 73acb51
Show file tree

Hide file tree

Showing 4 changed files with 66 additions and 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
@@ -443,7 +443,6 @@ private static OperationStatus DecodeWithWhiteSpaceBlockwise(ReadOnlySpan<byte>
                 }
 
                 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<byte> 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<byte> 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<char>
                     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
@@ -13,7 +13,7 @@
       <Link>System\Memory\Base64\Base64TestBase.cs</Link>
     </Compile>
     <Compile Include="..\..\System.Memory\tests\Base64\Base64TestHelper.cs">
-      <Link>System\Memory\Base64Url\Base64TestHelper.cs</Link>
+      <Link>System\Memory\Base64\Base64TestHelper.cs</Link>
     </Compile>
     <Compile Include="..\..\System.Memory\tests\Base64Url\Base64UrlDecoderUnitTests.cs">
       <Link>System\Memory\Base64Url\Base64UrlDecoderUnitTests.cs</Link>

diff --git 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<byte> 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<byte> 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
@@ -221,7 +221,17 @@ private static OperationStatus DecodeWithWhiteSpaceBlockwise<TBase64Decoder>(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.