PHP5 Upload Class - GO PHP5!

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
Benjamin
Site Administrator
Posts: 6935
Joined: Sun May 19, 2002 10:24 pm

PHP5 Upload Class - GO PHP5!

Post by Benjamin »

Well since two other members posted upload classes, I figured I'd share one I wrote a few months ago. I have never used it or tested it yet.. but it should be good to go with your own customizations.

Code: Select all

<?php
class upload
{
    private $error_message = null;
    private $config = array('MAX_UPLOAD_SIZE'    => 0,
                            'OVERWRITE_IF_EXIST' => false,
                            'ALLOWED_EXTENSIONS' => array(),
                            'BLOCKED_EXTENSIONS' => array(),
                            'DEFAULT_CHMOD'      => '0644');

    public function __construct()
    {

    }

    public function set_option($key, $value)
    {
        if (isset($this->config[$key]))
        {
            $this->config[$key] = $value;
            return true;
        } else {
            return false;
        }
    }

    public function save_file($file, $destination, $new_file_name = null)
    {
        $this->file            = $file;
        $this->destination     = rtrim($destination, '/') . '/';
        $this->new_file_name   = $new_file_name;

        try {
            $this->check_destination();
            $this->check_file();
            $this->move_file();
            $this->set_permissions();
            return true;
        } catch (Exception $e) {
            if ($e->getCode() == 0)
            {
                $this->error_message = $e->getMessage();
                return false;
            } else {
                trigger_error($e->getMessage(), E_USER_ERROR);
                $this->error_message = 'An internal error has occurred.  Please try again later.';
                return false;
            }
        }
    }

    private function set_permissions()
    {
        if (!chmod($this->file_name, $this->config['DEFAULT_CHMOD']))
        {
            throw new Exception("function chmod returned false while trying to change the permissions of '{$this->file_name}' to {$this->config['DEFAULT_CHMOD']}", 1);
        }
    }

    private function move_file()
    {
        $old = error_reporting(0);

        if (!move_uploaded_file($_FILES[$this->file]['tmp_name'], $this->save_path))
        {
            throw new Exception("function move_uploaded_file returned false while trying to move a file from '{$_FILES[$this->file]['tmp_name']}' to '{$this->save_path}'", 1);
        }

        error_reporting($old);
    }

    private function check_destination()
    {
        if (!is_dir($this->destination))
        {
            throw new Exception("The destination '{$this->destination}' does not appear to be a directory.", 1);
        }

        if (!is_writable($this->destination))
        {
            throw new Exception("The destination '{$this->destination}' is not writable by user " . get_current_user() . '.', 1);
        }

    }

    private function set_file_info()
    {
        $this->file_info = pathinfo($_FILES[$this->file]['name']);
        $this->file_name = (($this->new_file_name !== null) ? $this->new_file_name : $this->file_info['filename']) . ".{$this->file_info['extension']}";
        $this->save_path = $this->destination . $this->file_name;
    }

    private function check_file()
    {
        // is there a file where we are looking for it?
        if (!isset($_FILES[$this->file]))
        {
            throw new Exception("\$_FILES['{$this->file}'] is not populated", 1);
        }
        
        $this->set_file_info();

        // were there any upload errors?
        if ($_FILES[$this->file]['error'] != 0)
        {
            switch ($_FILES[$this->file]['error'])
            {
                case 1:
                    throw new Exception('Uploaded files cannot be greater than ' . number_format(ini_get('upload_max_filesize')) . ' bytes.');
                    break;
                case 2:
                    // this data comes from the user, do not trust it.
                    throw new Exception('The uploaded file is too large. Please try uploading a smaller file.');
                    break;
                case 3:
                    throw new Exception('A network error has caused your upload to be only partially received.  Please try again.');
                    break;
                case 4:
                    throw new Exception('An uploaded file was not received.  Please try again.');
                    break;
                case 6:
                    throw new Exception('A temporary folder for saving uploads does not exist.  Please configure a temporary folder in your php.ini file.', 1);
                    break;
                case 7:
                    throw new Exception('An uploaded file could not be written to disk.  Please verify permissions and free disk space.', 1);
                    break;
                case 8:
                    throw new Exception("Files of the extension {$this->file_info['extension']} have been disallowed.");
                    break;
                default:
                    throw new Exception("An unknown error of type {$_FILES[$this->file]['error']} has occurred during a file upload.");
            }
        }

        // is the file too large?
        if ($this->config['MAX_UPLOAD_SIZE'] > 0 && ($_FILES[$this->file]['size'] > $this->config['MAX_UPLOAD_SIZE']))
        {
            throw new Exception('Uploaded files cannot be greater than ' . number_format($_FILES[$this->file]['size']) . ' bytes.');
        }

        // does the file already exist?
        if (!$this->config['OVERWRITE_IF_EXIST'] && file_exists($this->save_path))
        {
            throw new Exception("The file {$this->file_name} already exists on the server.  Please rename the file.");
        } elseif (file_exists($this->save_path) && !is_writable($this->save_path)) {
            throw new Exception("The file {$this->file_name} cannot be overwritten.  Please rename the file.");
        }

        // does the file have an allowed extension..
        if (count($this->config['ALLOWED_EXTENSIONS']) > 0 && !in_array($this->file_info['extension'], $this->config['ALLOWED_EXTENSIONS']))
        {
            throw new Exception("The file extension {$this->file_info['extension']} is not an allowed upload type.");
        }

        // does the file have a blocked extension..
        if (count($this->config['BLOCKED_EXTENSIONS']) > 0 && (in_array($this->file_info['extension'], $this->config['BLOCKED_EXTENSIONS'])))
        {
            throw new Exception("The file extension {$this->file_info['extension']} is not an allowed upload type.");
        }
    }
}
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Post by Christopher »

It must mean something when code is over 70% exceptions and error messages?
(#10850)
User avatar
Benjamin
Site Administrator
Posts: 6935
Joined: Sun May 19, 2002 10:24 pm

Post by Benjamin »

LOL - That made me laugh.

If there is no error the upload succeeded is one thing that it means. Handling file uploads is interesting because there are quite a few point of failures. Some more likely to occur than others. This class throws two types of exceptions. Exceptions for internal errors, and Exceptions for user errors. I just realized that I either accidentally deleted or failed to add a getter for error_message, although it could just be made public as well.
User avatar
AKA Panama Jack
Forum Regular
Posts: 878
Joined: Mon Nov 14, 2005 4:21 pm

PHP 4/5 version

Post by AKA Panama Jack »

Here is a version that should work on both PHP 4 and 5 without the need to use exceptions. :)

Code: Select all

<?php
class upload
{
	// Public Variables
	var $file;
	var $destination;
	var $new_file_name;
	var $file_info;
	var $file_name;
	var $save_path;

	// Private Variables
	var $_error_message = null;
	var $_config = array('MAX_UPLOAD_SIZE'	=> 0,
							'OVERWRITE_IF_EXIST' => false,
							'ALLOWED_EXTENSIONS' => array(),
							'BLOCKED_EXTENSIONS' => array(),
							'DEFAULT_CHMOD'	  => '0644');

	function set_option($key, $value)
	{
		if (isset($this->_config[$key]))
		{
			$this->_config[$key] = $value;
			return true;
		} else {
			return false;
		}
	}

	function save_file($file, $destination, $new_file_name = null)
	{
		$this->file			= $file;
		$this->destination	 = rtrim($destination, '/') . '/';
		$this->new_file_name   = $new_file_name;

		if($this->_check_destination() && $this->_check_file() && $this->_move_file() && $this->_set_permissions())
		{
			return true;
		} else {
			return false;
		}
	}

	function trigger_error($error_msg, $error_type = E_USER_NOTICE)
	{
		if ($error_type == E_USER_NOTICE)
		{
			$this->_error_message = $error_msg;
		} else {
			trigger_error($error_msg, $error_type);
			$this->_error_message = 'An internal error has occurred.  Please try again later.';
		}
		return false;
	}

	function _set_permissions()
	{
		if (!chmod($this->file_name, $this->_config['DEFAULT_CHMOD']))
		{
			return $this->trigger_error("function chmod returned false while trying to change the permissions of '{$this->file_name}' to {$this->_config['DEFAULT_CHMOD']}", E_USER_ERROR);
		}
		return true;
	}

	function _move_file()
	{
		$old = error_reporting(0);

		if (!move_uploaded_file($_FILES[$this->file]['tmp_name'], $this->save_path))
		{
			return $this->trigger_error("function move_uploaded_file returned false while trying to move a file from '{$_FILES[$this->file]['tmp_name']}' to '{$this->save_path}'", E_USER_ERROR);
		}

		error_reporting($old);
		return true;
	}

	function _check_destination()
	{
		if (!is_dir($this->destination))
		{
			return $this->trigger_error("The destination '{$this->destination}' does not appear to be a directory.", E_USER_ERROR);
		}

		if (!is_writable($this->destination))
		{
			return $this->trigger_error("The destination '{$this->destination}' is not writable by user " . get_current_user() . '.', E_USER_ERROR);
		}

		return true;
	}

	function _set_file_info()
	{
		$this->file_info = pathinfo($_FILES[$this->file]['name']);
		$this->file_name = (($this->new_file_name !== null) ? $this->new_file_name : $this->file_info['filename']) . ".{$this->file_info['extension']}";
		$this->save_path = $this->destination . $this->file_name;
	}

	function _check_file()
	{
		// is there a file where we are looking for it?
		if (!isset($_FILES[$this->file]))
		{
			return $this->trigger_error("\$_FILES['{$this->file}'] is not populated", E_USER_ERROR);
		}
		
		$this->_set_file_info();

		// were there any upload errors?
		if ($_FILES[$this->file]['error'] != 0)
		{
			switch ($_FILES[$this->file]['error'])
			{
				case 1:
					return $this->trigger_error('Uploaded files cannot be greater than ' . number_format(ini_get('upload_max_filesize')) . ' bytes.');
					break;
				case 2:
					// this data comes from the user, do not trust it.
					return $this->trigger_error('The uploaded file is too large. Please try uploading a smaller file.');
					break;
				case 3:
					return $this->trigger_error('A network error has caused your upload to be only partially received.  Please try again.');
					break;
				case 4:
					return $this->trigger_error('An uploaded file was not received.  Please try again.');
					break;
				case 6:
					return $this->trigger_error('A temporary folder for saving uploads does not exist.  Please _configure a temporary folder in your php.ini file.', E_USER_ERROR);
					break;
				case 7:
					return $this->trigger_error('An uploaded file could not be written to disk.  Please verify permissions and free disk space.', E_USER_ERROR);
					break;
				case 8:
					return $this->trigger_error("Files of the extension {$this->file_info['extension']} have been disallowed.");
					break;
				default:
					return $this->trigger_error("An unknown error of type {$_FILES[$this->file]['error']} has occurred during a file upload.");
			}
		}

		// is the file too large?
		if ($this->_config['MAX_UPLOAD_SIZE'] > 0 && ($_FILES[$this->file]['size'] > $this->_config['MAX_UPLOAD_SIZE']))
		{
			return $this->trigger_error('Uploaded files cannot be greater than ' . number_format($_FILES[$this->file]['size']) . ' bytes.');
		}

		// does the file already exist?
		if (!$this->_config['OVERWRITE_IF_EXIST'] && file_exists($this->save_path))
		{
			return $this->trigger_error("The file {$this->file_name} already exists on the server.  Please rename the file.");
		} elseif (file_exists($this->save_path) && !is_writable($this->save_path)) {
			return $this->trigger_error("The file {$this->file_name} cannot be overwritten.  Please rename the file.");
		}

		// does the file have an allowed extension..
		if (count($this->_config['ALLOWED_EXTENSIONS']) > 0 && !in_array($this->file_info['extension'], $this->_config['ALLOWED_EXTENSIONS']))
		{
			return $this->trigger_error("The file extension {$this->file_info['extension']} is not an allowed upload type.");
		}

		// does the file have a blocked extension..
		if (count($this->_config['BLOCKED_EXTENSIONS']) > 0 && (in_array($this->file_info['extension'], $this->_config['BLOCKED_EXTENSIONS'])))
		{
			return $this->trigger_error("The file extension {$this->file_info['extension']} is not an allowed upload type.");
		}
		return true;
	}
}
?>
Last edited by AKA Panama Jack on Thu Jul 12, 2007 1:54 am, edited 4 times in total.
User avatar
Benjamin
Site Administrator
Posts: 6935
Joined: Sun May 19, 2002 10:24 pm

Post by Benjamin »

Don't forget to add return true to all the methods in your if statement. I prefer to avoid making code PHP4 compatible. It's always becomes a hassle in the long run.
User avatar
AKA Panama Jack
Forum Regular
Posts: 878
Joined: Mon Nov 14, 2005 4:21 pm

Post by AKA Panama Jack »

It returns a true on all of the if checks. :)

I prefer to make code PHP 4 and PHP 5 compatible so the same code will run on both instead of getting locked into a single version. Takes less effort and less maintenance in the future. :D
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Post by Christopher »

I would recommend replacing all of the error messages with error codes. I know that none of those messages will be acceptable to my clients ... let me supply my own error messages. Just pass through the system error codes and start your at 100 or something. If you want to have a separate error message class/function that converts codes to your messages fine -- just don't make me use it. Getting rid of all of those strings will also make the code much more readable.
(#10850)
User avatar
Benjamin
Site Administrator
Posts: 6935
Joined: Sun May 19, 2002 10:24 pm

Post by Benjamin »

arborint wrote:I would recommend replacing all of the error messages with error codes.
That's not a bad idea. I considered that, but for my use I can simply customize them to my liking. If I added error codes I would have to write documentation, which isn't really something I have time to do at this point.

offtopic @ AKA Panama Jack - If a function doesn't return anything, and putting that return value of nothing into a conditional evaluates to true, I would consider that a bug in PHP. Why? Because I am a very linear and verbose person. null, 0, false, and empty strings all evaluate to false, and a return value of nothing is well.. NULL in my opinion. I certainly wouldn't rely on that sort of language specific behavior in code that I write.
User avatar
AKA Panama Jack
Forum Regular
Posts: 878
Joined: Mon Nov 14, 2005 4:21 pm

Post by AKA Panama Jack »

astions wrote:If a function doesn't return anything, and putting that return value of nothing into a conditional evaluates to true, I would consider that a bug in PHP.
My goof, I forgot to place the return true; lines at the end of each function.

If you check you will see it has been corrected.

I am one of those people who will not go exclusively PHP 5 because it is too limiting in the scope of servers using it. PHP 5 just hasn't taken off and being exclusively PHP 5 hinders the broad acceptance of your product. Once PHP 4 goes the way of PHP 3 then changing to exclusive PHP 5 programming is prudent but it doesn't look likely for a very, very long time.
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Post by Christopher »

astions wrote:That's not a bad idea. I considered that, but for my use I can simply customize them to my liking. If I added error codes I would have to write documentation, which isn't really something I have time to do at this point.
Just add a helper function or method like:

Code: Select all

function getErrorMessage($error) {
    switch ($error) {
    case 1:
              return 'Uploaded files cannot be greater than ' . number_format(ini_get('upload_max_filesize')) . ' bytes.';
              break;
    case 2:
              // this data comes from the user, do not trust it.
              return 'The uploaded file is too large. Please try uploading a smaller file.';
              break;
     // ...
    }
}
The code is easier to read and the "documentation" is easier to read too.
(#10850)
alex.barylski
DevNet Evangelist
Posts: 6267
Joined: Tue Dec 21, 2004 5:00 pm
Location: Winnipeg

Post by alex.barylski »

Personally I've never liked the idea of a file uploader class, maybe a single function, but not a class.

I think you've gone a little exception crazy. :P

Remember that an exception is just that "an exception" not a rule. Use them sparingly or when normal error handling won't suffice. Exceptions are intended to be used as sort of a last resort to what cannot be handled normally. Throwing an exception when a file cannot be written is pointless because this can be determined by the caller by using is_writable() when a function returns FALSE and indicates failure. Additionally, it's a recoverable operation, as you can change file permissions on the fly.

Exceptions should be used to give context to an otherwise mysterious error (collect information pertinent to the problem which can *only* be collected in that context).

Perhaps throwing only a single exception and letting the caller catch the error, perform a couple checks, change permissions and continue would be better. Although exceptions are not really intended to recovered from, but instead offer a mechanism to crash gracefully. :)

Exceptions and their use are tricky to define in the context of PHP because it's not multi-threaded, you never have race conditions or situations which absolutly require throwing or catching an exception. I think their use is more practical as generalized error checking on a block of code, rather than throwing the exception itself.

Cheers :)
User avatar
Benjamin
Site Administrator
Posts: 6935
Joined: Sun May 19, 2002 10:24 pm

Post by Benjamin »

That's debatable.
alex.barylski
DevNet Evangelist
Posts: 6267
Joined: Tue Dec 21, 2004 5:00 pm
Location: Winnipeg

Post by alex.barylski »

Absolutely. Almost everything in software development is debatable. Those are just some general rules of thumb, I have picked up over the years in working with C++

Nothing more, nothing less. I do believe that exceptions are 'heavy' objects - that is their footprint is not small, they consume resources more than an average object. That is a consideration I factor into my decision making process in whether to use an exception.

Just curious, but if you disagree with the rules I've hilited, what are you terms of use or usage policies on using Exceptions?
User avatar
Ambush Commander
DevNet Master
Posts: 3698
Joined: Mon Oct 25, 2004 9:29 pm
Location: New Jersey, US

Post by Ambush Commander »

With regards to localizing error messages, I'd have to say that a company would probably want to hook it into their existing i18n infrastructure, so a class like this probably would not work as a copy and paste one.
User avatar
Benjamin
Site Administrator
Posts: 6935
Joined: Sun May 19, 2002 10:24 pm

Post by Benjamin »

Ambush Commander wrote:With regards to localizing error messages, I'd have to say that a company would probably want to hook it into their existing i18n infrastructure, so a class like this probably would not work as a copy and paste one.
Right, it's pretty much just a base class to use as a starting point. There really is no one size fits all upload class. I think this is about as close as you could get to it. It's easy to plugin and modify though. Pulling out or changing error messages wouldn't be a difficult task either.
Hockey wrote:Personally I've never liked the idea of a file uploader class, maybe a single function, but not a class.
This thing is VERY simple to use, and if something fails you won't wonder why, it will be in the error message variable. I don't believe you could easily get all the same functionality out of a function
Hockey wrote:I think you've gone a little exception crazy. :P
Maybe this highlights how many things really can go wrong with a file upload.
Hockey wrote:Remember that an exception is just that "an exception" not a rule. Use them sparingly or when normal error handling won't suffice. Exceptions are intended to be used as sort of a last resort to what cannot be handled normally. Throwing an exception when a file cannot be written is pointless because this can be determined by the caller by using is_writable() when a function returns FALSE and indicates failure. Additionally, it's a recoverable operation, as you can change file permissions on the fly.
When I write code I try to write the least amount possible. This generally keeps it simple and easy to debug, even when creating complex applications. AKA Panama Jack modified the class so that it was compatible with PHP 4. This required adding an extra method to the class and loading it full of returns instead of exceptions. What he did was the only other way possible to emulate the logic of exceptions. The only alternative would be to use a switch, but a switch can't cross methods. PHP provides me tools, and I'll use what I feel is the best tool for the job.
Hockey wrote:Exceptions should be used to give context to an otherwise mysterious error (collect information pertinent to the problem which can *only* be collected in that context).
I can't agree. If throwing exceptions provides me with the logic I require, that is what I'll use.
Hockey wrote:Perhaps throwing only a single exception and letting the caller catch the error, perform a couple checks, change permissions and continue would be better. Although exceptions are not really intended to recovered from, but instead offer a mechanism to crash gracefully. :)
Why? I can throw one or a thousand but in any case I know what happened. - Something failed. On failure I want to abort and get back to a known state. I don't want a runaway train thrashing around.
Hockey wrote:Exceptions and their use are tricky to define in the context of PHP because it's not multi-threaded, you never have race conditions or situations which absolutly require throwing or catching an exception. I think their use is more practical as generalized error checking on a block of code, rather than throwing the exception itself.
Don't be scared to use exceptions because you feel they are reserved for fatal error situations. If they are the right tool for the job, use them.
arborint wrote:Just add a helper function or method like....
That would be a fairly simple change. To me it's a matter of preference. I'll probably end up using this in multiple projects, but either way I'll know where to change the error messages.

I think in the end it boils down to this..

Code: Select all

$upload = new upload();
if ($upload->save_file('field_name', '/home/myaccount/public_html/uploads/'))
{
    echo 'File Uploaded';
}
Post Reply