From e92466bbbad11f0fd9f50d9c98aa44554024e890 Mon Sep 17 00:00:00 2001 From: Bizley Date: Thu, 30 May 2024 13:36:12 +0200 Subject: [PATCH] Fix #19855: Fixed `yii\validators\FileValidator` to not limit some of its rules only to array attribute --- framework/CHANGELOG.md | 1 + framework/validators/FileValidator.php | 45 ++--- .../models/FakedValidationTypedModel.php | 18 ++ .../validators/FileValidatorTest.php | 167 +++++++++++++++++- 4 files changed, 193 insertions(+), 38 deletions(-) create mode 100644 tests/data/validators/models/FakedValidationTypedModel.php diff --git a/framework/CHANGELOG.md b/framework/CHANGELOG.md index 0a25c23a24c..8ed5a4669fc 100644 --- a/framework/CHANGELOG.md +++ b/framework/CHANGELOG.md @@ -29,6 +29,7 @@ Yii Framework 2 Change Log - Bug #20141: Update `ezyang/htmlpurifier` dependency to version `4.17` (@terabytesoftw) - Bug #19817: Add MySQL Query `addCheck()` and `dropCheck()` (@bobonov) - Bug #20165: Adjust pretty name of closures for PHP 8.4 compatibility (@staabm) +- Bug #19855: Fixed `yii\validators\FileValidator` to not limit some of its rules only to array attribute (bizley) - Enh: #20171: Support JSON columns for MariaDB 10.4 or higher (@terabytesoftw) 2.0.49.2 October 12, 2023 diff --git a/framework/validators/FileValidator.php b/framework/validators/FileValidator.php index a93814a39ab..90ff0aa3ce5 100644 --- a/framework/validators/FileValidator.php +++ b/framework/validators/FileValidator.php @@ -208,40 +208,23 @@ public function init() */ public function validateAttribute($model, $attribute) { - if ($this->maxFiles != 1 || $this->minFiles > 1) { - $rawFiles = $model->$attribute; - if (!is_array($rawFiles)) { - $this->addError($model, $attribute, $this->uploadRequired); + $files = $this->filterFiles(is_array($model->$attribute) ? $model->$attribute : [$model->$attribute]); + $filesCount = count($files); + if ($filesCount === 0 && $this->minFiles > 0) { + $this->addError($model, $attribute, $this->uploadRequired); - return; - } - - $files = $this->filterFiles($rawFiles); - $model->$attribute = $files; - - if (empty($files)) { - $this->addError($model, $attribute, $this->uploadRequired); - - return; - } - - $filesCount = count($files); - if ($this->maxFiles && $filesCount > $this->maxFiles) { - $this->addError($model, $attribute, $this->tooMany, ['limit' => $this->maxFiles]); - } + return; + } - if ($this->minFiles && $this->minFiles > $filesCount) { - $this->addError($model, $attribute, $this->tooFew, ['limit' => $this->minFiles]); - } + if ($this->maxFiles > 0 && $filesCount > $this->maxFiles) { + $this->addError($model, $attribute, $this->tooMany, ['limit' => $this->maxFiles]); + } + if ($this->minFiles > 0 && $this->minFiles > $filesCount) { + $this->addError($model, $attribute, $this->tooFew, ['limit' => $this->minFiles]); + } - foreach ($files as $file) { - $result = $this->validateValue($file); - if (!empty($result)) { - $this->addError($model, $attribute, $result[0], $result[1]); - } - } - } else { - $result = $this->validateValue($model->$attribute); + foreach ($files as $file) { + $result = $this->validateValue($file); if (!empty($result)) { $this->addError($model, $attribute, $result[0], $result[1]); } diff --git a/tests/data/validators/models/FakedValidationTypedModel.php b/tests/data/validators/models/FakedValidationTypedModel.php new file mode 100644 index 00000000000..39d5795cf72 --- /dev/null +++ b/tests/data/validators/models/FakedValidationTypedModel.php @@ -0,0 +1,18 @@ + 'path']); $val->validateAttribute($m, 'attr_files'); - $this->assertTrue($m->hasErrors('attr_files')); + $this->assertFalse($m->hasErrors('attr_files')); $m = FakedValidationModel::createWithAttributes(['attr_files' => []]); $val->validateAttribute($m, 'attr_files'); - $this->assertTrue($m->hasErrors('attr_files')); - $this->assertSame($val->uploadRequired, current($m->getErrors('attr_files'))); + $this->assertFalse($m->hasErrors('attr_files')); + $m = FakedValidationModel::createWithAttributes( [ 'attr_files' => $this->createTestFiles( @@ -314,6 +315,32 @@ public function testValidateAttribute_minFilesTwoMaxFilesUnlimited_hasError() $this->assertTrue($model->hasErrors('attr_images')); } + /** + * https://github.com/yiisoft/yii2/issues/19855 + */ + public function testValidateArrayAttributeWithMinMaxOneAndOneFile() + { + $validator = new FileValidator(['maxFiles' => 1, 'minFiles' => 0]); + $files = $this->createTestFiles( + [ + [ + 'name' => 'image.png', + 'size' => 1024, + 'type' => 'image/png', + ], + [ + 'name' => 'image.png', + 'size' => 1024, + 'type' => 'image/png', + ], + ] + )[0]; + $model = FakedValidationModel::createWithAttributes(['attr_images' => [$files]]); + + $validator->validateAttribute($model, 'attr_images'); + $this->assertFalse($model->hasErrors('attr_images')); + } + /** * @param array $params * @return UploadedFile[] @@ -395,8 +422,7 @@ public function testValidateAttribute() $val->validateAttribute($m, 'attr_files'); $this->assertFalse($m->hasErrors()); $val->validateAttribute($m, 'attr_files_empty'); - $this->assertTrue($m->hasErrors('attr_files_empty')); - $this->assertSame($val->uploadRequired, current($m->getErrors('attr_files_empty'))); + $this->assertFalse($m->hasErrors('attr_files_empty')); // single File with skipOnEmpty = false $val = new FileValidator(['skipOnEmpty' => false]); @@ -404,8 +430,7 @@ public function testValidateAttribute() $val->validateAttribute($m, 'attr_files'); $this->assertFalse($m->hasErrors()); $val->validateAttribute($m, 'attr_files_empty'); - $this->assertTrue($m->hasErrors('attr_files_empty')); - $this->assertSame($val->uploadRequired, current($m->getErrors('attr_files_empty'))); + $this->assertFalse($m->hasErrors('attr_files_empty')); $m = $this->createModelForAttributeTest(); // too big @@ -672,4 +697,132 @@ public function mimeTypeCaseInsensitive() { ['image/jxra', 'image/jxrA', true], ]; } + + public function testValidateTypedAttributeNoErrors() + { + if (version_compare(PHP_VERSION, '7.4', '<')) { + $this->markTestSkipped('Requires typed properties'); + } + + $validator = new FileValidator(['minFiles' => 0, 'maxFiles' => 2]); + $file = $this->createTestFiles( + [ + [ + 'name' => 'image.png', + 'size' => 1024, + 'type' => 'image/png', + ] + ] + ); + $model = new FakedValidationTypedModel(); + $model->single = $file; + $model->multiple = [$file]; + $validator->validateAttribute($model, 'single'); + $this->assertFalse($model->hasErrors('single')); + $validator->validateAttribute($model, 'multiple'); + $this->assertFalse($model->hasErrors('multiple')); + } + + public function testValidateTypedAttributeExactMinNoErrors() + { + if (version_compare(PHP_VERSION, '7.4', '<')) { + $this->markTestSkipped('Requires typed properties'); + } + + $validator = new FileValidator(['minFiles' => 1]); + $file = $this->createTestFiles( + [ + [ + 'name' => 'image.png', + 'size' => 1024, + 'type' => 'image/png', + ] + ] + ); + $model = new FakedValidationTypedModel(); + $model->single = $file; + $model->multiple = [$file]; + $validator->validateAttribute($model, 'single'); + $this->assertFalse($model->hasErrors('single')); + $validator->validateAttribute($model, 'multiple'); + $this->assertFalse($model->hasErrors('multiple')); + } + + public function testValidateTypedAttributeExactMaxNoErrors() + { + if (version_compare(PHP_VERSION, '7.4', '<')) { + $this->markTestSkipped('Requires typed properties'); + } + + $validator = new FileValidator(['maxFiles' => 1]); + $file = $this->createTestFiles( + [ + [ + 'name' => 'image.png', + 'size' => 1024, + 'type' => 'image/png', + ] + ] + ); + $model = new FakedValidationTypedModel(); + $model->single = $file; + $model->multiple = [$file]; + $validator->validateAttribute($model, 'single'); + $this->assertFalse($model->hasErrors('single')); + $validator->validateAttribute($model, 'multiple'); + $this->assertFalse($model->hasErrors('multiple')); + } + + public function testValidateTypedAttributeMinError() + { + if (version_compare(PHP_VERSION, '7.4', '<')) { + $this->markTestSkipped('Requires typed properties'); + } + + $validator = new FileValidator(['minFiles' => 2]); + $file = $this->createTestFiles( + [ + [ + 'name' => 'image.png', + 'size' => 1024, + 'type' => 'image/png', + ] + ] + ); + $model = new FakedValidationTypedModel(); + $model->single = $file; + $model->multiple = [$file]; + $validator->validateAttribute($model, 'single'); + $this->assertTrue($model->hasErrors('single')); + $validator->validateAttribute($model, 'multiple'); + $this->assertTrue($model->hasErrors('multiple')); + } + + public function testValidateTypedAttributeMaxError() + { + if (version_compare(PHP_VERSION, '7.4', '<')) { + $this->markTestSkipped('Requires typed properties'); + } + + $validator = new FileValidator(['maxFiles' => 1]); + $files = $this->createTestFiles( + [ + [ + 'name' => 'image.png', + 'size' => 1024, + 'type' => 'image/png', + ], + [ + 'name' => 'image.png', + 'size' => 1024, + 'type' => 'image/png', + ] + ] + ); + $model = new FakedValidationTypedModel(); + // single attribute cannot be checked because maxFiles = 0 === no limits + $model->multiple = $files; + $validator->validateAttribute($model, 'multiple'); + $this->assertTrue($model->hasErrors('multiple')); + } }