Regression in FileValidator since Yii 2.0.50 when validating required files

[marcovtwout]

What steps will reproduce the problem?

• Create a form with a validation rule like for a required upload field:

['example', 'file', 'skipOnEmpty' => false],

• Set enableClientValidation = false in the ActiveForm. (client side validation still works as expected and will prevent reproduction of the issue)
• Post the form without selecting any file.

What is the expected result?

A validation error 'Please upload a file'.

What do you get instead?

The validation passes.

Additional info

The regression is introduced by the changes to validateAttribute() in #20167 which were released with Yii 2.0.50. It still works as expected on 2.0.49.

@bizley Since skipOnEmpty is set to false, validateOnAttribute() is called and should still handle the case where no file has been uploaded. The validation should be executed even if $minFiles is still set to the default of 0. Previously, this happened here, but this code is not reached anymore in the given scenario:

yii2/framework/validators/FileValidator.php

Line 258 in 7c5b213

return [$this->uploadRequired, []];

Q	A
Yii version	2.0.50
PHP version	-
Operating system	-

[bizley]

I'm a bit confused because I totally missed that logic. Is it stated only in this vardoc? To be honest I think this is very unclear for the users (= "I require a file and the required number of files must be at least 0"). I would only add upgrade note for this one. @samdark what do you think?

[marcovtwout]

@bizley It's part of the main example in the definitive guide as well: https://www.yiiframework.com/doc/guide/2.0/en/input-file-upload#creating-models

Here's the client side validation for comparison https://github.com/yiisoft/yii2/blob/master/framework/assets/yii.validation.js#L404

Requiring to set minFiles might also confuse some users as its related to functionality you only need when you want the input to deal with multiple files (giving a specific $tooFew error message instead of the $uploadRequired message).

If you go looking in the API for required field functionality without knowing the design, the $uploadRequired parameter documents clearly how to set it. While I agree its not the best design and a bit implicit from the code alone, I'd favor backward compatibility in this case.

[bizley]

I see your point 👍🏻 Do you think changing line 212 to

if ($filesCount === 0 && $this->uploadRequired !== null) {

would be enough?

[marcovtwout]

@bizley I think if ($filesCount === 0) would be enough. The field is defined as string only and similar code that uses validation texts does not check for null either.