File Upload Class, first time OOP

Coding Critique is the place to post source code for peer review by other members of DevNetwork. Any kind of code can be posted. Code posted does not have to be limited to PHP. All members are invited to contribute constructive criticism with the goal of improving the code. Posted code should include some background information about it and what areas you specifically would like help with.

Popular code excerpts may be moved to "Code Snippets" by the moderators.

Moderator: General Moderators

Post Reply
User avatar
Sindarin
Forum Regular
Posts: 521
Joined: Tue Sep 25, 2007 8:36 am
Location: Greece

File Upload Class, first time OOP

Post by Sindarin »

First shot at a class, I needed a reusable file upload script.
Comments please, and on how I can make it more efficient.

EDIT: I did most of the changes proposed here plus I added fileInfo (although doesn't work on my server yet) mime type checking, a couple of functions to get the allowed extensions/mime types so to display them to the user, comma separated, added some extra error codes for http upload errors and also made it possible to upload to the folder using FTP through cURL. :D
There might be some cleanup work to do though this is much my completed class for now.

index.php (for testing)

Code: Select all

 
<?php
 
/* require the class file for the uploader */
require_once('file-uploader-class.php');
 
?>
<html>
 
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<title>File Upload</title>
 
<style type="text/css">
body
{
font-family: Verdana, Georgia, Arial;
font-size:12px;
cursor:default;
}
</style>
 
</head>
<body>
<?php 
 
 
/* create a new uploader object */
$uploader = new fileUploader(20000000, 0, 0, 2200, 2200, 0, array('.gif','.png','.jpg') , array('image/jpeg','image/jpg','image/png','image/gif'), 'uploads/images', 'http', '', '', ''); 
//(ftp method)
//$uploader = new fileUploader(20000000, 0, 0, 2200, 2200, 0, array('.gif','.png','.jpg') , array('image/jpeg','image/jpg','image/png','image/gif'), 'uploads/images', 'ftp', 'localhost/file-uploader-class', 'DEFAULT', ''); 
 
if (isset($_GET['action']) && strip_tags($_GET['action']) == 'upload')
{
 
/* upload from filename1 field */
$upload_message1 = $uploader->fileUpload('filename1','overwrite1');
 
/* upload from filename2 field */
$upload_message2 = $uploader->fileUpload('filename2','overwrite2');
 
/*output results*/
echo '<br />File 1 result:<strong>'.$upload_message1.'</strong><br />
<br />File 2 result:<strong>'.$upload_message2.'</strong><br />';
 
}
?>
<h3>File Upload</h3>
 
<form name="upload" style="width:400px;overflow:hidden;display:inline;" id="upload" enctype="multipart/form-data" method="post" action="index.php?action=upload">
<fieldset style="width:255px;">
<legend>UPLOAD</legend>
File: 
<br />
<input type="hidden" name="MAX_FILE_SIZE" value="<?php echo $uploader->max_filesize; ?>" />
<input type="file" id="filename1" name="filename1" accept="<?php echo $uploader->listAllowedMimeTypes(); ?>" />
<br />
<label for="overwrite1"><input type="checkbox" id="overwrite1" name="overwrite1" value="1" /> overwrite if exists</label>
<br /><br />Allowed extensions: <?php echo $uploader->listAllowedExtensions(); ?><br /><br />
</fieldset>
 
<fieldset style="width:255px;">
<legend>UPLOAD</legend>
File: 
<input type="hidden" name="MAX_FILE_SIZE" value="<?php echo $uploader->max_filesize; ?>" />
<br /><input type="file" id="filename2" name="filename2" accept="<?php echo $uploader->listAllowedMimeTypes(); ?>" />
<br />
<label for="overwrite2"><input type="checkbox" id="overwrite2" name="overwrite2" value="1" /> overwrite if exists</label>
<br /><br />Allowed extensions: <?php echo $uploader->listAllowedExtensions(); ?><br /><br />
</fieldset>
<br />
<input type="submit" name="upload" value="Upload" />
</form>
</body>
</html>
 
 
file-uploader-class.php

Code: Select all

 
<?php
 
/* 
 
file-uploader-class.php
 
description: uploads a file in the specified directory
version: 2.0
last update: 22-Oct-2009
utilization URL: general
 
*/
 
 
class fileUploader
{
    
    public $max_filesize = 20000000; //20 MegaBytes
    public $max_file_width = 1000; //1000 pixels
    public $max_file_height = 1000; //1000 pixels
    public $min_file_width = 0; //0 pixels
    public $min_file_height = 0; //0 pixels
    
    public $filename_to_md5 = 0;
    
    public $file_upload_dir = 'uploads'; //ftp needs full directory e.g. (site.com/ftp)
    public $file_upload_path;
    public $file_protocol = 'http'; //http or ftp
 
    public $file_name = '';
    public $file_name_original;
    public $file_size;
    public $file_width = 0;
    public $file_height = 0;
    public $file_extension = '';
    public $file_temp_name = '';
    public $file_name_tail;
    public $file_error = 4;
    
    public $file_overwrite = 0;
    public $filesize_in_kbs;
    public $file_field_name = 'filename';
    public $file_overwrite_name = 'overwrite';
    
    public $ftp_username = '';
    public $ftp_password = '';
    public $ftp_domain = '';
    
    public $file_allowed_extensions = array();
    public $file_allowed_mime_types = array();
    
    private $file_upload_successful = 1;
    
    private $UPLOAD_NO_FILE = 0;
    private $UPLOAD_NO_FOLDER = 1;
    private $UPLOAD_FILETYPE_INVALID = 2;
    private $UPLOAD_MIMETYPE_INVALID = 3;
    private $UPLOAD_FILENAME_EXISTS = 4;
    private $UPLOAD_FILENAME_INVALID = 5;
    private $UPLOAD_FILESIZE_LARGE = 6;
    private $UPLOAD_FILEDIMENSIONS_LARGE = 7;
    private $UPLOAD_FILEDIMENSIONS_SMALL = 8;
    private $UPLOAD_FILESIZE_LARGE_INI = 9;
    private $UPLOAD_FILESIZE_LARGE_FORM = 10;
    private $UPLOAD_PARTIAL_ONLY = 11;
    private $UPLOAD_NO_TEMPDIR = 12;
    private $UPLOAD_NO_WRITE = 13;
    private $UPLOAD_EXTENSION_STOP = 14;
    private $UPLOAD_FILEINFO_ERROR = 15;
    private $UPLOAD_CURL_ERROR = 16;
    private $UPLOAD_ERROR = 17;
    private $UPLOAD_SUCCESS = 18;
    private $UPLOAD_SUCCESS_OVERWRITTEN = 19;
 
 
    public function __construct( $max_filesize = false, $min_file_width = false, $min_file_height = false, $max_file_width = false, $max_file_height = false, $filename_to_md5 = false, $file_allowed_extensions = false, $file_allowed_mime_types = false, $file_upload_dir = false, $file_protocol = false, $ftp_domain = false, $ftp_username = false, $ftp_password = false)
    {
 
        $this->max_filesize = $max_filesize;
        $this->max_file_width = $max_file_width;
        $this->max_file_height = $max_file_height;
        $this->min_file_width = $min_file_width;
        $this->min_file_height = $min_file_height;
        $this->filename_to_md5 = $filename_to_md5;
            
        $this->file_allowed_extensions = $file_allowed_extensions;
        $this->file_allowed_mime_types = $file_allowed_mime_types;
            
        $this->file_upload_dir = $file_upload_dir.'/';
        $this->file_protocol = $file_protocol;
        $this->ftp_domain = $ftp_domain;
        $this->ftp_username = $ftp_username;
        $this->ftp_password = $ftp_password;
 
    }
    
    public function listAllowedExtensions()
    {
        $file_extensions = implode(',',$this->file_allowed_extensions);
        return $file_extensions;
    }
    
    public function listAllowedMimeTypes()
    {
        $mime_types = implode(',',$this->file_allowed_mime_types);
        return $mime_types;
    }
 
    public function fileUpload($file_field_name = false, $file_overwrite_name = false)
    {
 
        $this->file_field_name = $file_field_name;  
        $this->file_overwrite_name = $file_overwrite_name;
        
        //post file name and other attributes from upload field
        $this->file_name = strip_tags($_FILES[$this->file_field_name]['name']);
        $this->file_temp_name = $_FILES[$this->file_field_name]['tmp_name'];
        $this->file_size = $_FILES[$this->file_field_name]['size'];
        $this->file_error = $_FILES[$this->file_field_name]['error'];
        
        //small delay
        //sleep(2);
        
        //keep original filename
        $this->file_name_original = $this->file_name;
 
        //determine if an overwrite field has been assigned
        if ($this->file_overwrite_name == '')
        {
            $this->file_overwrite = 0;
        }
        else
        {
            $this->file_overwrite = strip_tags($_POST[$this->file_overwrite_name]);
        }
 
        //filename to lowercase
        $this->file_name = strtolower($this->file_name);
 
        //get file extension only if the allowed extensions array is not empty
        if (isset($this->file_allowed_extensions))
        {
            $this->file_extension = strtolower(substr($this->file_name,strrpos($this->file_name,".")));
        }
 
        //filename to md5
        if ($this->filename_to_md5 == 1)
        {
            $this->file_name_tail = mt_rand(0,10000);
            $this->file_name = md5($this->file_name).$this->file_name_tail.$this->file_extension;
            
        }
 
        //determine final upload path
        if ($this->file_protocol=='http')
        {
            $this->file_upload_path = $this->file_upload_dir.$this->file_name;
        }
        else
        {
            $this->file_upload_original_path = $this->file_upload_dir.$this->file_name; //keep original for finfo
            $this->file_upload_path = $this->ftp_domain.'/'.$this->file_upload_dir.$this->file_name_original;
        }
 
        //calculate filesize in kbs
        $this->filesize_in_kbs = $this->file_size/1000;
 
        /* file checks */ 
 
        //initialize successful variable
        $this->file_upload_successful = 1;
 
        //no file selected
        if ($this->file_name_original == '' || $this->file_name_original == null || empty($this->file_name_original) && $this->file_upload_successful == 1)
        {
            $this->file_upload_successful = 0;
            return $this->UPLOAD_NO_FILE;
        }
 
        //no folder present
        if ($this->file_protocol=='http' && $this->file_upload_successful == 1)
        {
            if (!is_dir($this->file_upload_dir)) 
            {
                $this->file_upload_successful = 0;
                return $this->UPLOAD_NO_FOLDER;
            }
        }
 
        //file type check
        if (isset($this->file_allowed_extensions))
        {
            if (!in_array($this->file_extension, $this->file_allowed_extensions) && $this->file_upload_successful == 1)
            {
                $this->file_upload_successful = 0;
                return $this->UPLOAD_FILETYPE_INVALID;
            }
        }
 
        //check if file exists
        if (file_exists($this->file_upload_dir.$this->file_name) && $this->file_upload_successful == 1 && !$this->file_overwrite == 1)
        {   
            $this->file_upload_successful = 0;
            return $this->UPLOAD_FILENAME_EXISTS;
        }
 
        //check filename validity
        if ( $this->filename_to_md5 == 1)
        {}
        else
        {
            if ((preg_match("/^([a-z0-9]+[\040_\-]?)*\.[a-z]{3}$/i",basename($this->file_name))) && $this->file_upload_successful == 1)
                {}
                else
                {
                    $this->file_upload_successful = 0;
                    return $this->UPLOAD_FILENAME_INVALID;
                }
        }
 
        //file size check
        if( $this->file_size > $this->max_filesize && $this->file_upload_successful == 1)
        {
            $this->file_upload_successful = 0;
            return $this->UPLOAD_FILESIZE_LARGE;
        }
 
        //max file dimensions check
        list($this->file_width, $this->file_height) = getimagesize($this->file_temp_name);
        
        if(($this->file_width > $this->max_file_width || $this->file_height > $this->max_file_height) && $this->file_upload_successful == 1)
        {
            $this->file_upload_successful = 0;
            return $this->UPLOAD_FILEDIMENSIONS_LARGE;
        }
 
        //min file dimensions check
        if(($this->file_width < $this->min_file_width || $this->file_height < $this->min_file_height) && $this->file_upload_successful == 1)
        {
            $this->file_upload_successful = 0;
            return $this->UPLOAD_FILEDIMENSIONS_SMALL;
        }
 
        if ($this->file_upload_successful == 1 )
        {
            // FTP UPLOAD //
            if ($this->file_protocol == 'ftp')
            {
                
                //upload via FTP through curl   
                if (function_exists(curl_init))
                {               
                    $curl_handle = curl_init();
                    $fopen_handle = fopen($this->file_temp_name, 'r');
                    curl_setopt($curl_handle, CURLOPT_URL, 'ftp://'.$this->ftp_username.':'.$this->ftp_password.'@'.$this->file_upload_path);
                    curl_setopt($curl_handle, CURLOPT_UPLOAD, 1);
                    curl_setopt($curl_handle, CURLOPT_INFILE, $fopen_handle);
                    curl_setopt($curl_handle, CURLOPT_INFILESIZE, filesize($this->file_temp_name));
                    curl_exec ($curl_handle);
                    $ftp_error = curl_errno($curl_handle);
                    curl_close($curl_handle);
                    fclose($fopen_handle);
                
                    if ($ftp_error == 0) 
                        {           
                        
                                //check real filetype by using fileInfo
                                if (function_exists(finfo_open) && isset($this->file_allowed_mime_types))
                                {       
                                        //start fileinfo connection
                                        $finfo = finfo_open(FILEINFO_MIME);
 
                                        if (!$finfo) 
                                        {
                                            //there was an error with fileInfo still delete the file uploaded because it could be harmful
                                            //unlink($this->file_upload_path);
                                            return $this->UPLOAD_FILEINFO_ERROR;
                                        }
 
                                        //get real absolute path to check the file
                                        $absupdir = realpath($this->file_upload_dir);
                                        $get_mimetype = finfo_file($finfo, $absupdir.'/'.$this->file_name);
 
                                        $get_mimetype = explode(';', $get_mimetype);
 
                                        //close fileinfo connection
                                        finfo_close($finfo);
 
                                        if (!in_array($get_mimetype[0], $this->file_allowed_mime_types))
                                        {
                                            //suspicious file, delete it from server
                                            unlink($absupdir.'/'.$this->file_name);
                                            return $this->UPLOAD_MIMETYPE_INVALID;
                                        }
                    
 
                                }
                        
                        
                            if ($this->file_overwrite == 1)
                            {
                                return $this->UPLOAD_SUCCESS_OVERWRITTEN;
                            }
                            else
                            {
                                return $this->UPLOAD_SUCCESS;
                            }
                        } 
                        else 
                        {
                            return $this->UPLOAD_ERROR;
                        }
                }
                else
                {
                    return $this->UPLOAD_CURL_ERROR;
                }
        
            }
            else
            {
                
                // HTTP UPLOAD //
                //move the file to the upload folder
                if (move_uploaded_file($this->file_temp_name, $this->file_upload_path) && $this->file_error == 0)
                {
            
                    //check real filetype by using fileInfo
                    if (function_exists(finfo_open) && isset($this->file_allowed_mime_types))
                        {       
                        //start fileinfo connection
                        $finfo = finfo_open(FILEINFO_MIME);
 
                            if (!$finfo) 
                            {
                                //there was an error with fileInfo still delete the file uploaded because it could be harmful
                                //unlink($this->file_upload_path);
                                return $this->UPLOAD_FILEINFO_ERROR;
                            }
 
                            //get real absolute path to check the file
                            $absupdir = realpath($this->file_upload_dir);
                            $get_mimetype = finfo_file($finfo, $absupdir.'/'.$this->file_name);
                    
                            $get_mimetype = explode(';', $get_mimetype);
 
                            //close fileinfo connection
                            finfo_close($finfo);
 
                            if (!in_array($get_mimetype[0], $this->file_allowed_mime_types))
                            {
                                //suspicious file, delete it from server
                                unlink($this->file_upload_path);
                                return $this->UPLOAD_MIMETYPE_INVALID;
                            }
                    
 
                        }
                    
            
                if ($this->file_overwrite == 1)
                    {
                        return $this->UPLOAD_SUCCESS_OVERWRITTEN;
                    }
                    else
                    {
                        return $this->UPLOAD_SUCCESS;
                    }
 
                }
                else
                {
                    //return file upload error
                    switch($this->file_error)
                    {
                    case 1:
                        return $this->UPLOAD_FILESIZE_LARGE_INI;
                    case 2:
                        return $this->UPLOAD_FILESIZE_LARGE_FORM;
                    break;
                    case 3:
                        return $this->UPLOAD_PARTIAL_ONLY;
                    break;
                    case 4:
                        return $this->UPLOAD_NO_FILE;
                    break;
                    case 5:
                        return $this->UPLOAD_NO_TEMPDIR;
                    break;
                    case 6:
                        return $this->UPLOAD_NO_WRITE;
                    break;
                    case 7:
                        return $this->UPLOAD_EXTENSION_STOP;
                    break;
                    default:
                        return $this->UPLOAD_ERROR; 
                    }
                
                }
            
            }
 
        }
        else
        {
            return $this->UPLOAD_ERROR;
        }
    }
 
}
 
?>
Last edited by Sindarin on Sat Oct 24, 2009 11:33 am, edited 8 times in total.
User avatar
jackpf
DevNet Resident
Posts: 2119
Joined: Sun Feb 15, 2009 7:22 pm
Location: Ipswich, UK

Re: File Upload Class, first time OOP

Post by jackpf »

Learn to indent 8O

Also, "var" is PHP 4.0 stylee. You should use public, private or protected instead.
User avatar
Sindarin
Forum Regular
Posts: 521
Joined: Tue Sep 25, 2007 8:36 am
Location: Greece

Re: File Upload Class, first time OOP

Post by Sindarin »

:P the text editor messed my tabs.

edit: fixed
Also, "var" is PHP 4.0 stylee. You should use public, private or protected instead.
will do.
User avatar
pickle
Briney Mod
Posts: 6445
Joined: Mon Jan 19, 2004 6:11 pm
Location: 53.01N x 112.48W
Contact:

Re: File Upload Class, first time OOP

Post by pickle »

  • Use camel case (FileUploader instead of file_uploader, and fileUpload() instead of file_upload())
  • Make your $UPLOAD_* variables class constants, or at the very least make them private. There should be no need to edit them. Declaring them as constants will also allow you to reference them statically (FileUploader::UPLOAD_*) - I think.
  • Replace the file_upload() (PHP4) function with __construct() (PHP5). It behaves exactly the same, but __construct() is the way of the future
  • Define defaults for all the properties you are requiring for the file_upload() function. Also, set default values in the arguments list: file_upload($max_filesize=FALSE, ...), so that if the user doesn't provide that value, or sets it to FALSE, you can load the default set in the class definition
  • You don't need to strip_tags() from the file temp name or size, as those come from your server, not the client
  • Relying on the file extension to determine what type of file you've got - is not very safe at all. I can easily change the extension of my file to whatever I want. Look into the FileInfo extension if you can.
  • I find it dangerous to default a success variable ($this->file_upload_successful) to true. I usually default mine to false, then only set it to true if everything is in order
  • Your file_upload() function is HUGE. I had a professor in university that said functions should be no longer than 30 lines. Sometimes that not practical, but I have used that advice to realize when I need to refactor.
  • What's the purpose of MD5-ing the filename with some random text in it? You won't be able to reproduce that hash, which is usually what MD5-ing is for.
  • Does mt_rand() produce floats? I thought it only produced integers, which would make the floor() call superfluous
Real programmers don't comment their code. If it was hard to write, it should be hard to understand.
User avatar
jackpf
DevNet Resident
Posts: 2119
Joined: Sun Feb 15, 2009 7:22 pm
Location: Ipswich, UK

Re: File Upload Class, first time OOP

Post by jackpf »

Relying on the file extension to determine what type of file you've got - is not very safe at all. I can easily change the extension of my file to whatever I want. Look into the FileInfo extension if you can.
Just wondering, how would you be able to do that?
User avatar
pickle
Briney Mod
Posts: 6445
Joined: Mon Jan 19, 2004 6:11 pm
Location: 53.01N x 112.48W
Contact:

Re: File Upload Class, first time OOP

Post by pickle »

Just rename the file:

mynastyvirus.vba => cuteponies.jpg
Real programmers don't comment their code. If it was hard to write, it should be hard to understand.
User avatar
Sindarin
Forum Regular
Posts: 521
Joined: Tue Sep 25, 2007 8:36 am
Location: Greece

Re: File Upload Class, first time OOP

Post by Sindarin »

Use camel case (FileUploader instead of file_uploader, and fileUpload() instead of file_upload())
Hmm, I am more comfortable with lowercare and underscores :( Can't I just rename the functions like mycool_file_upload() instead?
Make your $UPLOAD_* variables class constants, or at the very least make them private. There should be no need to edit them. Declaring them as constants will also allow you to reference them statically (FileUploader::UPLOAD_*) - I think.
I converted them to constants using public const, do I gain memory by changing them to constants?
Replace the file_upload() (PHP4) function with __construct() (PHP5). It behaves exactly the same, but __construct() is the way of the future
I got confused in that one, did you mean to replace file_uploader() [the first function] with __construct?
Define defaults for all the properties you are requiring for the file_upload() function. Also, set default values in the arguments list: file_upload($max_filesize=FALSE, ...), so that if the user doesn't provide that value, or sets it to FALSE, you can load the default set in the class definition
ok done, so setting the function arguments to false retrieves the values from the class defaults?
You don't need to strip_tags() from the file temp name or size, as those come from your server, not the client
I was just being panaroid here. :D
Relying on the file extension to determine what type of file you've got - is not very safe at all. I can easily change the extension of my file to whatever I want. Look into the FileInfo extension if you can.
hmm, okay I will. However yes I noticed that I could change the extension of an .exe to .png and it would be uploaded with no problem. Then I tried to use $_FILES['file']['type'] to get the mime type. But again the disguised .exe file uploaded with a mimetype of image/png although it wasn't. Does FileInfo actually detect the correct mimetype even with false extension?
Your file_upload() function is HUGE. I had a professor in university that said functions should be no longer than 30 lines. Sometimes that not practical, but I have used that advice to realize when I need to refactor.
No offence intended but what kind of tiny widgets was that professor making? D: Nowadays programs have a huge load of code in their functions, and the more features there are in a program the more lines of code are required. Sure i could make the function in less than 30 lines, but would it have the same features or security? What I can do though is reduce the size of possible junk code, that's why I am asking for comments. :)
I find it dangerous to default a success variable ($this->file_upload_successful) to true. I usually default mine to false, then only set it to true if everything is in order
making it private would help? If any file checking should fail it turns to zero so the file upload isn't done. It's merely there for checking.
What's the purpose of MD5-ing the filename with some random text in it? You won't be able to reproduce that hash, which is usually what MD5-ing is for.
Imagine a user uploads car.jpg which is the md5 of e6d96502596d7e7887b76646c5f615d9. Now a second user uploads a car.jpg. So the md5 is the same "e6d96502596d7e7887b76646c5f615d9" and overwrites the first one. That's why I added the random number at the end.
Does mt_rand() produce floats? I thought it only produced integers, which would make the floor() call superfluous
I needed a cup of coffee... :D

mynastyvirus.vba => cuteponies.jpg
but if it has a .jpg extension won't it be served as image/jpeg and not execute when I visit e.g. http://www.mysite.com/uploads/cuteponies.jpg ?

Thanks for your advice so far! :D
User avatar
jackpf
DevNet Resident
Posts: 2119
Joined: Sun Feb 15, 2009 7:22 pm
Location: Ipswich, UK

Re: File Upload Class, first time OOP

Post by jackpf »

pickle wrote:Just rename the file:

mynastyvirus.vba => cuteponies.jpg
But then it wouldn't execute. Also, the uploads directory shouldn't have executable permissions...so I don't see how that's a security risk, unless you're somehow able to include the file in another script...which shouldn't be possible.
User avatar
John Cartwright
Site Admin
Posts: 11470
Joined: Tue Dec 23, 2003 2:10 am
Location: Toronto
Contact:

Re: File Upload Class, first time OOP

Post by John Cartwright »

jackpf wrote:
pickle wrote:Just rename the file:

mynastyvirus.vba => cuteponies.jpg
But then it wouldn't execute. Also, the uploads directory shouldn't have executable permissions...so I don't see how that's a security risk, unless you're somehow able to include the file in another script...which shouldn't be possible.
Defense in depth, good sir.

You never know if a system is set to run images through the php processor. For instance, if someone may have set all jpg extensions because they are dynamically feeding images through a script.
User avatar
jackpf
DevNet Resident
Posts: 2119
Joined: Sun Feb 15, 2009 7:22 pm
Location: Ipswich, UK

Re: File Upload Class, first time OOP

Post by jackpf »

Oh right...I see. Fair enough :)
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Re: File Upload Class, first time OOP

Post by Christopher »

- I agree with the comments about the file naming ... which is pretty old-school PHP4 style.

- I think you could use more setters and have reasonable defaults -- rather than all those constructor parameters.

- I think the upload method should return 0 for success (not 9/10) and the error number for an error. I would prefer if upload() returned true for success and false for failure. Then have a getError() method. It would be nice to have a getErrorMsg() method that would give you a text error message in addition to the error number.

- A couple of warnings:

Line 30 in index.php (which does not matter that much) should be something like:

Code: Select all

if (isset($_GET['action']) && (strip_tags($_GET['action']) == 'upload'))
Line 100 in upload-file-class.php (which does matter) should be something like:

Code: Select all

           $this->file_overwrite = isset($_POST[$this->file_overwrite_name]) ? intval($_POST[$this->file_overwrite_name]) : 0;
 
(#10850)
User avatar
Sindarin
Forum Regular
Posts: 521
Joined: Tue Sep 25, 2007 8:36 am
Location: Greece

Re: File Upload Class, first time OOP

Post by Sindarin »

$this->file_overwrite = isset($_POST[$this->file_overwrite_name]) ? intval($_POST[$this->file_overwrite_name]) : 0;
I haven't seen this syntax before, could you tell me what ? and :0 does?

Also I saw PHP has an ['error'] output for files, but I can't get the MAX_FILE_SIZE html thing work on forms, it seems the file needs to be uploaded first to check for size. Anyone has got MAX_FILE_SIZE work?

Finally, I looked into the fileInfo extension but didn't work on my server. Guess it's not globally supported yet.
User avatar
jackpf
DevNet Resident
Posts: 2119
Joined: Sun Feb 15, 2009 7:22 pm
Location: Ipswich, UK

Re: File Upload Class, first time OOP

Post by jackpf »

Sindarin wrote:
$this->file_overwrite = isset($_POST[$this->file_overwrite_name]) ? intval($_POST[$this->file_overwrite_name]) : 0;
I haven't seen this syntax before, could you tell me what ? and :0 does?
Google the ternary operator ;)
Also I saw PHP has an ['error'] output for files, but I can't get the MAX_FILE_SIZE html thing work on forms, it seems the file needs to be uploaded first to check for size. Anyone has got MAX_FILE_SIZE work?
It should just work by placing it as a hidden input in the form. Although, don't rely on it since it can easily be modified.
User avatar
Sindarin
Forum Regular
Posts: 521
Joined: Tue Sep 25, 2007 8:36 am
Location: Greece

Re: File Upload Class, first time OOP

Post by Sindarin »

It should just work by placing it as a hidden input in the form. Although, don't rely on it since it can easily be modified.
Oh, I just need it for quick verification so that the client won't wait for the entire file to upload to tell him it's too big in size, there will be more filesize checking after that.
Do I need to $_POST MAX_FILE_SIZE as well for it to work or PHP receives it automatically?

oh btw fileInfo worked and it's just awesome! :D

I noticed something though, when trying to upload files larger than 128 MB which is the max on my php.ini, PHP will throw a warning:
Warning: POST Content-Length of 171168318 bytes exceeds the limit of 134217728 bytes in Unknown on line 0
how do I catch this?
User avatar
Sindarin
Forum Regular
Posts: 521
Joined: Tue Sep 25, 2007 8:36 am
Location: Greece

Re: File Upload Class, first time OOP

Post by Sindarin »

Updated the first post, changed many things as recommended and also added some new features. :)
Post Reply