StringSegment behavior (#55501)

* Add tests for old issue * Add test for new issue * fix behavior for old issue * fix behavior for new issue * Update IndexOf checks for consistency Co-authored-by: Günther Foidl <gue@korporal.at> Co-authored-by: Günther Foidl <gue@korporal.at>
dotnet · Jul 15, 2021 · 188c270 · 188c270
1 parent bcc7944
commit 188c270
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 14 deletions.
diff --git a/src/libraries/Microsoft.Extensions.Primitives/src/StringSegment.cs b/src/libraries/Microsoft.Extensions.Primitives/src/StringSegment.cs
@@ -452,22 +452,25 @@ public StringSegment Subsegment(int offset, int length)
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public int IndexOf(char c, int start, int count)
         {
-            int offset = Offset + start;
+            int index = -1;
 
-            if (!HasValue || start < 0 || (uint)offset > (uint)Buffer.Length)
+            if (HasValue)
             {
-                ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.start);
-            }
+                if ((uint)start > (uint)Length)
+                {
+                    ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.start);
+                }
 
-            if (count < 0)
-            {
-                ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.count);
-            }
+                if ((uint)count > (uint)(Length - start))
+                {
+                    ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.count);
+                }
 
-            int index = AsSpan().Slice(start, count).IndexOf(c);
-            if (index >= 0)
-            {
-                index += start;
+                index = AsSpan(start, count).IndexOf(c);
+                if (index >= 0)
+                {
+                    index += start;
+                }
             }
 
             return index;
@@ -516,12 +519,12 @@ public int IndexOfAny(char[] anyOf, int startIndex, int count)
 
             if (HasValue)
             {
-                if (startIndex < 0 || Offset + startIndex > Buffer.Length)
+                if ((uint)startIndex > (uint)Length)
                 {
                     ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.start);
                 }
 
-                if (count < 0 || Offset + startIndex + count > Buffer.Length)
+                if ((uint)count > (uint)(Length - startIndex))
                 {
                     ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.count);
                 }

diff --git a/src/libraries/Microsoft.Extensions.Primitives/tests/StringSegmentTest.cs b/src/libraries/Microsoft.Extensions.Primitives/tests/StringSegmentTest.cs
@@ -1134,6 +1134,19 @@ public void IndexOf_ReturnsMinusOne_IfElementNotInSegment()
             Assert.Equal(-1, result);
         }
 
+        [Fact]
+        public void IndexOf_ReturnsMinusOne_OnDefaultStringSegment()
+        {
+            // Arrange
+            StringSegment segment = default;
+
+            // Act
+            int result = segment.IndexOf(',');
+
+            // Assert
+            Assert.Equal(-1, result);
+        }
+
         [Fact]
         public void IndexOf_SkipsANumberOfCaracters_IfStartIsProvided()
         {
@@ -1211,6 +1224,19 @@ public void IndexOfAny_ReturnsMinusOne_IfElementNotInSegment()
             Assert.Equal(-1, result);
         }
 
+        [Fact]
+        public void IndexOfAny_ReturnsMinusOne_OnDefaultStringSegment()
+        {
+            // Arrange
+            StringSegment segment = default;
+
+            // Act
+            int result = segment.IndexOfAny(new[] { ',' });
+
+            // Assert
+            Assert.Equal(-1, result);
+        }
+
         [Fact]
         public void IndexOfAny_SkipsANumberOfCaracters_IfStartIsProvided()
         {
@@ -1239,6 +1265,17 @@ public void IndexOfAny_SearchOnlyInsideTheRange_IfStartAndCountAreProvided()
             Assert.Equal(-1, result);
         }
 
+        [Fact]
+        public void IndexOfAny_StartOverflowsWithOffset_OutOfRangeThrows()
+        {
+            // Arrange
+            StringSegment segment = new StringSegment("12345", 0, 1);
+
+            // Act & Assert
+            ArgumentOutOfRangeException exception = Assert.Throws<ArgumentOutOfRangeException>(() => segment.IndexOfAny(new []{ '5' }, 2, 3));
+            Assert.Equal("start", exception.ParamName);
+        }
+
         [Fact]
         public void LastIndexOf_ComputesIndex_RelativeToTheCurrentSegment()
         {
@@ -1265,6 +1302,19 @@ public void LastIndexOf_ReturnsMinusOne_IfElementNotInSegment()
             Assert.Equal(-1, result);
         }
 
+        [Fact]
+        public void LastIndexOf_ReturnsMinusOne_OnDefaultStringSegment()
+        {
+            // Arrange
+            StringSegment segment = default;
+
+            // Act
+            int result = segment.LastIndexOf(',');
+
+            // Assert
+            Assert.Equal(-1, result);
+        }
+
         [Fact]
         public void Value_DoesNotAllocateANewString_IfTheSegmentContainsTheWholeBuffer()
         {