milesj / uploader

[Deprecated] A CakePHP plugin for file uploading and validating.
MIT License
193 stars 73 forks source link

Reset property so it doesn't leak to other models #186

Closed mfn closed 9 years ago

mfn commented 9 years ago

If the FileValidation is used in multiple models which are validated within a single request, since Behaviours are effectively singletons, their internal state leaks over to the next model.

This prevents $_tmpFile still being set for the next model; otherwise this file would be tried to be deleted again in the next model validation which does not exist anymore.


We observed this problem when saving two models successively which both used FileValidation. Only in the first model a file was attached, the second model had no file (but validation rules) and these were executed. The led to the leakage of _tempFile and this a duplicate deletion attempt which failed.

Implementation detail: although the Transit package uses the following code:

    public function delete() {
        $this->reset();

        return @unlink($this->_path);
    }

which actually prevents warnings being generated, this can change with custom error handlers and thus still generate warnings depending on the environment.

milesj commented 9 years ago

Nice catch! Thanks for the fix.

mfn commented 9 years ago

@milesj there have been a few fixes already, would you mind rolling a new release? thanks!

milesj commented 9 years ago

Thanks for reminding me. Will get to it later today.