From 67b5f4ea19babffbf4917782b7da9d66468d0140 Mon Sep 17 00:00:00 2001 From: mdmunir Date: Tue, 24 Nov 2015 16:22:23 +0700 Subject: [PATCH 1/3] fix for unsafe validator Closes #8145 #8139 #11153 --- framework/base/Model.php | 2 +- framework/validators/Validator.php | 17 ++++++++++-- tests/framework/base/ModelTest.php | 44 ++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 3 deletions(-) diff --git a/framework/base/Model.php b/framework/base/Model.php index 4d8977f084..03a25f2dd7 100644 --- a/framework/base/Model.php +++ b/framework/base/Model.php @@ -754,7 +754,7 @@ class Model extends Component implements IteratorAggregate, ArrayAccess, Arrayab } $attributes = []; foreach ($scenarios[$scenario] as $attribute) { - if ($attribute[0] !== '!') { + if ($attribute[0] !== '!' && !in_array('!' . $attribute, $scenarios[$scenario])) { $attributes[] = $attribute; } } diff --git a/framework/validators/Validator.php b/framework/validators/Validator.php index e3a37d4893..0570c30eae 100644 --- a/framework/validators/Validator.php +++ b/framework/validators/Validator.php @@ -231,9 +231,22 @@ class Validator extends Component public function validateAttributes($model, $attributes = null) { if (is_array($attributes)) { - $attributes = array_intersect($this->attributes, $attributes); + $newAttributes = []; + foreach ($attributes as $attribute) { + if(in_array($attribute, $this->attributes) || in_array('!' . $attribute, $this->attributes)){ + $newAttributes[] = $attribute; + } + } + $attributes = $newAttributes; } else { - $attributes = $this->attributes; + $attributes = []; + foreach ($this->attributes as $attribute) { + if($attribute[0] === '!'){ + $attributes[] = substr($attribute, 1); + } else { + $attributes[] = $attribute; + } + } } foreach ($attributes as $attribute) { $skip = $this->skipOnError && $model->hasErrors($attribute) diff --git a/tests/framework/base/ModelTest.php b/tests/framework/base/ModelTest.php index 72968d196c..45cb9f04ab 100644 --- a/tests/framework/base/ModelTest.php +++ b/tests/framework/base/ModelTest.php @@ -175,6 +175,50 @@ class ModelTest extends TestCase $model->scenario = 'create'; $this->assertEquals(['account_id', 'user_id', 'email', 'name'], $model->safeAttributes()); $this->assertEquals(['account_id', 'user_id', 'email', 'name'], $model->activeAttributes()); + + $model = new RulesModel(); + $model->rules = [ + [['name','!email'], 'required'], + ]; + $this->assertEquals(['name'], $model->safeAttributes()); + $this->assertEquals(['name', 'email'], $model->activeAttributes()); + $model->attributes = ['name' => 'mdmunir', 'email' => 'mdm@mun.com']; + $this->assertNull($model->email); + $this->assertFalse($model->validate()); + + $model = new RulesModel(); + $model->rules = [ + [['name'], 'required'], + [['!user_id'], 'default', 'value' => '3426'], + ]; + $model->attributes = ['name' => 'mdmunir', 'user_id' => '62792684']; + $this->assertTrue($model->validate()); + $this->assertEquals('3426', $model->user_id); + + $model = new RulesModel(); + $model->rules = [ + [['name', 'email'], 'required'], + [['!email'], 'safe'] + ]; + $this->assertEquals(['name'], $model->safeAttributes()); + $model->attributes = ['name' => 'mdmunir', 'email' => 'm2792684@mdm.com']; + $this->assertFalse($model->validate()); + + $model = new RulesModel(); + $model->rules = [ + [['name', 'email'], 'required'], + [['email'], 'email'], + [['!email'], 'safe', 'on' => 'update'] + ]; + $model->setScenario(RulesModel::SCENARIO_DEFAULT); + $this->assertEquals(['name', 'email'], $model->safeAttributes()); + $model->attributes = ['name' => 'mdmunir', 'email' => 'm2792684@mdm.com']; + $this->assertTrue($model->validate()); + + $model->setScenario('update'); + $this->assertEquals(['name'], $model->safeAttributes()); + $model->attributes = ['name' => 'D426', 'email' => 'd426@mdm.com']; + $this->assertNotEquals('d426@mdm.com', $model->email); } public function testErrors() From 5db2afbaebc43afcfbd9f444d9c903ce0e967236 Mon Sep 17 00:00:00 2001 From: SilverFire - Dmitry Naumenko Date: Tue, 22 Mar 2016 23:02:40 +0200 Subject: [PATCH 2/3] Code style fixed, PHPDoc updated --- framework/validators/Validator.php | 14 +++++--------- tests/framework/base/ModelTest.php | 18 +++++++++++------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/framework/validators/Validator.php b/framework/validators/Validator.php index 0570c30eae..c64d64aa68 100644 --- a/framework/validators/Validator.php +++ b/framework/validators/Validator.php @@ -224,16 +224,15 @@ class Validator extends Component * Validates the specified object. * @param \yii\base\Model $model the data model being validated * @param array|null $attributes the list of attributes to be validated. - * Note that if an attribute is not associated with the validator, - * it will be ignored. - * If this parameter is null, every attribute listed in [[attributes]] will be validated. + * Note that if an attribute is not associated with the validator, or is is prefixed with `!` char - it will be + * ignored. If this parameter is null, every attribute listed in [[attributes]] will be validated. */ public function validateAttributes($model, $attributes = null) { if (is_array($attributes)) { $newAttributes = []; foreach ($attributes as $attribute) { - if(in_array($attribute, $this->attributes) || in_array('!' . $attribute, $this->attributes)){ + if (in_array($attribute, $this->attributes) || in_array('!' . $attribute, $this->attributes)) { $newAttributes[] = $attribute; } } @@ -241,13 +240,10 @@ class Validator extends Component } else { $attributes = []; foreach ($this->attributes as $attribute) { - if($attribute[0] === '!'){ - $attributes[] = substr($attribute, 1); - } else { - $attributes[] = $attribute; - } + $attributes[] = $attribute[0] === '!' ? substr($attribute, 1) : $attribute; } } + foreach ($attributes as $attribute) { $skip = $this->skipOnError && $model->hasErrors($attribute) || $this->skipOnEmpty && $this->isEmpty($model->$attribute); diff --git a/tests/framework/base/ModelTest.php b/tests/framework/base/ModelTest.php index 45cb9f04ab..a6fa22cdd1 100644 --- a/tests/framework/base/ModelTest.php +++ b/tests/framework/base/ModelTest.php @@ -3,11 +3,11 @@ namespace yiiunit\framework\base; use yii\base\Model; -use yiiunit\data\base\RulesModel; -use yiiunit\TestCase; -use yiiunit\data\base\Speaker; -use yiiunit\data\base\Singer; use yiiunit\data\base\InvalidRulesModel; +use yiiunit\data\base\RulesModel; +use yiiunit\data\base\Singer; +use yiiunit\data\base\Speaker; +use yiiunit\TestCase; /** * @group base @@ -175,10 +175,13 @@ class ModelTest extends TestCase $model->scenario = 'create'; $this->assertEquals(['account_id', 'user_id', 'email', 'name'], $model->safeAttributes()); $this->assertEquals(['account_id', 'user_id', 'email', 'name'], $model->activeAttributes()); + } + public function testUnsafeAttributes() + { $model = new RulesModel(); $model->rules = [ - [['name','!email'], 'required'], + [['name', '!email'], 'required'], // Name is safe to set, but email is not. Both are required ]; $this->assertEquals(['name'], $model->safeAttributes()); $this->assertEquals(['name', 'email'], $model->activeAttributes()); @@ -214,7 +217,7 @@ class ModelTest extends TestCase $this->assertEquals(['name', 'email'], $model->safeAttributes()); $model->attributes = ['name' => 'mdmunir', 'email' => 'm2792684@mdm.com']; $this->assertTrue($model->validate()); - + $model->setScenario('update'); $this->assertEquals(['name'], $model->safeAttributes()); $model->attributes = ['name' => 'D426', 'email' => 'd426@mdm.com']; @@ -388,7 +391,8 @@ class ModelTest extends TestCase public function testCreateValidators() { - $this->setExpectedException('yii\base\InvalidConfigException', 'Invalid validation rule: a rule must specify both attribute names and validator type.'); + $this->setExpectedException('yii\base\InvalidConfigException', + 'Invalid validation rule: a rule must specify both attribute names and validator type.'); $invalid = new InvalidRulesModel(); $invalid->createValidators(); From e16f7764062b48c4a2f3ed0bed085582c2f391a0 Mon Sep 17 00:00:00 2001 From: SilverFire - Dmitry Naumenko Date: Tue, 22 Mar 2016 23:29:25 +0200 Subject: [PATCH 3/3] Updated docs --- docs/guide/structure-models.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/docs/guide/structure-models.md b/docs/guide/structure-models.md index e48a0896a7..9c4cefee55 100644 --- a/docs/guide/structure-models.md +++ b/docs/guide/structure-models.md @@ -392,6 +392,19 @@ have to do it explicitly as follows, $model->secret = $secret; ``` +The same can be done in `rules()` method: + +```php +public function rules() +{ + return [ + [['username', 'password', '!secret'], 'required', 'on' => 'login'] + ]; +} +``` + +In this case attributes `username` and `password` are required, but `secret` must be assigned explicitly. + ## Data Exporting