milesj / uploader

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

FileValidation for files without extension not working #161

Closed johannesnagl closed 10 years ago

johannesnagl commented 10 years ago

the following url is a perfectly valid image: http://images02.kurier.at/46-56793099.jpg/kurier-slideshow-slide/37.069.210

unfortunately, the url is somehow "special".

since it ends with .210, i can't use an extension check for this upload. so i skipped this part. the more robust solution would be to use "type" => "image" as file validation.

unforunately, this check is also broken on the current master, because of the following lines:

public function type() {
    return $this->_cache(__FUNCTION__, function($file) {
        /** @type \Transit\File $file */

        $type = null;

        // We can't use the file command on windows
        if (!defined('PHP_WINDOWS_VERSION_MAJOR')) {
            $type = shell_exec(sprintf("file -b --mime %s", escapeshellarg($file->path())));

            if ($type && strpos($type, ';') !== false) {
                $type = strstr($type, ';', true);
            }
        }

        // Fallback because of fileinfo bug: https://bugs.php.net/bug.php?id=53035
        if (!$type) {
            $info = finfo_open(FILEINFO_MIME_TYPE);
            $type = finfo_file($info, $file->path());
            finfo_close($info);
        }

        // Check the mimetype against the extension
        // If they are different, use the extension since fileinfo returns invalid mimetypes
        // This could be problematic in the future, but unknown better alternative

        if ($ext = $file->ext()) {
            try {
                $extType = MimeType::getTypeFromExt($ext);

            // Use $_FILES['type'] last since sometimes it returns application/octet-stream and other mimetypes
            // When we should always have a true mimetype
            } catch (InvalidArgumentException $e) {
                $extType = $file->data('type');
            }

            if ($type !== $extType) {
                $type = $extType;
            }
        }

        return $type;
    });
}

the current code's fine and checks finfo_file() correctly. unfortunately, when we try to getTypeFromExt() $extType is null (php thinks file extension is .210). because of the fact, that $type !== $extType, the correct $type value gets overwritten with a blank value.

solution would be to check, if extType is !empty. what do you think? is this fix generic enough for you?

milesj commented 10 years ago

Yeah that fix seems reasonable. Will make the change EOD.

milesj commented 10 years ago

Fixed in Transit 1.4.6.