tags:

views:

51

answers:

2

Hi All,

I have been working on an image gallary. When the user uploads an Image I now check the size of the file. If it is less than 1MB I check to see that the file is actually an image type. Finally I resize the image to an appropriate gallary size, and create a small thumbnail of the image. However since adding in the code to check the types I have been experiancing and OutOfMemoryException.

Here's my controller method:

        [AcceptVerbs(HttpVerbs.Post)]
    public ActionResult Upload(Image image, HttpPostedFileBase ImageFile)
    {

        if (ImageFile.ContentLength > 0)
        {
            // Get the size in bytes of the file to upload.
            int fileSize = ImageFile.ContentLength;

            // Allow only files less than 1,048,576 bytes (approximately 1 MB) to be uploaded.
            if (fileSize < 1048576)
            {
                string fileclass = "";

                using (BinaryReader r = new BinaryReader(ImageFile.InputStream))
                {
                    byte buffer = r.ReadByte();
                    fileclass = buffer.ToString();
                    buffer = r.ReadByte();
                    fileclass += buffer.ToString();
                    r.Close();
                }

                switch (fileclass)
                {
                    case "7137":
                    case "255216":
                    case "13780":
                        try
                        {
                            string path = Server.MapPath("~/Uploads/");

                            ImageFile.SaveAs(path + ImageFile.FileName);

                            ResizeImageHelper resizeImageHelper = new ResizeImageHelper();
                            resizeImageHelper.ResizeImage(path + ImageFile.FileName, path + ImageFile.FileName, 640, 480, false);
                            resizeImageHelper.ResizeImage(path + ImageFile.FileName, path + "thumb" + ImageFile.FileName, 74, 74, false);

                            image.imageLocation = ImageFile.FileName;
                            image.imageThumb = "thumb" + ImageFile.FileName;

                            imageRepository.Add(image);
                            imageRepository.Save();

                            return RedirectToAction("Index", "Home");
                        }
                        catch (Exception ex)
                        {
                            return View("Error");
                        }
                }

            }
            else
            {
                //If file over 1MB
                return View("Error");
            }
        }
        else
        {
            //If file not uploaded
            return View("Error");
        }

        return View("Error");
    }

And here is the Resize method I use:

public void ResizeImage(string OriginalFile, string NewFile, int NewWidth, int MaxHeight, bool OnlyResizeIfWider)
    {
        System.Drawing.Image FullsizeImage = System.Drawing.Image.FromFile(OriginalFile);

        // Prevent using images internal thumbnail
        FullsizeImage.RotateFlip(System.Drawing.RotateFlipType.Rotate180FlipNone);
        FullsizeImage.RotateFlip(System.Drawing.RotateFlipType.Rotate180FlipNone);

        if (OnlyResizeIfWider)
        {
            if (FullsizeImage.Width <= NewWidth)
            {
                NewWidth = FullsizeImage.Width;
            }
        }

        int NewHeight = FullsizeImage.Height * NewWidth / FullsizeImage.Width;
        if (NewHeight > MaxHeight)
        {
            // Resize with height instead
            NewWidth = FullsizeImage.Width * MaxHeight / FullsizeImage.Height;
            NewHeight = MaxHeight;
        }

        System.Drawing.Image NewImage = FullsizeImage.GetThumbnailImage(NewWidth, NewHeight, null, IntPtr.Zero);

        // Clear handle to original file so that we can overwrite it if necessary
        FullsizeImage.Dispose();

        // Save resized picture
        NewImage.Save(NewFile);
    }

Can anyone advise with this? I am currently just playing around trying to learn new things :-)

Thanks,

Jon

Progress I have narrowed it down to this block, when commented out things function as normal:

using (BinaryReader r = new BinaryReader(ImageFile.InputStream))
                {
                    byte buffer = r.ReadByte();
                    fileclass = buffer.ToString();
                    buffer = r.ReadByte();
                    fileclass += buffer.ToString();
                    r.Close();

                }
+1  A: 

I'm assuming this doesn't happen the first run through, but after some time. Is this correct?

edit: removed incorrect assumption, but there's still an issue with IDisposable


You're not disposing of NewImage, and this will cause you issues in production.

I'd normally say 'just use a using', but try/finally is the same thing. Refactor to us a using at your own discretion.

System.Drawing.Image NewImage = null;
System.Drawing.Image FullsizeImage = null;

try
{
    FullsizeImage = System.Drawing.Image.FromFile(OriginalFile);

     [... snip ... ]

    NewImage = FullsizeImage.GetThumbnailImage(NewWidth, NewHeight, null, IntPtr.Zero);

    // Clear handle to original file so that we can overwrite it if necessary
    FullsizeImage.Dispose();

    // Save resized picture
    NewImage.Save(NewFile);
}
finally
{
    if (FullsizeImage != null)
        FullsizeImage.Dispose();
    if (NewImage != null)
        NewImage.Dispose();
}
Robert Paulson
No this happens on the first run. Looking into the logic on this line ImageFile.SaveAs(path + ImageFile.FileName); The file does not save correctly in the folder. It shows as 0 bytes.
Jonathan Stowell
I see. Well your image resize code is still missing it's `IDisposable.Dispose` calls. I'll leave this answer as a comment to that.
Robert Paulson
For any IDisposable object, you should really get in the habit of scoping the use of the object to handle .Dispose() calls for you. using (Image newImage = FullSizeImage.GetThumbnailImage(newWidth, newHeight, null, IntPtr.Zero) { // do stuff with newImage } // end scope automatically disposes of the object
Michael
@Michael Yes, that's what I wrote too, but don't worry - the try/finally code as written is equivalent to the code the compiler will generate with a using block. See http://msdn.microsoft.com/en-us/library/aa664736(VS.71).aspx
Robert Paulson
A: 

Ok Guys after discovering this part of the code was the problem:

using (BinaryReader r = new BinaryReader(ImageFile.InputStream))
            {
                byte buffer = r.ReadByte();
                fileclass = buffer.ToString();
                buffer = r.ReadByte();
                fileclass += buffer.ToString();
                r.Close();

            }

I changed it to this:

string fileclass = ImageFile.ContentType.ToString();

And altered my switch statement to:

switch (fileclass)
                {
                    case "image/jpg":
                    case "image/jpeg":
                    case "image/png":
                    case "image/gif":
                        try

I also implemented the suggestions from Robert. However I am unsure being new to .NET whether this method of checking the file type is as accurate as the previous? My research seems to suggest the previous could discover the file type even if the up loader had changed the extension e.g. renaming example.exe to example.jpg. I am unsure as to whether using the provided .NET functionality the same is still achieved?

Jonathan Stowell
I can't remember offhand, but the `ContentType` of the request could be set by the client performing the upload, and if that's the case, spoofed by a hostile client. Same goes for file extension. So I might not want to trust them entirely. You can also check that for an instance of a `System.Drawing.Image` the property `RawFormat` is of type `System.Drawing.Imaging.ImageFormat` and compare it to static instances. E.g. `if (uploadedImage.RawFormat == ImageFormat.Jpeg) ...`
Robert Paulson