empira / PDFsharp-1.5

A .NET library for processing PDF
MIT License
1.28k stars 588 forks source link

Check is needed for GDI+ implementation of AppendPath method of PdfSharp.Drawing.Pdf.XGraphicsPdfRenderer class #97

Closed MikeGratsas closed 1 year ago

MikeGratsas commented 5 years ago

Method internal void AppendPath(GraphicsPath path) was implemented in PdfSharp v. 1.32 as follows:

            int count = path.PointCount;
            if (count == 0)
                return;
            ...

However, this additional path.PointCount check is omitted in the current version:

AppendPath(XGraphics.MakeXPointArray(path.PathPoints, 0, path.PathPoints.Length), path.PathTypes);

Therefore, if GraphicsPath is empty and doesn't contain any objects (due to invalid parameters or another reason) an exception is thrown by GDI+ reading properties path.PathPoints and path.PathTypes. In my opinion, the mentioned path.PointCount; check is still needed to prevent exceptions.

ThomasHoevel commented 1 year ago

In my opinion, the check is still there. Closed until we have code that reproduces a real issue.

MikeGratsas commented 1 year ago

I attached the solution representing the issue upgrading to PdfSharp library version 1.50. If the project references PdfSharp v. 1.32, the program execution completes successfully. However, if you change the reference to PdfSharp library version to 1.50, as implemented in the project PdfSharpApplication-1.50.5147.csproj, you will get an unhandled exception. I wonder why four years are required to fix the mentioned issue. PdfSharpApplication.zip

ThomasHoevel commented 1 year ago

I tried your code. No exception with 1.32, no exception with 1.50.5147.
Wasted enough time, back to working on PDFsharp 6.0.0.

Floyddotnet commented 1 year ago

I tried your code. No exception with 1.32, no exception with 1.50.5147. Wasted enough time, back to working on PDFsharp 6.0.0.

Hi thomas, I have not been involved in this ticket before but it caught my attention.

I tried the example project "1.50.5147" and I get an ArgumentExecption.

Stacktrace:

   at System.Drawing.Drawing2D.GraphicsPath.get_PathPoints()
   at PdfSharp.Drawing.Pdf.XGraphicsPdfRenderer.AppendPath(GraphicsPath path)
   at PdfSharp.Drawing.Pdf.XGraphicsPdfRenderer.DrawPath(XPen pen, XBrush brush, XGraphicsPath path)
   at PdfSharp.Drawing.XGraphics.DrawPath(XBrush brush, XGraphicsPath path)
   at PdfSharpApplication.Program.Main(String[] args) in C:\Users\xxx\Downloads\PdfSharpApplication\PdfSharpApplication\Program.cs:line 16

Env:

Windows 10 Pro
Visual Studio 2022 64 Bit (17.4.4)

Also, I could not see any obvious error in the project (apart from the fact that the path does not contain any points 😁)

ThomasHoevel commented 1 year ago

Windows 10 here. Tried VS 2017 before. Now I tried VS 17.5.4. Still no exception. I hadn't seen the "hidden" project that is not included in the solution, but still no exception with that.

And this is the code I see after pressing F12 a few times:

    /// <summary>Appends the content of a GraphicsPath object.</summary>
    internal void AppendPath(CoreGraphicsPath path) => this.AppendPath(path.PathPoints, path.PathTypes);

    private void AppendPath(XPoint[] points, byte[] types)
    {
      int length = points.Length;
      if (length == 0)
        return;
      for (int index1 = 0; index1 < length; ++index1)
Floyddotnet commented 1 year ago

correct, accessing path.PathPoints throws the error. internal void AppendPath(CoreGraphicsPath path) => this.AppendPath(/* ---->*/ path.PathPoints /* <----*/, path.PathTypes);

System.ArgumentException: "Ungültiger Parameter." = System.ArgumentException: "Invalid Parameter."

see Stacktrace:

at System.Drawing.Drawing2D.GraphicsPath.get_PathPoints() = get method of PathPoints property.

Floyddotnet commented 1 year ago

If you want i can debug this at the next days (maybe at Thusday) and give you a full investigation report.

Floyddotnet commented 1 year ago

btw: Maybe you disable "Break on Exception" for "ArgumentExceptions"?

ThomasHoevel commented 1 year ago

btw: Maybe you disable "Break on Exception" for "ArgumentExceptions"?

No, all exceptions are active. And unhandled exceptions will always throw anyway.

Please let me know if you can replicate this.

MikeGratsas commented 1 year ago

You should use .NET Framework, not .NET Core to find an exception.

ThomasHoevel commented 1 year ago

OK, I can replicate the exception now.

TH-Soft commented 1 year ago

The issue will be fixed with PDFsharp 6.0.0-Preview-2 (coming soon now). Those who want to stick to version 1.50 or 1.51 can make the changes themselves.

The old code (line 1333):

#if GDI
        /// <summary>
        /// Appends the content of a GraphicsPath object.
        /// </summary>
        internal void AppendPath(GraphicsPath path)
        {
#if true
            AppendPath(XGraphics.MakeXPointArray(path.PathPoints, 0, path.PathPoints.Length), path.PathTypes);
#else

The new code:

#if GDI
        /// <summary>
        /// Appends the content of a GraphicsPath object.
        /// </summary>
        internal void AppendPath(GraphicsPath path)
        {
#if true
            int count = path.PointCount;
            if (count == 0)
                return; // Nothing to do if list is empty.
            var points = path.PathPoints;
            AppendPath(XGraphics.MakeXPointArray(points, 0, points.Length), path.PathTypes);
#else

If you don't want to change the PDFsharp package, make sure you do not call AppendPath for empty paths. I don't know if NuGet packages with that fix will be published for PDFsharp 1.50 or 1.51. If possible, switch to PDFsharp 6.0.0.