[NFR] Phalcon\Http\Request::hasFiles() returns true, when no files were sent

[alvassin]

How to reproduce:

Prepare form with enctype multipart/form-data and 1 input="file" field.
Then send the form without files.

var_dump($_FILES);
// outputs
array(1) {
  ["file"]=>
  array(5) {
    ["name"]=>
    string(0) ""
    ["type"]=>
    string(0) ""
    ["tmp_name"]=>
    string(0) ""
    ["error"]=>
    int(4)
    ["size"]=>
    int(0)
  }

// controller action context
var_dump($this->request->hasFiles()) // true

In fact, no files were transferred.

[alvassin]

I would suggest in addition to check tmp_name for each file, if it was uploaded.. or smth similar.
Current implementation allows to send form with no actual file and phalcon thinks that there is a file received.

Smth like this:

    /**
     * Check that there are any files received in request
     * @return bool
     */
    public function hasFiles()
    {
        // Check if array is empty
        if (count($_FILES) == 0) {
            return false;
        }

        // Handle cases when array is not empty, 
        // but files are not uploaded
        foreach($_FILES as $name => $file) {
            if (!empty($file['tmp_name'])) {
                return true;
            }
        }
    }

[alvassin]

Same suggestion regarding getUploadedFiles() method.

As it is called get UPLOADED files, not direct _FILES array, perhaps it should be more usable to return only those files, which have tmp_name property (are received).

I don't know if this is the behavior that most developers want, maybe they want to inform the user which files can't be uploaded and with this behavior they can't.

[alvassin]

@phalcon agree with you. Perhaps it is possible to implement method, that will perform a real check then?

[alvassin]

@phalcon from my point of view, this could work like following (yes, i understand that you are not able to change current working functionality, just my opinion).
Request::hasFiles() - checks if _FILES array is not empty
Request::hasUploadedFiles() - checks if any files were really uploaded
Request::getFiles() - returns all info based on _FILES array
Request::getUploadedFiles() - returns only really uploaded files

Changing the current behavior could break applications so that is not feasible.

We could add a boolean param to getUploadedFiles to return those that aren't errored, by default keeping the behavior as it is.