Add a length boundary to decode method

Author: troydai

src/Microsoft.Extensions.WebEncoders.Core/UrlPathDecoder.cs

@@ -30,18 +30,39 @@ public class UrlPathDecoder
         /// </param>
         /// <returns>The length of the result.</returns>
         public static int DecodeInPlace(char[] buffer)
+        {
+            return DecodeInPlace(buffer, buffer.Length);
+        }
+
+        /// <summary>
+        /// Unescape a url char array in place. Returns the length of the result.
+        /// 
+        /// - Everything is unescaped except %2F ('/')
+        /// - UTF8 bytes are tested for formatting, overlong encoding, surrogates and value ranges
+        /// - Invalid escaped sequence are copied to output as is
+        /// - It doesn't check if the string contains query
+        /// </summary>
+        /// <param name="buffer">
+        /// The char array contains sequence of charactors to be decoded. The
+        /// result will be saved in the same array.
+        /// </param>
+        /// <param name="len">
+        /// The lenght of the sequence of characters in buffer to be decoded.
+        /// </param>
+        /// <returns>The length of the result.</returns>
+        public static int DecodeInPlace(char[] buffer, int len)
         {
             if (buffer == null)
             {
                 throw new ArgumentNullException(nameof(buffer));
             }
 
-            if (GetNextEncoded(0, buffer) == buffer.Length)
+            if (GetNextEncoded(buffer, 0, len) == len)
             {
-                return buffer.Length;
+                return len;
             }
 
-            return DecodeCore(buffer, buffer);
+            return DecodeCore(buffer, len);
         }
 
         /// <summary>
@@ -69,60 +90,72 @@ public static string Decode(string original)
             var buffer = original.ToCharArray();
 
             // decode in place
-            var len = DecodeCore(buffer, buffer);
+            var len = DecodeCore(buffer, buffer.Length);
             return new string(buffer, 0, len);
         }
 
         /// <summary>
-        /// Unescape a url path string.
-        /// 
-        /// - The result is saved in place
+        /// Decode the sequence of charactors in give char array.
         /// </summary>
-        private static int DecodeCore(char[] source, char[] destination)
+        /// <param name="buffer">
+        /// The array of characters to be decoded. It's both the source and output of the
+        /// operation which means the decode happens in place.
+        /// </param>
+        /// <param name="length">
+        /// The length of the source sequence in the <paramref name="buffer"/> array to be decoded.
+        /// </param>
+        /// <returns>
+        /// The length of the result.
+        /// </returns>
+        private static int DecodeCore(char[] buffer, int length)
         {
-            var bufferPosition = 0;
-            var sourcePosition = 0;
+            // two indices to read and write
+            var readerPosition = 0;
+            var writerPosition = 0;
 
+            // operating buffer
             var unescapedChars = new char[1];
             var unescapedCharsCount = 0;
             var bytesBuffer = new byte[4];
 
-            while (sourcePosition < source.Length)
+            while (readerPosition < length)
             {
-                var next = GetNextEncoded(sourcePosition, source);
-                var copyLength = next - sourcePosition;
-                Copy(source, ref sourcePosition, destination, ref bufferPosition, copyLength);
+                var next = GetNextEncoded(buffer, readerPosition, length);
+                var copyLength = next - readerPosition;
+
+                CopyInPlace(buffer, length, copyLength, ref readerPosition, ref writerPosition);
 
-                if (sourcePosition >= source.Length)
+                if (readerPosition >= length)
                 {
                     break;
                 }
 
-                var consumed = Unescape(source, next, bytesBuffer, ref unescapedChars, ref unescapedCharsCount);
+                var consumed = Unescape(buffer, length, next, bytesBuffer, ref unescapedChars, ref unescapedCharsCount);
                 if (consumed == 0)
                 {
                     // Skip unescaping the % as the sequence follows it can't be correctly
                     // decoded under UTF8
-                    Copy(source, ref sourcePosition, destination, ref bufferPosition, 1);
+                    CopyInPlace(buffer, length, 1, ref readerPosition, ref writerPosition);
                 }
                 else if (unescapedCharsCount == 1 && SkipUnescape(unescapedChars[0]))
                 {
                     // Skip unescaping specified characters (eg. '/')
                     // Copy the original sequence to destination
-                    Copy(source, ref sourcePosition, destination, ref bufferPosition, consumed);
+                    CopyInPlace(buffer, length, consumed, ref readerPosition, ref writerPosition);
                 }
                 else
                 {
                     // Copy unescaped chararter. Move to the next charactor in source.
                     for (int i = 0; i < unescapedCharsCount; ++i)
                     {
-                        destination[bufferPosition++] = unescapedChars[i];
+                        buffer[writerPosition++] = unescapedChars[i];
                     }
-                    sourcePosition += consumed;
+
+                    readerPosition += consumed;
                 }
             }
 
-            return bufferPosition;
+            return writerPosition;
         }
 
         private static bool SkipUnescape(char charactor)
@@ -144,9 +177,9 @@ private static bool SkipUnescape(char charactor)
         /// - The length of the sequence, including the % charactor, will be returned.
         /// Otherwise 0 is returned.
         /// </summary>
-        private static int Unescape(char[] source, int start, byte[] bytesBuffer, ref char[] output, ref int count)
+        private static int Unescape(char[] source, int sourceBoundary, int start, byte[] bytesBuffer, ref char[] output, ref int count)
         {
-            if (start + 2 >= source.Length)
+            if (start + 2 >= sourceBoundary)
             {
                 return 0;
             }
@@ -205,7 +238,7 @@ private static int Unescape(char[] source, int start, byte[] bytesBuffer, ref ch
                 return 0;
             }
 
-            if (start + (bytesCount * 3) > source.Length)
+            if (start + (bytesCount * 3) > sourceBoundary)
             {
                 // less than expected bytes to decode
                 return 0;
@@ -274,39 +307,88 @@ private static int Unescape(char[] source, int start, byte[] bytesBuffer, ref ch
         }
 
         /// <summary>
-        /// Copy characters in source string to target char array. After copied the indices points at the beginning
-        /// offset of both source and destination are updated to point to the position after the last copied character
+        /// Copy characters in an array in place.
         /// </summary>
-        private static int Copy(char[] source, ref int sourceStart, char[] destination, ref int destinatonStart, int length)
+        /// <param name="buffer">
+        /// The array containing the characters to be copied. It is both the 
+        /// source and output of the operation.
+        /// </param>
+        /// <param name="sourceLength">
+        /// The length of the source sequence
+        /// </param>
+        /// <param name="count">
+        /// The count of the charaters to be copied in the <paramref name="buffer"/>
+        /// </param>
+        /// <param name="readerPosition">
+        /// The index where characters are copied from. The parameter will be
+        /// set to the position right behind the last read character after copy.
+        /// </param>
+        /// <param name="writerPosition">
+        /// The index where characters are copied to. The parameter will be set to
+        /// the position right behind the last written character adter copy.
+        /// </param>
+        /// <returns>The number of charactors actually be copied.</returns>
+        private static int CopyInPlace(char[] buffer, int sourceLength, int count, ref int readerPosition, ref int writerPosition)
         {
-            for (var i = 0; i < length; ++i)
+            if (buffer == null)
+            {
+                throw new ArgumentNullException(nameof(buffer));
+            }
+
+            if (sourceLength > buffer.Length)
+            {
+                throw new ArgumentOutOfRangeException(nameof(sourceLength), $"The length of the source sequence can't be longer than the size of the buffer.");
+            }
+
+            if (readerPosition < 0 || readerPosition >= sourceLength)
+            {
+                throw new ArgumentOutOfRangeException(nameof(readerPosition), $"The index of the source sequence {readerPosition} is out of range.");
+            }
+
+            if (writerPosition < 0 || writerPosition >= buffer.Length)
+            {
+                throw new ArgumentOutOfRangeException(nameof(writerPosition), $"The index of the output sequence {writerPosition} is out of range.");
+            }
+
+            if (writerPosition > readerPosition)
+            {
+                throw new ArgumentException($"The index of output sequence {writerPosition} is behind the read sequence {readerPosition}.");
+            }
+
+            for (var i = 0; i < count; ++i)
             {
-                destination[destinatonStart++] = source[sourceStart++];
+                buffer[writerPosition++] = buffer[readerPosition++];
 
-                if (destinatonStart >= destination.Length ||
-                    sourceStart >= source.Length)
+                // when reader pointer surpass the boundary of the source sequence; or
+                //      writer pointer surpass the boundary of the buffer
+                // return the count of the copied charcters
+                if (writerPosition >= buffer.Length || readerPosition >= sourceLength)
                 {
                     return i + 1;
                 }
             }
 
-            return length;
+            return count;
         }
 
         /// <summary>
-        /// Find the next % in the sequence. If % is not found, return the sequence length.
+        /// Find the next % in the sequence of range [start, end)
         /// </summary>
-        private static int GetNextEncoded(int start, char[] array)
+        /// <param name="buffer">The array of character in which the % is seacrhed.</param>
+        /// <param name="start">The start of the search range.</param>
+        /// <param name="end">The end of the search range.</param>
+        /// <returns>The index of the first %, or <paramref name="end"/> if % is not found.</returns>
+        private static int GetNextEncoded(char[] buffer, int start, int end)
         {
-            for (var i = start; i < array.Length; ++i)
+            for (var i = start; i < end; ++i)
             {
-                if (array[i] == '%')
+                if (buffer[i] == '%')
                 {
                     return i;
                 }
             }
 
-            return array.Length;
+            return end;
         }
 
         private static bool TryGetUnescapedByte(char[] buffer, int position, out byte result)

test/Microsoft.Extensions.WebEncoders.Tests/UrlPathDecoderTests.cs

@@ -124,7 +124,7 @@ protected override void PositiveAssert(string raw, string expect)
         }
     }
 
-    public class UrlPathInPlaceDecoderTests: UrlPathDecoderTestBase
+    public class UrlPathInPlaceDecoderTests : UrlPathDecoderTestBase
     {
         protected override void PositiveAssert(string raw, string expect)
         {
@@ -154,5 +154,27 @@ protected override void NegativeAssert(string raw)
             Assert.Equal(raw.Length, len);
             Assert.Equal(raw.ToCharArray(), buf);
         }
+
+        [Theory]
+        [InlineData("/foo%2Fbar", 10, "/foo%2Fbar", 10)]
+        [InlineData("/foo%2Fbar", 9, "/foo%2Fba", 9)]
+        [InlineData("/foo%2Fbar", 8, "/foo%2Fb", 8)]
+        [InlineData("%D0%A4", 6, "Ф", 1)]
+        [InlineData("%D0%A4", 5, "%D0%A", 5)]
+        [InlineData("%D0%A4", 4, "%D0%", 4)]
+        [InlineData("%D0%A4", 3, "%D0", 3)]
+        [InlineData("%D0%A4", 2, "%D", 2)]
+        [InlineData("%D0%A4", 1, "%", 1)]
+        [InlineData("%D0%A4", 0, "", 0)]
+        [InlineData("%C2%B5%40%C3%9F%C3%B6%C3%A4%C3%BC%C3%A0%C3%A1", 45, "µ@ßöäüàá", 8)]
+        [InlineData("%C2%B5%40%C3%9F%C3%B6%C3%A4%C3%BC%C3%A0%C3%A1", 44, "µ@ßöäüà%C3%A", 12)]
+        public void DecodeWithBoundary(string raw, int rawLength, string expect, int expectLength)
+        {
+            var buf = raw.ToCharArray();
+            var len = UrlPathDecoder.DecodeInPlace(buf, rawLength);
+
+            Assert.Equal(expectLength, len);
+            Assert.Equal(expect.ToCharArray(), new ArraySegment<char>(buf, 0, expectLength));
+        }
     }
 }