Fixed order or checks in IpValidator

Fixes #13198

framework/CHANGELOG.md

@@ -32,6 +32,7 @@ Yii Framework 2 Change Log
 - Bug #13118: Fixed `handleAction()` function in `yii.js` to handle attribute `data-pjax=0` as disabled PJAX (silverfire)
 - Bug #13128: Fixed incorrect position of {pos} string in ColumnSchemaBuilder `__toString` (df2)
 - Bug #13105: Fixed `validate()` method in `yii.activeForm.js` to prevent unexpected form submit when `forceValidate` set to `true` (silverfire)
+- Bug #13198: Fixed order of checks in `yii\validators\IpValidator` that sometimes caused wrong error message (silverfire)
 - Enh #475: Added Bash and Zsh completion support for the `./yii` command (cebe, silverfire)
 - Enh #6242: Access to validator in inline validation (arogachev)
 - Enh #6373: Introduce `yii\db\Query::emulateExecution()` to force returning an empty result for a query (klimov-paul)

framework/assets/yii.validation.js

@@ -344,19 +344,19 @@ yii.validation = (function ($) {
 
             var ipVersion = value.indexOf(':') === -1 ? 4 : 6;
             if (ipVersion == 6) {
-                if (!options.ipv6) {
-                    pub.addMessage(messages, options.messages.ipv6NotAllowed, value);
-                }
                 if (!(new RegExp(options.ipv6Pattern)).test(value)) {
                     pub.addMessage(messages, options.messages.message, value);
                 }
-            } else {
+                if (!options.ipv6) {
-                if (!options.ipv4) {
+                    pub.addMessage(messages, options.messages.ipv6NotAllowed, value);
-                    pub.addMessage(messages, options.messages.ipv4NotAllowed, value);
                 }
+            } else {
                 if (!(new RegExp(options.ipv4Pattern)).test(value)) {
                     pub.addMessage(messages, options.messages.message, value);
                 }
+                if (!options.ipv4) {
+                    pub.addMessage(messages, options.messages.ipv4NotAllowed, value);
+                }
             }
         }
     };

framework/validators/IpValidator.php

@@ -369,12 +369,12 @@ class IpValidator extends Validator
                 $cidr = static::IPV6_ADDRESS_LENGTH;
             }
 
-            if (!$this->ipv6) {
-                return [$this->ipv6NotAllowed, []];
-            }
             if (!$this->validateIPv6($ip)) {
                 return [$this->message, []];
             }
+            if (!$this->ipv6) {
+                return [$this->ipv6NotAllowed, []];
+            }
 
             if ($this->expandIPv6) {
                 $ip = $this->expandIPv6($ip);
@@ -388,13 +388,12 @@ class IpValidator extends Validator
                 $isCidrDefault = true;
                 $cidr = static::IPV4_ADDRESS_LENGTH;
             }
-
-            if (!$this->ipv4) {
-                return [$this->ipv4NotAllowed, []];
-            }
             if (!$this->validateIPv4($ip)) {
                 return [$this->message, []];
             }
+            if (!$this->ipv4) {
+                return [$this->ipv4NotAllowed, []];
+            }
         }
 
         if (!$this->isAllowed($ip, $cidr)) {

tests/framework/validators/IpValidatorTest.php

@@ -61,7 +61,7 @@ class IpValidatorTest extends TestCase
     }
 
     public function provideBadIps() {
-        return [['not.an.ip'], [['what an array', '??']], [123456], [true], [false]];
+        return [['not.an.ip'], [['what an array', '??']], [123456], [true], [false], ['bad:forSure']];
     }
 
     /**
@@ -74,6 +74,37 @@ class IpValidatorTest extends TestCase
         $this->assertFalse($validator->validate($badIp));
     }
 
+    /**
+     * @dataProvider provideBadIps
+     */
+    public function testValidateModelAttributeNotAnIP($badIp)
+    {
+        $validator = new IpValidator();
+        $model = new FakedValidationModel();
+
+        $model->attr_ip = $badIp;
+        $validator->validateAttribute($model, 'attr_ip');
+        $this->assertEquals('attr_ip must be a valid IP address.', $model->getFirstError('attr_ip'));
+        $model->clearErrors();
+
+
+        $validator->ipv4 = false;
+
+        $model->attr_ip = $badIp;
+        $validator->validateAttribute($model, 'attr_ip');
+        $this->assertEquals('attr_ip must be a valid IP address.', $model->getFirstError('attr_ip'));
+        $model->clearErrors();
+
+
+        $validator->ipv4 = true;
+        $validator->ipv6 = false;
+
+        $model->attr_ip = $badIp;
+        $validator->validateAttribute($model, 'attr_ip');
+        $this->assertEquals('attr_ip must be a valid IP address.', $model->getFirstError('attr_ip'));
+        $model->clearErrors();
+    }
+
     public function testValidateValueIPv4()
     {
         $validator = new IpValidator();
@@ -359,4 +390,4 @@ class IpValidatorTest extends TestCase
         $this->assertEquals('fa01::2/614', $model->attr_ip);
         $this->assertEquals('attr_ip contains wrong subnet mask.', $model->getFirstError('attr_ip'));
     }
-}
+}

tests/js/tests/yii.validation.test.js

@@ -1547,6 +1547,8 @@ describe('yii.validation', function () {
         withData({
             'empty string, skip on empty': ['', {skipOnEmpty: true}, []],
             'not IP': ['not IP', {}, ['Invalid value.']],
+            'not IP, IPv4 is disabled': ['not:IP', {ipv4: false}, ['Invalid value.']],
+            'not IP, IPv6 is disabled': ['not IP', {ipv6: false}, ['Invalid value.']],
             // subnet, IPv4
             'IPv4, subnet option is not defined': ['192.168.10.0', {}, []],
             'IPv4, subnet option is set to "false"': ['192.168.10.0', {subnet: false}, []],
@@ -1635,12 +1637,12 @@ describe('yii.validation', function () {
             'invalid IPv4, IPv4 option is set to "false"': [
                 '192,168.10.0',
                 {ipv4: false},
-                ['IPv4 is not allowed.', 'Invalid value.']
+                ['Invalid value.', 'IPv4 is not allowed.']
             ],
             'invalid IPv6, IPv6 option is set to "false"': [
                 '2001,0db8:11a3:09d7:1f34:8a2e:07a0:765d',
                 {ipv6: false},
-                ['IPv6 is not allowed.', 'Invalid value.']
+                ['Invalid value.', 'IPv6 is not allowed.']
             ]
         }, function (value, customOptions, expectedMessages) {
             it(getValidatorMessage(expectedMessages), function () {