Skip to content

Commit

Permalink
Add span overloads to GraphicsPathIterator (#11594)
Browse files Browse the repository at this point in the history
* Add span overloads to GraphicsPathIterator

Related to #8954.

This also adds more aggressive input validation to avoid buffer overruns.

* Validate end index is greater than or equal to start index.
  • Loading branch information
JeremyKuhne committed Jun 27, 2024
1 parent e1cd1ba commit b3b877e
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 8 deletions.
14 changes: 14 additions & 0 deletions src/System.Drawing.Common/src/CompatibilitySuppressions.xml
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,20 @@
<Right>lib/net8.0/System.Drawing.Common.dll</Right>
<IsBaselineSuppression>true</IsBaselineSuppression>
</Suppression>
<Suppression>
<DiagnosticId>CP0014</DiagnosticId>
<Target>T:System.Drawing.Drawing2D.GraphicsPathIterator:[T:System.Runtime.CompilerServices.NullableAttribute]</Target>
<Left>lib/net8.0/System.Drawing.Common.dll</Left>
<Right>lib/net8.0/System.Drawing.Common.dll</Right>
<IsBaselineSuppression>true</IsBaselineSuppression>
</Suppression>
<Suppression>
<DiagnosticId>CP0014</DiagnosticId>
<Target>T:System.Drawing.Drawing2D.GraphicsPathIterator:[T:System.Runtime.CompilerServices.NullableContextAttribute]</Target>
<Left>lib/net8.0/System.Drawing.Common.dll</Left>
<Right>lib/net8.0/System.Drawing.Common.dll</Right>
<IsBaselineSuppression>true</IsBaselineSuppression>
</Suppression>
<Suppression>
<DiagnosticId>CP0014</DiagnosticId>
<Target>T:System.Drawing.Drawing2D.Matrix:[T:System.Runtime.CompilerServices.NullableAttribute]</Target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,13 +147,28 @@ public void Rewind()
GC.KeepAlive(this);
}

/// <inheritdoc cref="CopyData(ref PointF[], ref byte[], int, int)"/>
public unsafe int Enumerate(ref PointF[] points, ref byte[] types)
=> Enumerate(points.OrThrowIfNull().AsSpan(), types.OrThrowIfNull().AsSpan());

/// <inheritdoc cref="CopyData(ref PointF[], ref byte[], int, int)"/>
#if NET9_0_OR_GREATER
public
#else
private
#endif
unsafe int Enumerate(Span<PointF> points, Span<byte> types)
{
if (points.Length != types.Length)
if (points.Length != types.Length
|| points.Length < Count)
{
throw Status.InvalidParameter.GetException();
}

if (points.Length == 0)
{
return 0;
}

fixed (PointF* p = points)
fixed (byte* t = types)
Expand All @@ -171,10 +186,37 @@ public unsafe int Enumerate(ref PointF[] points, ref byte[] types)
}
}

/// <summary>
/// Copies the <see cref="GraphicsPath.PathPoints"/> property and <see cref="GraphicsPath.PathTypes"/> property data
/// of the associated <see cref="GraphicsPath"/>.
/// </summary>
/// <param name="points">Upon return, contains <see cref="PointF"/> structures that represent the points in the path.</param>
/// <param name="types">Upon return, contains bytes that represent the types of points in the path.</param>
/// <param name="startIndex">The index of the first point to copy.</param>
/// <param name="endIndex">The index of the last point to copy.</param>
/// <returns>The number of points copied.</returns>
public unsafe int CopyData(ref PointF[] points, ref byte[] types, int startIndex, int endIndex)
=> CopyData(points.OrThrowIfNull().AsSpan(), types.OrThrowIfNull().AsSpan(), startIndex, endIndex);

/// <inheritdoc cref="CopyData(ref PointF[], ref byte[], int, int)"/>
#if NET9_0_OR_GREATER
public
#else
private
#endif
unsafe int CopyData(Span<PointF> points, Span<byte> types, int startIndex, int endIndex)
{
if ((points.Length != types.Length) || (endIndex - startIndex + 1 > points.Length))
int count = endIndex - startIndex + 1;

if ((points.Length != types.Length)
|| endIndex < 0
|| startIndex < 0
|| endIndex < startIndex
|| count > points.Length
|| endIndex >= Count)
{
throw Status.InvalidParameter.GetException();
}

fixed (PointF* p = points)
fixed (byte* t = types)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,19 @@ public void Enumerate_PointsTypesMismatch_ThrowsArgumentException(PointF[] point
AssertExtensions.Throws<ArgumentException>(null, () => gpi.Enumerate(ref points, ref types));
}

[Fact]
public void Enumerate_NotEnoughSpace_ThrowsArgumentException()
{
using GraphicsPath gp = new();
gp.AddLine(new(0, 0), new(1, 1));
using GraphicsPathIterator gpi = new(gp);

PointF[] points = [];
byte[] types = [];

AssertExtensions.Throws<ArgumentException>(null, () => gpi.Enumerate(ref points, ref types));
}

public static IEnumerable<object[]> NullPointsTypes_TestData()
{
yield return new object[] { null, new byte[1] };
Expand All @@ -273,11 +286,11 @@ public static IEnumerable<object[]> NullPointsTypes_TestData()

[Theory]
[MemberData(nameof(NullPointsTypes_TestData))]
public void Enumerate_NullPointsTypes_ThrowsNullReferenceException(PointF[] points, byte[] types)
public void Enumerate_NullPointsTypes_ThrowsArgumentNullException(PointF[] points, byte[] types)
{
using GraphicsPath gp = new();
using GraphicsPathIterator gpi = new(gp);
Assert.Throws<NullReferenceException>(() => gpi.Enumerate(ref points, ref types));
Assert.Throws<ArgumentNullException>(() => gpi.Enumerate(ref points, ref types));
}

[Theory]
Expand All @@ -291,11 +304,11 @@ public void CopyData_PointsTypesMismatch_ThrowsArgumentException(PointF[] points

[Theory]
[MemberData(nameof(NullPointsTypes_TestData))]
public void CopyData_NullPointsTypes_ThrowsNullReferenceException(PointF[] points, byte[] types)
public void CopyData_NullPointsTypes_ThrowsArgumentNullException(PointF[] points, byte[] types)
{
using GraphicsPath gp = new();
using GraphicsPathIterator gpi = new(gp);
Assert.Throws<NullReferenceException>(() => gpi.CopyData(ref points, ref types, 0, 1));
Assert.Throws<ArgumentNullException>(() => gpi.CopyData(ref points, ref types, 0, 1));
}

[Theory]
Expand All @@ -320,14 +333,14 @@ public static IEnumerable<object[]> CopyData_StartEndIndexesOutOfRange_TestData(

[Theory]
[MemberData(nameof(CopyData_StartEndIndexesOutOfRange_TestData))]
public void CopyData_StartEndIndexesOutOfRange_ReturnsExpected(PointF[] points, byte[] types, int startIndex, int endIndex)
public void CopyData_WithData_StartEndIndexesOutOfRange_ThrowsArgumentException(PointF[] points, byte[] types, int startIndex, int endIndex)
{
PointF[] resultPoints = new PointF[points.Length];
byte[] resultTypes = new byte[points.Length];

using GraphicsPath gp = new(points, types);
using GraphicsPathIterator gpi = new(gp);
Assert.Equal(0, gpi.CopyData(ref resultPoints, ref resultTypes, startIndex, endIndex));
AssertExtensions.Throws<ArgumentException>(null, () => gpi.CopyData(ref resultPoints, ref resultTypes, startIndex, endIndex));
}

[Fact]
Expand Down

0 comments on commit b3b877e

Please sign in to comment.