2
\$\begingroup\$

I am using the following code to rename the uploaded files for any future name clashes. Please tell if my approach is right or I should follow some other approach or approach which is better.

[HttpPost]
    public ActionResult Upload(HttpPostedFileBase fileData)
    {
        if (fileData != null && fileData.ContentLength > 0)
        {
            //var fileName = Server.MapPath("~/Content/Images/" + Path.GetFileName(fileData.FileName));
            int pictureCount = 800000;
            pictureCount += db.Pictures.Count();
            string extension = Path.GetExtension(fileData.FileName);
            string renamedImage = Server.MapPath("~/Content/Images/Categories/cat" + pictureCount + extension);
            fileData.SaveAs(renamedImage);
            return Json(Path.GetFileName(renamedImage));
        }
        return Json(false);
    }
\$\endgroup\$

2 Answers 2

3
\$\begingroup\$

If you have two simultaneous file uploads db.Pictures.Count(); will return same number. So you will get a name clash.

If db.Pictures is a database then you can create and auto increment identity field and use it to save the file.

If there is no option with database you can use two more approaches: Use GUID instead of picture name

[HttpPost]
public ActionResult Upload(HttpPostedFileBase fileData)
{
    if (fileData != null && fileData.ContentLength > 0)
    {
        string extension = Path.GetExtension(fileData.FileName);

        string renamedImage = null;
        do
        {
            renamedImage = Server.MapPath("~/Content/Images/Categories/cat" + System.Guid.NewGuid().ToString("N") + extension);

        } while( /*verify that renamedImage file does not exist*/ );

        fileData.SaveAs(renamedImage);
        return Json(Path.GetFileName(renamedImage));
    }
    return Json(false);
}

The drawback of this approach is that it is possible to upload same pictures to the server.

To mitigate that you can try using hash of the image as a file name. Before writing file contents to the disk check if that file already exists. Note: the check and file save operation must be made in thread-safe manner.

\$\endgroup\$
2
  • \$\begingroup\$ thanks mate...I don't suppose any multiple simultaneous upload but this is definitely a point of consideration. db.Pictures is infact a database table with auto-increment Id but i don't know how to use it in that manner \$\endgroup\$ Commented Dec 4, 2011 at 11:49
  • \$\begingroup\$ I'd go with the guid-option... \$\endgroup\$ Commented Dec 4, 2011 at 15:45
1
\$\begingroup\$

You should certainly save the file with a Guid filename (or even save it to the database in a VARBINARY(MAX) column or in a FILESTREAM column (which actually serializes it to disk).

I also tend to store the MimeType, so I know how to display the file in the browser (not necessary in your case, but think what happens if you extend this to allow uploading of PDFs etc which I tend to do).

When I allow file uploads I tend to store this kind of data and have a Guid as the primary key on the database:

// postedFile is the HttpPostedFileBase
uploadedFile.MimeType = postedFile.ContentType;
uploadedFile.FileData = new byte[postedFile.ContentLength];
postedFile.InputStream.Read(uploadedFile.FileData, 0, postedFile.ContentLength);
uploadedFile.FileHash = _uploadedFileService.ComputeFileHash(uploadedFile.FileData); // see below for this code
uploadedFile.UserId = User.UserId;
uploadedFile.DateCreated = DateTime.UtcNow;
uploadedFile.OriginalFilename = postedFile.FileName;

Computing the MD5 filehash is a great way to make sure you don't save the same file twice under different names.

Here is some code (if you know which OS you'll be running on, you don't need to do the check obviously, I've just put it here for your benefit!):

public byte[] ComputeFileHash(byte[] fileData)
{
    if (System.Environment.OSVersion.Version.Major == 5)
    {
        // Windows XP
        System.Security.Cryptography.MD5 md5 = System.Security.Cryptography.MD5.Create();
        return md5.ComputeHash(fileData);
    }
    else
    {
        // Windows Vista, 7, Server 2008
        System.Security.Cryptography.MD5Cng md5 = new System.Security.Cryptography.MD5Cng();
        return md5.ComputeHash(fileData);
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.