dotnet / winforms

Windows Forms is a .NET UI framework for building Windows desktop applications.
MIT License
4.33k stars 961 forks source link

Image.FromStream can throw undocumented exceptions #8820

Open corranrogue9 opened 5 years ago

corranrogue9 commented 5 years ago

The documentation for Image.FromStream indicates that an ArgumentException can be thrown if the provided image is not of a supported format. However, on Windows, it can also throw InvalidOperationException, ExternalException, FileNotFoundException, etc. as per the StatusException method. The callers of this method do not catch and wrap these exceptions into ArgumentExceptions.

It should also be noted that an ExternalException can be thrown in the event that the stream does not contain a valid image. Further, in this case, the actual HResult is clobbered by E_FAIL, so the caller of Image.FromStream cannot even handle the case themselves by catching the ExternalException and checking for the appropriate HResult.

You can reproduce the above issue with the image generated by:

            // this is a magic number length which, when containing the right bits, the GDI+ library that the Image 
            // class leverages is unable to handle
            var photo = new byte[15633];
            // this is a jpeg header
            var header = Convert.FromBase64String("/9j/4AAQSkZJRgABAQEAAA==");
            // this is a sequence of bytes which, when at the end of a jpeg of the given length, is not a valid jpeg
            // format and GDI+ does not handle gracefully
            var footer = Convert.FromBase64String(
                "/8AAEQgEOAeAAwEiAAIRAQMRAf/EAB8AAAEFAQEBAQEBAAAAAAAAAAABAgMEBQYHCAkKC//EALUQAAIBAwMCBAMFBQQEAAABfQE" +
                "CAwAEEQUSITFBBhNRYQcicRQygZGhCCNCscEVUtHwJDNicoIJChYXGBkaJSYnKCkqNDU2Nzg5OkNERUZHSElKU1RVVldYWVpjZG" +
                "VmZ2hpanN0dXZ3eHl6g4SFhoeIiYqSk5SVlpeYmZqio6Slpqeoqaqys7S1tre4ubrCw8TFxsfIycrS09TV1tfY2drh4uPk5ebn6" +
                "Onq8fLz9PX29/j5+v/EAB8BAAMBAQEBAQEBAQEAAAAAAAABAgMEBQYHCAkKC//EALURAAIBAgQEAwQHBQQEAAECdwABAgMRBAUh" +
                "MQYSQVEHYXETIjKBCBRCkaGxwQkjM1LwFWJy0QoWJDThJfEXGBkaJicoKSo1Njc4OTpDREVGR0hJSlNUVVZXWFlaY2RlZmdoaWp" +
                "zdHV2d3h5eoKDhIWGh4iJipKTlJWWl5iZmqKjpKWmp6ipqrKztLW2t7i5usLDxMXGx8jJytLT1NXW19jZ2uLj5OXm5+jp6vLz9P" +
                "X29/j5+v/aAAwDAQACEQM=");
            Buffer.BlockCopy(header, 0, photo, 0, header.Length);
            Buffer.BlockCopy(footer, 0, photo, photo.Length - footer.Length, footer.Length);
danmoseley commented 5 years ago

@corranrogue9 is this the same in .NET Framework?

I'm wondering whether we should just document this. (would move to https://github.com/dotnet/dotnet-api-docs)

corranrogue9 commented 5 years ago

Yes, the same in .NET framework as well. I think there are two parts here: the documentation reflecting reality, and that some ExternalException cases make it difficult to differentiate whether the callee or the caller has caused the error.

abelykh0 commented 5 years ago

In addition, if the image is invalid, under Linux (Debian), it throws System.Runtime.InteropServices.ExternalException ("A generic error occurred in GDI+"). You can consider this a documentation issue, or a bug. Exact same code returns documented System.ArgumentException under Windows.

JeremyKuhne commented 1 year ago

It's pretty hard to make substantial improvements here. We are somewhat tied by generally not being able to change exception types and GDI+ often does not give great error info. There is also a bit of "magic" in interop error translation. We can incrementally add things to docs as we discover them, but it would be pretty hard to know that we've documented all cases.