Fixes #127 Check for existence of gdiImage comes to late

[th-joerger]

I encountered the same problem as #127 in my production environment. Several customers have been severly affected with hundreds of wrongly printed qr codes.

The culprit seems to be this dysfunctional code:

                    else if (image._stream != null!)
                    {
                        if (image._stream is MemoryStream ms)
                        {
                            var md5 = System.Security.Cryptography.MD5.Create();
                            var hash = md5.ComputeHash(ms);
                            image._path = "*md5:" + HashToString(hash);
                        }
                        else
                        {
                            // TODO Use hash code.
                            image._path = "*" + Guid.NewGuid().ToString("B");
                        }
                    }
#if GDI
                    else if (image._gdiImage != null!)
                    {
                        // TODO Use hash code.
                        image._path = "*" + Guid.NewGuid().ToString("B");
                    }
#endif

The md5 is always *md5:d41d8cd98f00b204e9800998ecf8427e which is the md5 hash of an empty input. The stream image._stream is already consumed when computing the hash, because in GDI in XImage.cs, the stream is already used to create an Image.

The work around for GDI with the check for the existence of image._gdiImage comes to late after the md5 hash is already computed.

When the memory stream is reset, everything works as expected.

                        if (image._stream is MemoryStream ms)
                        {
                            ms.Seek(0, SeekOrigin.Begin); // make sure, ms is not already consumed
                            var md5 = System.Security.Cryptography.MD5.Create();
                            var hash = md5.ComputeHash(ms);
                            image._path = "*md5:" + HashToString(hash);
                        }

So, I think the fix is, to check for the existence of image._gdiImage before computing the md5 hash. I send you a PR including a test. I would also appreciate you properly merging PRs instead of simply copying the code and squashing my contribution.

[ThomasHoevel]

Thanks for your contribution. We already fixed that issue in our internal repository and our solution is much simpler than yours.

I would also appreciate you properly merging PRs instead of simply copying the code and squashing my contribution.

This is a repository we use to publish released versions, it is not the repository we use for development.

[th-joerger]

Thank you for your kind response. When do you plan to release your much simpler solution?

This is a repository we use to publish released versions, it is not the repository we use for development.

Can you point me to the repository for development? I could not find it.

[ThomasHoevel]

The bug has been fixed in PDFsharp without using any code from this PR.

It spoils the idea of the ImageSelector class to check for GdiImage before checking for MemoryStream.

[th-joerger]

It spoils the idea of the ImageSelector class to check for GdiImage before checking for MemoryStream.

But this was exactly, what I suggested and fixed in the PR. You need to check GdiImage before you check MemoryStream.

The ImageSelector class does currently NOT (also in today's release) check GdiImage before checking for MemoryStream.

I also implemendet a Todo: Hash-Compare of GdiImages.