Image uploading with PHP

Question

I've been building an event promotion site in PHP and MySQL for the past couple of months where anyone can sign up and add their local event's details along with a poster, which I resize.

As it stands, I've got the whole process working fine locally and on a hosting service, but before the site goes live I have a couple of questions on the way I'm doing it.

This is the function code I'm using to handle the image uploads. I check for filesize before this section.

$extension = substr($filename, strpos($filename,'.'), strlen($filename)-1); 
$filetypes = array('.jpg', '.jpeg', '.gif', '.bmp', '.png', '.JPG', '.PNG', '.JPEG', '.GIF', '.BMP');
if($_FILES['image']['error'] == 4){
  $error = "No image";
  return $error; 
}
else if(($_FILES['image']['error'] == 2) || ($_FILES['image']['error'] == 1)){
  $error = "File size too big";
  return $error;
}
else if(!in_array($extension, $filetypes)){
  $error = "This isn't an image that is supported";
  return $error;
}
else if(($_FILES['image']['error'] == 7) || ($_FILES['image']['error'] == 3)){
  $error = "Error occurred. Try again";
  return $error;
}
else{
  if(($extension == '.jpg') || ($extension == '.jpeg')){
    $source = imagecreatefromjpeg($uploaded);
  }
  else if($extension == '.png'){
    $source = imagecreatefrompng($uploaded);
  }
  else{
    $source = imagecreatefromgif($uploaded);
  }
  list($width, $height) = getimagesize($uploaded);
  $ratio = $width / $height;
  $new_width = 300;
  $new_height = round(300 / $ratio); 
  $canvas = imagecreatetruecolor($new_width, $new_height);
  imagecopyresampled($canvas, $source, 0, 0, 0, 0, $new_width, $new_height, $width,       $height);
  $name = date("dmyHis").rand(0, 9);
  $path = $_SERVER[ 'DOCUMENT_ROOT' ] . '/images/uploaded/'.$name.'.jpg';
  $new_image = imagejpeg($canvas, $path,  100);
  $poster['name'] = $name.'.jpg';
  $poster['width'] = $new_width;
  $poster['height'] = $new_height;
  return $name.'.jpg';
}

As it stands, there are a couple of bugs that I know about, or haven't fully looked into, such as some images throwing an error from imagecreatefromwhatever, and if the image name has a '.' in it, it'll also throw an error.

Once the process is done, I'll save the image name into a 'poster' field in MySQL, which will be used to get the correct image from the folder when being viewed.

What I really wanted to know is if there's any other problems I'm likely to face with image uploads?

• I'm expecting a fair amount of traffic, so is this code going to run alright with heavy usage?
• Are there any other pitfalls or things I should be looking out for?
• Am I using the best method for the job?
• My filesize limit at the moment is 2MB, is this too high?
• Even if a user uploads something over 2MB, the script will still run, and I assume the file will be uploaded to the server for name stripping and filesize comparison etc., how will this affect my bandwidth usage?
• How long do original files stay on the server?

If anyone has any good reading on the subject I would much appreciate it!

Thank you.

edit: Formatting.

edit 2: I didn't make myself clear about the original files. What I mean is the original files that I use the $_FILES variable to access. Say it's 1.9MB, will there be 1.9MB's worth of image sitting on the server the whole time I'm fiddling with the extensions and that? Should I clear this once I've created a new image?

Answer 1

Rich, I've done similar, primarily with iMagick. GD is functionally similar, so I expect no problems. My site handled a hundred images a week without issue, and reliably served to ~1k/week. I did all processing on the backend as it appears you're doing in this example, so few worries about heavy inbound traffic (such as a pickup on DIGG) crushing your server.

The biggest challenge that you're opening yourself up to vulnerabilities by allowing uploads of any sort. You've probably heard IT security guys say that the only way to protect from hackers is to get off the web....it's kinda like that. I wouldn't live in complete fear, because it looks like you've taken fair steps to audit file types and sizes. An additional consideration is to look at the permissions on your server--open the directory write only to the user agent of the server and block browsing to the directory for extra security. If you want to be doubly safe, write to a different account (if you have such available) to limit the exposure to your code. It's not necessary, but it's a good extra step if you have concerns. Finally, place your uploader behind a simple password system with captcha to block the auto-exploit checkers....give users free access with a simple registration step. It's a small UI hassle, but could make all the difference security-wise.

I might consider stopping the process if the user file is over 2mb limit. That's just me. You wouldn't want the system to crash and burn if some user trys to force a bad file with a .jpg file extension on your server. Bandwidth is likely only a concern if you're hosting yourself or if you're paying by the meg--sure, this'll raise your bandwidth, but even with a hundred uploads a day you're likely not going to be pushing a standard server beyond its means unless the corresponding traffic visiting the site is in the thousands. Most hosts will allow you to monitor server load. I've had good luck on the cheap personally with HostGator

Files stay on the server indefinitely, assuming nothing bad happens to your account and you pay your bills. For that reason, make sure to separate your upload folder from that of any other content--it gets to be a nightmare once a couple hundred files have been uploaded. Backup often, just to be safe.

Answer 2

First of all, well done, looks like you put a lot of work in it.

There are a few things that can make life just a little easier for you. The list of things below is not to disappoint you, but for you to learn!

You take everything from the first . as the extension as it stands now. Therefore the error when someone puts a . in the filename. This can be done better.

$extension = '';
if ( preg_match("/\\.([a-z]+)$/i",$filename,$match) )
{
    $extension = strtolower($match[1]);
}

will give you the extension, in lowercase, without the dot, which means you don't have to test for JPG as well as jpg, etc. (actually, while uploading the browser will tell you the file type, regardless of the extension, but lets just skip that for now - the extension test will do fine)

The image reading, with if JPG else if PNG else GIF is not really clean: you should test for gif as well and only then go to the "else" category, and just throw up an error. (meaning you can drop the check you did before!)

When you say $source = imagecreatefrom...($filename) you better prepend it with a @ to avoid the warning on corrupted images (normally, don't use @, but in this case you cannot know if the image is corrupt). Then always check return values (ALWAYS do that), like

$source = @imagecreatefrompng($filename);
if ( !$source ) return "Error parsing image";

The image is now loaded, so the size if known; you don't have to query the file again. Instead of getimagesize() you could use imagesx($source) and imagesy($source)

This is enough to fix for now, I think. ;-)

edit: Tiny problem btw with rand(0,9) in filename meaning that chances are big you get files mixed up if more than one client uploads within the same second. (with 11 uploads per second you have a problem for sure)