I'm trying to develop some templates for common PHP tasks I've been dealing with. One of which is a general image file upload handler.
So far I'm using the following reusable code which seems to be working fine without any noticeable bug:
<?php
if ( !isset($_POST['submit']) ) {
goto page_content;}
if ( $_FILES['file_upload']['error']===4 ) {
echo 'No file uploaded';
goto page_content;}
if ( $_FILES['file_upload']['error']===1 || $_FILES['file_upload']['error']===2 ) {
echo 'File exceeds maximum size limit';
goto page_content;}
if ( $_FILES['file_upload']['error']!==0 ) {
echo 'Failed to upload the file';
goto page_content;}
if ( !is_uploaded_file($_FILES['file_upload']['tmp_name']) ) {
echo 'Failed to upload the file';
goto page_content;}
require_once('imageResize.php');
$err = imageResize($_FILES['file_upload']['tmp_name'], 'random.png' );
if ( $err !== 0 ) {
echo 'Invalid image format';
goto page_content;}
echo 'Image uploaded successfully';
page_content:
?>
<form action="filename.php" method="POST" enctype="multipart/form-data">
<input type="hidden" name="MAX_FILE_SIZE" value="1000000">
<input type="file" name="file_upload" accept="image/*">
<input type="submit" name="submit">
</form>
Additional file imageResize.php
:
<?php
// image resize
function imageResize($source, $target){
$size = getimagesize($source);
if ($size === false) {return 1;} // invalid image format
$sourceImg = @imagecreatefromstring(@file_get_contents($source));
if ($sourceImg === false) {return 2;} //invalid image format
$width = imagesx($sourceImg);
$height = imagesy($sourceImg);
$sidelenght = min($width,$height);
$targetImg = imagecreatetruecolor(100, 100);
imagecopyresampled($targetImg, $sourceImg, 0, 0, ($width-$sidelenght)/2, ($height-$sidelenght)/2, 100, 100, $sidelenght, $sidelenght);
imagedestroy($sourceImg);
imagepng($targetImg, $target);
imagedestroy($targetImg);
return 0;
}
?>
Some main characteristics of this code are:
- provides messages for the most common errors that can happened during the upload process
- it allows the client to upload an image file up to 1Mb size
- resizes all images to a standard 100x100 px size
- save all images to a standard PNG format
Questions
- Is this code safe? Or are there any vulnerability that could be exploited by an malicious client? In this case, how can I solve it?
- To avoid several nested
if-then-else
conditions (which can become hard to read), I'm currently usinggoto
(which can become a bad control structure practice). Is there a better alternative?
-
\$\begingroup\$ Following others suggestion, this is a repost from StackOverflow \$\endgroup\$Mark Messa– Mark Messa2018年07月04日 17:30:37 +00:00Commented Jul 4, 2018 at 17:30
1 Answer 1
DO NOT use GOTO. They have been criticised since 1960s.
- Your indentation is broken for the first snippet. It is also inconsistent with your second snippet. Keep it consistent throughout a project. Try looking at some linters or follow a style guide.
- Do not suppress your errors by using
@
. Let the function throw any and all errors it encounters. You should instead make use oftry-catch
blocks. - While you could replace the error lookups by error code to a
switch-case
block, I'd suggest that you use an associated array (or a hashtable) to keep a mapping fromERROR_CODE => "ERROR MESSAGE"
. This will go in a separateerrors.php
maybe, and referenced as needed. - Define the target image resolution as a constant, instead of placing magic numbers.
- You could also have a client side javascript snippet for checking file size, so that users do not waste their bandwidth and time waiting for upload process.
-
\$\begingroup\$ I'm surprised there is a GOTO in PHP. I've learned something. \$\endgroup\$KIKO Software– KIKO Software2018年07月05日 15:42:41 +00:00Commented Jul 5, 2018 at 15:42
-
\$\begingroup\$ @KIKOSoftware Just for the records, the
GOTO
available at PHP is not full unrestricted. \$\endgroup\$Mark Messa– Mark Messa2018年07月05日 19:04:40 +00:00Commented Jul 5, 2018 at 19:04
Explore related questions
See similar questions with these tags.