views:

48

answers:

4

I have an image resized program and it works. The problem is when a user selects a non-image file in the file select dialog, it crashes. How can I check for image files?

+1  A: 

Your first line of defense, of course, would be simply to check the file's extension:

Function IsImageFile(ByVal filename As String) As Boolean
    Dim ext As String = Path.GetExtension(filename).ToLowerInvariant()

    ' This supposes your program can deal only with JPG files; '
    ' you could add other extensions here as necessary. '
    Return ext = ".jpg" OrElse ext = ".jpeg"
End Function

Better yet, as SLC suggests in a comment, set your dialog's Filter property:

dialog.Filter = "Image files|*.jpg;*.jpeg"

This isn't a guarantee -- ideally you'd want to check the file itself to verify it's an image, and theoretically you should also be able to load files with anomalous extensions if they are in fact image files (maybe just ask for the user's acknowledgement first) -- but it's an easy start.

Dan Tao
better to set the file type filter in the dialog box
SLC
It would be better to check for other case variants of the extension using `string.Equals(ext, ".jpg", StringComparison.InvariantCultureIgnoreCase)`.
0xA3
@0xA3: In short little methods like this I tend to go with `ToLower` as it's less typing than writing out potentially several `string.Equals` method calls.
Dan Tao
`ToLower()` is fine for simple samples like this. However, not only that a new string object is created, it is also not safe and failes the [Turkey Test](http://www.codinghorror.com/blog/2008/03/whats-wrong-with-turkey.html). At least one should use `ToLowerInvariant()` in production code.
0xA3
@0xA3: Yes, a new string is created; but considering it's likely to be 3-5 characters, I wouldn't sweat it. It's still vastly preferable (in my opinion) to typing `ext.Equals(".jpg", StringComparison.InvariantCultureIgnoreCase) || ext.Equals(".jpeg", StringComparison.InvariantCultureIgnoreCase) || [...]` -- obviously in a performance-critical scenario, things would be different. I had forgotten about the Turkey test, though!
Dan Tao
That depends. You usually would also have to cater for a `NullReferenceException` (here if your file doesn't have an extension). Something you don't need to do with the static `string.Equals` method.
0xA3
@0xA3: I used to always include all the defensive checks and throw appropriate exceptions whenever I posted code on SO; eventually I realized I was doing a lot of typing that wasn't really relevant to the OP's question. So I've tried to kick that habit in favor of just posting whatever's directly relevant to the question. This is just a long way of explaining why I didn't bother including `if (string.IsNullOrEmpty(filename)) return false;` -- after all, there's no point calling `string.Equals` over and over on a null string.
Dan Tao
@0xA3: By the way `Path.GetExtension` only returns `null` if it's *passed* `null`; so the scenario of a filename without an extension wouldn't be problematic.
Dan Tao
+2  A: 

A very primitive check is to simply try to load the image. If it is not valid an OutOfMemoryException will be thrown:

static bool IsImageValid(string filename)
{
    try
    {
        System.Drawing.Image img = System.Drawing.Image.FromFile(filename);
    }
    catch (OutOfMemoryException)
    {
        // Image.FromFile throws an OutOfMemoryException 
        // if the file does not have a valid image format or
        // GDI+ does not support the pixel format of the file.
        //
        return false;
    }
    return true;
}

If I understood your question correctly your application it going to load the image anyway. Therefore simply wrapping the load operation in a try/catch block does not mean any additional overhead. For the VB.NET solution of this approach check the answer by @Alex Essilfie.

The ones wondering why Image.FromFile is throwing an OOM on invalid files should read the answer of Hans Passant to the following question:

Is there a reason Image.FromFile throws an OutOfMemoryException for an invalid image format?

0xA3
What a strange exception to throw for this case, huh?
Dan Tao
@Dan Tao: The reason for this is the mapping of the old Windows error code 3 (FILE_NOT_FOUND) which is returned by the native GDI+ function that loads image (error codes were rather limited and it was not easy to add new ones like INVALID_IMAGE_FORMAT). I don't know exactly why it has been mapped to an `OutOfMemoryException` but I assume there was reason for it.
0xA3
A: 

The most robust way would be to understand the signatures of the files you need to load.

JPEG has a particular header format, for example.

This way your code won't be as easily fooled if you just look at the extension.

163's answer should get you most of the way along these lines.

John at CashCommons
I see what you did there...
SLC
a simple extension filter would be a good first start, but you would want something like this after, just to make sure that renamed files don't slip through, but of course, encapsulating it all inside a try/catch statement would be a final test to make sure that the result won't crash your program. The more error checking, the better (to some degree anyway).
MaQleod
+1  A: 

Here's the VB.NET equivalent of 0xA3's answer since the OP insisted on a VB version.

Function IsValidImage(filename As String) As Boolean
    Try
        Dim img As System.Drawing.Image = System.Drawing.Image.FromFile(filename)
    Catch generatedExceptionName As OutOfMemoryException
        ' Image.FromFile throws an OutOfMemoryException  
        ' if the file does not have a valid image format or 
        ' GDI+ does not support the pixel format of the file. 
        ' 
        Return False
    End Try
    Return True
End Function

You use it as follows:

If IsValidImage("c:\path\to\your\file.ext") Then
    'do something
    '
Else
    'do something else
    '
End If

Edit:
I don't recommend you check file extensions. Anyone can save a different file (text document for instance) with a .jpg extension and trick you app into beleiving it is an image.

The best way is to load the image using the function above or to open the first few bytes and check for the JPEG signature.



You can find more information about JPEG files and their headers here:

Alex Essilfie
Thanks for doing that extra work, +1 :)
0xA3
Ordinarily I wouldn't have done this but the OP insisted so I had to. You deserve the credit anyway as you answered earlier.
Alex Essilfie