From 78b34ddefd9cd25dfdbb8857ee63db6297237699 Mon Sep 17 00:00:00 2001 From: hrrrrustic Date: Mon, 12 Jul 2021 13:10:35 +0300 Subject: [PATCH 1/6] Add tests for old issue --- .../tests/StringSegmentTest.cs | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/src/libraries/Microsoft.Extensions.Primitives/tests/StringSegmentTest.cs b/src/libraries/Microsoft.Extensions.Primitives/tests/StringSegmentTest.cs index 26fac42eb8f04..e5eb6609078a6 100644 --- 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() { @@ -1265,6 +1291,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() { From 0cb330eab5d2e0d0c272ebde9156f5260c1a17eb Mon Sep 17 00:00:00 2001 From: hrrrrustic Date: Mon, 12 Jul 2021 13:55:43 +0300 Subject: [PATCH 2/6] Add test for new issue --- .../tests/StringSegmentTest.cs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/libraries/Microsoft.Extensions.Primitives/tests/StringSegmentTest.cs b/src/libraries/Microsoft.Extensions.Primitives/tests/StringSegmentTest.cs index e5eb6609078a6..aea63a8df70b2 100644 --- a/src/libraries/Microsoft.Extensions.Primitives/tests/StringSegmentTest.cs +++ b/src/libraries/Microsoft.Extensions.Primitives/tests/StringSegmentTest.cs @@ -1265,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(() => segment.IndexOfAny(new []{ '5' }, 2, 3)); + Assert.Equal("start", exception.ParamName); + } + [Fact] public void LastIndexOf_ComputesIndex_RelativeToTheCurrentSegment() { From 78f7f5b1510a1a5418c02d00428d5fe60ada0da8 Mon Sep 17 00:00:00 2001 From: hrrrrustic Date: Mon, 12 Jul 2021 13:58:08 +0300 Subject: [PATCH 3/6] fix behavior for old issue --- .../src/StringSegment.cs | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Primitives/src/StringSegment.cs b/src/libraries/Microsoft.Extensions.Primitives/src/StringSegment.cs index 69a4a5e36b8b8..a0045ff38abe5 100644 --- a/src/libraries/Microsoft.Extensions.Primitives/src/StringSegment.cs +++ b/src/libraries/Microsoft.Extensions.Primitives/src/StringSegment.cs @@ -452,22 +452,26 @@ public StringSegment Subsegment(int offset, int length) [MethodImpl(MethodImplOptions.AggressiveInlining)] public int IndexOf(char c, int start, int count) { + int index = -1; int offset = Offset + start; - if (!HasValue || start < 0 || (uint)offset > (uint)Buffer.Length) + if (HasValue) { - ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.start); - } + if (start < 0 || (uint)offset > (uint)Buffer.Length) + { + ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.start); + } - if (count < 0) - { - ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.count); - } + if (count < 0) + { + 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; From 0f43edbb332b530cda898d256e766c58fba03dc4 Mon Sep 17 00:00:00 2001 From: hrrrrustic Date: Mon, 12 Jul 2021 14:12:16 +0300 Subject: [PATCH 4/6] fix behavior for new issue --- .../Microsoft.Extensions.Primitives/src/StringSegment.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Primitives/src/StringSegment.cs b/src/libraries/Microsoft.Extensions.Primitives/src/StringSegment.cs index a0045ff38abe5..a559eae48278e 100644 --- a/src/libraries/Microsoft.Extensions.Primitives/src/StringSegment.cs +++ b/src/libraries/Microsoft.Extensions.Primitives/src/StringSegment.cs @@ -520,12 +520,12 @@ public int IndexOfAny(char[] anyOf, int startIndex, int count) if (HasValue) { - if (startIndex < 0 || Offset + startIndex > Buffer.Length) + if (startIndex < 0 || startIndex > Length) { ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.start); } - if (count < 0 || Offset + startIndex + count > Buffer.Length) + if (count < 0 || startIndex + count > Length) { ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.count); } From 1fd3d1ba5d24327c15dffdc41eb5e2580005600d Mon Sep 17 00:00:00 2001 From: hrrrrustic Date: Mon, 12 Jul 2021 14:13:00 +0300 Subject: [PATCH 5/6] Update IndexOf checks for consistency --- .../Microsoft.Extensions.Primitives/src/StringSegment.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Primitives/src/StringSegment.cs b/src/libraries/Microsoft.Extensions.Primitives/src/StringSegment.cs index a559eae48278e..11d7aee8c3b13 100644 --- a/src/libraries/Microsoft.Extensions.Primitives/src/StringSegment.cs +++ b/src/libraries/Microsoft.Extensions.Primitives/src/StringSegment.cs @@ -453,16 +453,15 @@ public StringSegment Subsegment(int offset, int length) public int IndexOf(char c, int start, int count) { int index = -1; - int offset = Offset + start; if (HasValue) { - if (start < 0 || (uint)offset > (uint)Buffer.Length) + if (start < 0 || start > Length) { ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.start); } - if (count < 0) + if (count < 0 || start + count > Length) { ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.count); } From b96959f1ad866492c0e80b84ba54a60d0f6d76d8 Mon Sep 17 00:00:00 2001 From: hrrrrustic <35951936+hrrrrustic@users.noreply.github.com> Date: Mon, 12 Jul 2021 23:33:31 +0300 Subject: [PATCH 6/6] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Günther Foidl --- .../Microsoft.Extensions.Primitives/src/StringSegment.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Primitives/src/StringSegment.cs b/src/libraries/Microsoft.Extensions.Primitives/src/StringSegment.cs index 11d7aee8c3b13..f1cd6ca6e4e72 100644 --- a/src/libraries/Microsoft.Extensions.Primitives/src/StringSegment.cs +++ b/src/libraries/Microsoft.Extensions.Primitives/src/StringSegment.cs @@ -456,12 +456,12 @@ public int IndexOf(char c, int start, int count) if (HasValue) { - if (start < 0 || start > Length) + if ((uint)start > (uint)Length) { ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.start); } - if (count < 0 || start + count > Length) + if ((uint)count > (uint)(Length - start)) { ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.count); } @@ -519,12 +519,12 @@ public int IndexOfAny(char[] anyOf, int startIndex, int count) if (HasValue) { - if (startIndex < 0 || startIndex > Length) + if ((uint)startIndex > (uint)Length) { ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.start); } - if (count < 0 || startIndex + count > Length) + if ((uint)count > (uint)(Length - startIndex)) { ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.count); }