Upload an image to a folder through a form

I am using this code to upload an image. Please review it and give your feedback regarding performance, security, and quality. This also works in PDO project.

 

 
 

 
Upload File 
Filename1: 
 
 
 
 

 0){ 
echo "Error: " . $_FILES["photo"]["error"] . ""; 
} else{ 
$allowed = array("jpg" => "image/jpg", "jpeg" => "image/jpeg", "gif" => "image/gif", "png" => "image/png"); 
$filename = $_FILES["photo"]["name"]; 
$filetype = $_FILES["photo"]["type"]; 
$filesize = $_FILES["photo"]["size"]; 

// Verify file extension 
$ext = pathinfo($filename, PATHINFO_EXTENSION); 
if(!array_key_exists($ext, $allowed)) die("Error: Please select a valid file format."); 

// Verify file size - 5MB maximum 
$maxsize = 5 * 1024 * 1024; 
if($filesize > $maxsize) die("Error: File size is larger than the allowed limit."); 

// Verify MYME type of the file 
if(in_array($filetype, $allowed)){ 
// Check whether file exists before uploading it 
if(file_exists("upload/" . $_FILES["photo"]["name"])){ 
echo $_FILES["photo"]["name"] . " is already exists."; 
} else{ 
move_uploaded_file($_FILES["photo"]["tmp_name"], "upload/" . $_FILES["photo"]["name"]); 
echo "Your file was uploaded successfully."; 
} 
} 
else
{ 
echo "Error: There was a problem uploading your file - please try again."; 
} 
} 

} else{ 
echo "Error: Invalid parameters - please contact your server administrator."; 
} 
?>

You should invert the order of your PHP processing and your HTML output. When working in PHP, always strive to adhere to this approach, as it will help you as you start working on more complex logic. You will inevitably find yourself needing to set HTTP response readers, cookie values, etc. in your responses. You need to do this work BEFORE sending any output to the browser - either that or use a hacky output buffering approach.

If you need to set conditional output (i.e. success/error messages, dynamic content, variable output, etc.), consider setting these values into variables and having only basic PHP insertions in the HTML portion of the code. You should avoid any complex PHP calculations or logic once output to browser has started.

Other thoughts:

• You need work on your coding style. Primarily in your case that is in the area of using indentations in your code (both HTML and PHP) to better convey nesting of code blocks.

• Don't output system-generated error messages directly to the user. You may want to log these, but you should always consider end user error messages separately from system errors. Oftentimes, that may require mapping of PHP-generated errors to user-friendly error messaging. Here is good example: echo "Error: " . $_FILES["photo"]["error"] . "



";

• Things like file size that are too large may best end up with server responding with 400 or 413 HTTP response code in response header rather than just simple error message. You should actually not handle this at all in your PHP code, but rather in your PHP configuration, so that file upload limits are applied at PHP level, not in your code logic. If for some reason you need to change the file size limit for a particular script, you could make runtime change to configuration. There is no reason to defer this file size check into your code. The PHP file upload common pitfalls page has some good additional info in this area - http://php.net/manual/en/features.file-upload.common-pitfalls.php

• From a security standpoint, you are doing nothing to prevent against cross-site request forgery (CSRF) attacks. Typically, this is handled with comparison of hidden form input field with value in session. This really should be a standard part of any form your create in any web application.

• You are doing nothing to validate $_FILES and some of the values it holds before operating against the information. The file name, in particular is something you might want to validate, to make sure the user isn't passing a filenames like ../../index.php which could end up writing files in arbitrary locations in your web directory.

StackExchange Code Review Q#144880, answer score: 6