Support for composite file extension validation

[darkdef]

Q	A
Is bugfix?	✔️
New feature?	✔️
Breaks BC?	✔️/❌
Tests pass?	✔️/❌
Fixed issues	#18094

Support for composite file extension validation (#18094)

[samdark]

@saidbakr would you please test it?

[saidbakr]

@saidbakr would you please test it?

I will test as soon as possible today.

[saidbakr]

@saidbakr would you please test it?

Please, could you tell me how to update Yii to the latest development version. I use basic app and I have tried, composer update and composer update yii2. Is there any possible changes to the dependencies in composer.json?

{
    "name": "yiisoft/yii2-app-basic",
    "description": "Yii 2 Basic Project Template",
    "keywords": ["yii2", "framework", "basic", "project template"],
    "homepage": "http://www.yiiframework.com/",
    "type": "project",
    "license": "BSD-3-Clause",
    "support": {
        "issues": "https://github.com/yiisoft/yii2/issues?state=open",
        "forum": "http://www.yiiframework.com/forum/",
        "wiki": "http://www.yiiframework.com/wiki/",
        "irc": "irc://irc.freenode.net/yii",
        "source": "https://github.com/yiisoft/yii2"
    },
    "minimum-stability": "stable",
    "require": {
        "php": ">=5.6.0",
        "yiisoft/yii2": "~2.0.14",
        "yiisoft/yii2-bootstrap": "~2.0.0",
        "yiisoft/yii2-swiftmailer": "~2.0.0 || ~2.1.0"
    },
    "require-dev": {
        "yiisoft/yii2-debug": "~2.1.0",
        "yiisoft/yii2-gii": "~2.1.0",
        "yiisoft/yii2-faker": "~2.0.0",
        "codeception/codeception": "^4.0",
        "codeception/verify": "~0.5.0 || ~1.1.0",
        "codeception/specify": "~0.4.6",
        "symfony/browser-kit": ">=2.7 <=4.2.4",
        "codeception/module-filesystem": "^1.0.0",
        "codeception/module-yii2": "^1.0.0",
        "codeception/module-asserts": "^1.0.0"
    },....

[My6UoT9]

@saidbakr u can try:
"yiisoft/yii2": "dev-master", instead of "yiisoft/yii2": "~2.0.14",
then it should get the latest version, but I did not test that.

[darkdef]

"yiisoft/yii2": "dev-master"

It's valid, but this changes is not merged into dev-master

[saidbakr]

I have updated it to Yii version 2.0.36-dev by changing the dependency in composer.json for Yii2 from "yiisoft/yii2": "~2.0.14" to "yiisoft/yii2": "2.0.x-dev" and then running composer update yiisoft/yii2. I think it should be updated to the latest development version.

Now, the issue, as my understand, still the same. The following is the validation rule of ContactForm model:

class ContactForm extends Model
{
    public $name;
    public $email;
    public $subject;
    public $body;
    public $filen;
    public $verifyCode;


    /**
     * @return array the validation rules.
     */
    public function rules()
    {
        return [
            
            [['filen'], 'file', 'extensions' => 'br.fox, png'],
            
        ];
    }

When I try to upload file named a.br.fox the validation returns error

[samdark]

@saidbakr no, it's not dev version. It is a fork. Either you can try specifying it or do a hot-patch manually by overwriting files in vendor (just for testing).

[saidbakr]

@saidbakr no, it's not dev version. It is a fork. Either you can try specifying it or do a hot-patch manually by overwriting files in vendor (just for testing).

I have copied the two files to there paths in vendor/yiisoft, changed the server port php -S localhost:7075 from 7070 and clearing all cache by deleting runtime/cache and web/assets contents.
The client-side JavaScript validation works fine, but after submit the server-side validation returns the same error

[samdark]

At least something :)

[samdark]

@saidbakr would you please share your validator config?

[saidbakr]

@saidbakr would you please share your validator config?

It is in a previous comment. However, it is here again:

/**
     * @return array the validation rules.
     */
    public function rules()
    {
        return [
            
            [['filen'], 'file', 'extensions' => 'br.fox, png'],
            
        ];
    }

[samdark]

Would you please add:

'checkExtensionByMimeType' => false

as we've discussed in the issue?

[saidbakr]

Would you please add:

'checkExtensionByMimeType' => false

as we've discussed in the issue?

Oh no, I am sorry, I forgot it!
I added it and it works fine now. The file has been uploaded successfully.

/**
     * @return array the validation rules.
     */
    public function rules()
    {
        return [
            
            [['filen'], 'file', 'extensions' => 'br.fox, png', 'checkExtensionByMimeType' => false],
            
        ];
    }

However, we may have to make an Ajax validation to reflect the absence of checkExtensionByMimeType in the client-side. In other words, the JavaScript validation before submit should has any way to check the mimetype from the server before submit too. if 'checkExtensionByMimeType' => true

[saidbakr]

Another note: is there any problem to change the property name from checkExtensionByMimeType to checkMimeType?

[samdark]

@saidbakr that's not the same thing:

/**
     * @var bool whether to check file type (extension) with mime-type. If extension produced by
     * file mime-type check differs from uploaded file extension, the file will be considered as invalid.
     */
    public $checkExtensionByMimeType = true;

Setting it to false doesn't mean that mime type won't be checked in case you specify $mimeTypes. Actually, you should specify it.

[samdark]

However, we may have to make an Ajax validation to reflect the absence of checkExtensionByMimeType in the client-side.

Do you mean AJAX or client validation?

[saidbakr]

However, we may have to make an Ajax validation to reflect the absence of checkExtensionByMimeType in the client-side.

Do you mean AJAX or client validation?

I don't think that JavaScript as a client side could able to check mimetype. So I regarded Ajax. The current implementation tells that JavaScript just check the extension as a string only.

[samdark]

I don't think that JavaScript as a client side could able to check mimetype

It is currently doing that :) https://github.com/yiisoft/yii2/blob/master/framework/assets/yii.validation.js#L419

[saidbakr]

I don't think that JavaScript as a client side could able to check mimetype

It is currently doing that :) https://github.com/yiisoft/yii2/blob/master/framework/assets/yii.validation.js#L419

Very good, the last update of yii.validation.js you have mentioned is doing it in both sides. However, the error message, I think, should be improved to be more descriptive. i.e I have the following message:

Only files with these extensions are allowed: br.fox, png.

I think in the case of checkExtensionByMimeType => true the error message should be something like:

Only files with these mime types are allowed: br.fox, png.

[saidbakr]

Side Note
In such cases of validation rules that may depend on any client-side scripting, the developer should be mentioned, in the documentation, to clear web/assets contents, or preventing assets caching from the configuration file under components:

'assetManager' => [
            'class' => 'yii\web\AssetManager',
            'forceCopy' => true, //Prevent caching assets during development
            'linkAssets' => true, //Prevent caching assets during development
]

Without clearing web/assets, the change of checkExtensionByMimeType from true to false or vice versa will not be reflected on the client-side directly.

[samdark]

checkExtensionByMimeType does not affect how frontend works at all. What it does when turned on is:

1. For an extension a common mime type is determined based on config https://github.com/yiisoft/yii2/blob/master/framework/helpers/mimeTypes.php.
2. Extension is checked. If it's valid then mime-type from point 1 is checked in the file.

In case it's turned off, mime-type isn't automatically checked but you can still specify it via mimeTypes.

[samdark]

@rob006 would you please take a look at the code?

[rob006]

Tests are missing.

[yii-bot]

Thank you for putting effort in the improvement of the Yii framework.
We have reviewed your pull request.

In order for the framework and your solution to remain stable in the future, we have a unit test requirement in place. Therefore we can only accept your pull request if it is covered by unit tests.

Could you add these please?

Thanks!

P.S. If you have any questions about the creation of unit tests? Don't hesitate to ask for support. More information about unit tests

This is an automated comment, triggered by adding the label pr:request for unit tests.

[saidbakr]

checkExtensionByMimeType does not affect how frontend works at all. What it does when turned on is:

1. For an extension a common mime type is determined based on config https://github.com/yiisoft/yii2/blob/master/framework/helpers/mimeTypes.php.

2. Extension is checked. If it's valid then mime-type from point 1 is checked in the file.

In case it's turned off, mime-type isn't automatically checked but you can still specify it via mimeTypes.

Let me explain the issue in other words:

1 Starting with validation below in the model:

return [
            [['filen'], 'file', 'extensions' => 'br.fox, png', 'checkExtensionByMimeType' => true],
]

2 Trying to upload file named a.br.fox, there is no any error rendered before submit but after submit, an error is returned and the file does not be uploaded.

[samdark]

That is expected. We don't have a list of mime types on the client so leaving that to server validation.

[samdark]

Thanks!