Uploading multiple files using a class

Below I have implemented a class that will validate and upload files based on the specified rules.

```
class Uploader
{
protected $finfo;

// extensions corresponding mime types
protected $mimes = [
'jpg' => 'image/jpeg',
'pdf' => 'application/pdf',
'png' => 'image/png',
'gif' => 'image/gif'
];

// default settings
protected $defaults = [
'auto_rename' => true, // rename the uploaded file to a random ID
'new_name' => null, // rename the uploaded file to this name
'ext_whitelist' => null, // mime types corresponding to the types of files allowed to be uploaded
//'max_height' => null, // maximum height (in pixels) that the file can be; null for no limit
'max_size' => null, // maximum size (in megabytes) that the file can be; null for no limit
//'max_width' => null, // maximum width (in pixels) that the file can be; null for no limit
'path' => null, // path to the folder where the upload should be placed
'required' => false // require that file needs to be uploaded
];

// settings for fields to be uploaded
protected $rules = array();

// files that were processed properly
protected $processed = array();

// errors found during processing or uploading
protected $errors = array();

/**
* Initialize uploader.
*/
public function __construct()
{
$this->finfo = new finfo();
}

/**
* Set default settings.
* @param array $params
*/
public function setConfig(array $params)
{
foreach ($this->defaults as $name => $value) {
isset($params[$name]) && $this->defaults[$name] = $params[$name];
}
}

/**
* Set the settings for the fields to be uploaded.
* @param array $params
*/
public function setRules(array $params)
{
foreach ($params as $name => $val

I like your code. The public interface is mostly well thought out, it seems easy to use, the code is mostly well structured and readable, well commented, well formatted, etc.

A couple of issues do exist though:

Security: PHP File Upload

It is possible to bypass finfo check (depending on the system), and thus to upload files with a dangerous type, such as PHP files.

You should get the extension from the file name, not from a mimetype check.

You could add an additional mimetype check, but the extension check needs to be independent of it.

Security: Weak Extension Check

Now, even if your extension check would work, it is rather weak as it doesn't check for equality. Instead, it checks if the extension exists in a given string, which is a weaker check.

For example, a user may want to allow phtml (which doesn't lead to code execution with most current server configurations), but not pht (which does lead to code execution with most current server configurations). The user will set 'ext_whitelist' => 'foobar|phtml', and they will expect that phtml files can be uploaded, and that pht files cannot. But that isn't what happens.

Misc

• process and run are a bit generic. run could probably be upload, and process may be checkValidity or something.

• $defaults should be renamed, to either $defaultConfig or $defaultRules (whichever one is actually correct).

• Do you really need separate process and run methods? It seems to just complicate the interface. Or is there ever a situation in which a user does want to call one, but not the other? If not, just make both private, and add a new upload method which calls both.

• The difference between config and rules isn't quite clear to me. Config seems to be the default rules + the upload path? Or just the default rules? If that is the case, setConfig should really be named setDefaultRules (no reason to confuse a user with config, if there isn't anything to configure except rules). If that's wrong, the interface and documentation should make the difference clearer.

• $rules doesn't just contain rules, but actually contains the file reference + its associated rules. The naming here is a bit confusing. Reading foreach ($this->rules as $name => $rules), it looks as if you were iterating over rules, while you are actually iterating over files.

• magic numbers: they make code hard to understand. What's 1048576? I would save it in a properly named variable, or at a minimum add a comment (or even a function) which makes it clear what is happening here.

StackExchange Code Review Q#125519, answer score: 3