Skip to content

Commit

Permalink
Merge pull request #8129 from kenjis/fix-validation-dot-array-syntax
Browse files Browse the repository at this point in the history 
fix: Validation rule with `*` gets incorrect values as dot array syntax
  • Loading branch information
@kenjis
kenjis authored Nov 3, 2023
2 parents 37af29e + 4913acc commit 3266127
Show file tree
Hide file tree
Showing 8 changed files with 161 additions and 34 deletions.
41 changes: 28 additions & 13 deletions system/Validation/Validation.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,12 +168,14 @@ public function run(?array $data = null, ?string $group = null, ?string $dbGroup
}

if (strpos($field, '*') !== false) {
$values = array_filter(array_flatten_with_dots($data), static fn ($key) => preg_match(
'/^'
. str_replace(['\.\*', '\*\.'], ['\..+', '.+\.'], preg_quote($field, '/'))
. '$/',
$key
), ARRAY_FILTER_USE_KEY);
$flattenedArray = array_flatten_with_dots($data);

$values = array_filter(
$flattenedArray,
static fn ($key) => preg_match(self::getRegex($field), $key),
ARRAY_FILTER_USE_KEY
);

// if keys not found
$values = $values ?: [$field => null];
} else {
Expand Down Expand Up @@ -211,6 +213,20 @@ public function run(?array $data = null, ?string $group = null, ?string $dbGroup
return false;
}

/**
* Returns regex pattern for key with dot array syntax.
*/
private static function getRegex(string $field): string
{
return '/\A'
. str_replace(
['\.\*', '\*\.'],
['\.[^.]+', '[^.]+\.'],
preg_quote($field, '/')
)
. '\z/';
}

/**
* Runs the validation process, returning true or false determining whether
* validation was successful or not.
Expand Down Expand Up @@ -814,9 +830,7 @@ private function retrievePlaceholders(string $rule, array $data): array
*/
public function hasError(string $field): bool
{
$pattern = '/^' . str_replace('\.\*', '\..+', preg_quote($field, '/')) . '$/';

return (bool) preg_grep($pattern, array_keys($this->getErrors()));
return (bool) preg_grep(self::getRegex($field), array_keys($this->getErrors()));
}

/**
Expand All @@ -829,10 +843,11 @@ public function getError(?string $field = null): string
$field = array_key_first($this->rules);
}

$errors = array_filter($this->getErrors(), static fn ($key) => preg_match(
'/^' . str_replace(['\.\*', '\*\.'], ['\..+', '.+\.'], preg_quote($field, '/')) . '$/',
$key
), ARRAY_FILTER_USE_KEY);
$errors = array_filter(
$this->getErrors(),
static fn ($key) => preg_match(self::getRegex($field), $key),
ARRAY_FILTER_USE_KEY
);

return $errors === [] ? '' : implode("\n", $errors);
}
Expand Down
66 changes: 56 additions & 10 deletions tests/system/Validation/ValidationTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1111,17 +1111,17 @@ public function testRulesForSingleRuleWithAsteriskWillReturnError(): void
$request = new IncomingRequest($config, new URI(), 'php://input', new UserAgent());

$this->validation->setRules([
'id_user.*' => 'numeric',
'name_user.*' => 'alpha',
'contacts.*.name' => 'required',
'id_user.*' => 'numeric',
'name_user.*' => 'alpha',
'contacts.friends.*.name' => 'required',
]);

$this->validation->withRequest($request->withMethod('post'))->run();
$this->assertSame([
'id_user.0' => 'The id_user.* field must contain only numbers.',
'name_user.0' => 'The name_user.* field may only contain alphabetical characters.',
'name_user.2' => 'The name_user.* field may only contain alphabetical characters.',
'contacts.friends.0.name' => 'The contacts.*.name field is required.',
'contacts.friends.0.name' => 'The contacts.friends.*.name field is required.',
], $this->validation->getErrors());

$this->assertSame(
Expand All @@ -1130,8 +1130,8 @@ public function testRulesForSingleRuleWithAsteriskWillReturnError(): void
$this->validation->getError('name_user.*')
);
$this->assertSame(
'The contacts.*.name field is required.',
$this->validation->getError('contacts.*.name')
'The contacts.friends.*.name field is required.',
$this->validation->getError('contacts.friends.*.name')
);
}

Expand Down Expand Up @@ -1228,17 +1228,17 @@ public function testTranslatedLabelTagReplacement(): void
}

/**
* @dataProvider provideDotNotationOnIfExistRule
* @dataProvider provideIfExistRuleWithAsterisk
*
* @see https://github.com/codeigniter4/CodeIgniter4/issues/4521
*/
public function testDotNotationOnIfExistRule(bool $expected, array $rules, array $data): void
public function testIfExistRuleWithAsterisk(bool $expected, array $rules, array $data): void
{
$actual = $this->validation->setRules($rules)->run($data);
$this->assertSame($expected, $actual);
}

public static function provideDotNotationOnIfExistRule(): iterable
public static function provideIfExistRuleWithAsterisk(): iterable
{
yield 'dot-on-end-fail' => [
false,
Expand Down Expand Up @@ -1613,7 +1613,7 @@ public function testRuleWithLeadingAsterisk(): void
/**
* @see https://github.com/codeigniter4/CodeIgniter4/issues/5942
*/
public function testRequireWithoutWithWildCard(): void
public function testRequireWithoutWithAsterisk(): void
{
$data = [
'a' => [
Expand All @@ -1631,4 +1631,50 @@ public function testRequireWithoutWithWildCard(): void
$this->validation->getError('a.1.c')
);
}

/**
* @see https://github.com/codeigniter4/CodeIgniter4/issues/8128
*/
public function testRuleWithAsteriskToMultiDimensionalArray(): void
{
$data = [
'contacts' => [
'name' => 'Joe Smith',
'just' => [
'friends' => [
[
'name' => 'Fred Flinstone',
],
[
'name' => 'Wilma',
],
],
],
],
];

$this->validation->setRules(
['contacts.just.friends.*.name' => 'required|max_length[1]']
);
$this->assertFalse($this->validation->run($data));
$this->assertSame(
[
'contacts.just.friends.0.name' => 'The contacts.just.friends.*.name field cannot exceed 1 characters in length.',
'contacts.just.friends.1.name' => 'The contacts.just.friends.*.name field cannot exceed 1 characters in length.',
],
$this->validation->getErrors()
);

$this->validation->reset();
$this->validation->setRules(
['contacts.*.name' => 'required|max_length[1]']
);
$this->assertFalse($this->validation->run($data));
$this->assertSame(
// The data for `contacts.*.name` does not exist. So it is interpreted
// as `null`, and this error message returns.
['contacts.*.name' => 'The contacts.*.name field is required.'],
$this->validation->getErrors()
);
}
}
7 changes: 7 additions & 0 deletions user_guide_src/source/changelogs/v4.4.4.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,13 @@ Release Date: Unreleased
BREAKING
********

Validation with Dot Array Syntax
================================

A validation rule with the wildcard ``*`` now validates only data in correct
dimensions as "dot array syntax".
See :ref:`Upgrading <upgrade-444-validation-with-dot-array-syntax>` for details.

***************
Message Changes
***************
Expand Down
19 changes: 19 additions & 0 deletions user_guide_src/source/installation/upgrade_444.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,25 @@ Mandatory File Changes
Breaking Changes
****************

.. _upgrade-444-validation-with-dot-array-syntax:

Validation with Dot Array Syntax
================================

If you are using :ref:`dot array syntax <validation-dot-array-syntax>` in validation
rules, a bug where ``*`` would validate data in incorrect dimensions has been fixed.

In previous versions, the rule key ``contacts.*.name`` captured data with any
level like ``contacts.*.name``, ``contacts.*.*.name``, ``contacts.*.*.*.name``,
etc., incorrectly.

The following code explains details:

.. literalinclude:: upgrade_444/001.php
:lines: 2-

If you have code that depends on the bug, fix the the rule key.

*********************
Breaking Enhancements
*********************
Expand Down
38 changes: 38 additions & 0 deletions user_guide_src/source/installation/upgrade_444/001.php
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
<?php

use Config\Services;

$validation = Services::validation();

$data = [
'contacts' => [
'name' => 'Joe Smith',
'just' => [
'friends' => [
['name' => 'SATO Taro'],
['name' => 'Li Ming'],
['name' => 'Heinz Müller'],
],
],
],
];

$validation->setRules(
['contacts.*.name' => 'required|max_length[8]']
);

$validation->run($data); // false

d($validation->getErrors());
/*
Before: Captured `contacts.*.*.*.name` incorrectly.
[
contacts.just.friends.0.name => "The contacts.*.name field cannot exceed 8 characters in length.",
contacts.just.friends.2.name => "The contacts.*.name field cannot exceed 8 characters in length.",
]
After: Captures no data for `contacts.*.name`.
[
contacts.*.name => string (38) "The contacts.*.name field is required.",
]
*/
15 changes: 11 additions & 4 deletions user_guide_src/source/libraries/validation.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,8 @@ To give a labeled error message you can set up as:
.. note:: ``setRules()`` will overwrite any rules that were set previously. To add more than one
rule to an existing set of rules, use ``setRule()`` multiple times.

.. _validation-dot-array-syntax:

Setting Rules for Array Data
============================

Expand All @@ -328,6 +330,10 @@ You can use the ``*`` wildcard symbol to match any one level of the array:
.. literalinclude:: validation/010.php
:lines: 2-

.. note:: Prior to v4.4.4, due to a bug, the wildcard ``*`` validated data in incorrect
dimensions. See :ref:`Upgrading <upgrade-444-validation-with-dot-array-syntax>`
for details.

"dot array syntax" can also be useful when you have single dimension array data.
For example, data returned by multi select dropdown:

Expand Down Expand Up @@ -591,7 +597,7 @@ If you need to retrieve all error messages for failed fields, you can use the ``

If no errors exist, an empty array will be returned.

When using a wildcard, the error will point to a specific field, replacing the asterisk with the appropriate key/keys::
When using a wildcard (``*``), the error will point to a specific field, replacing the asterisk with the appropriate key/keys::

// for data
'contacts' => [
Expand All @@ -606,10 +612,10 @@ When using a wildcard, the error will point to a specific field, replacing the a
]

// rule
'contacts.*.name' => 'required'
'contacts.friends.*.name' => 'required'

// error will be
'contacts.friends.1.name' => 'The contacts.*.name field is required.'
'contacts.friends.1.name' => 'The contacts.friends.*.name field is required.'

Getting a Single Error
======================
Expand Down Expand Up @@ -830,7 +836,8 @@ alpha_numeric_punct No Fails if field contains anything other than
alphanumeric, space, or this limited set of
punctuation characters: ``~`` (tilde),
``!`` (exclamation), ``#`` (number),
``$`` (dollar), ``% (percent), & (ampersand),
``$`` (dollar), ``%`` (percent),
``&`` (ampersand),
``*`` (asterisk), ``-`` (dash),
``_`` (underscore), ``+`` (plus),
``=`` (equals), ``|`` (vertical bar),
Expand Down
7 changes: 1 addition & 6 deletions user_guide_src/source/libraries/validation/009.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* The data to test:
* [
* 'contacts' => [
* 'name' => 'Joe Smith',
* 'name' => 'Joe Smith',
* 'friends' => [
* [
* 'name' => 'Fred Flinstone',
Expand All @@ -21,8 +21,3 @@
$validation->setRules([
'contacts.name' => 'required|max_length[60]',
]);

// Fred Flintsone & Wilma
$validation->setRules([
'contacts.friends.name' => 'required|max_length[60]',
]);
2 changes: 1 addition & 1 deletion user_guide_src/source/libraries/validation/010.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@

// Fred Flintsone & Wilma
$validation->setRules([
'contacts.*.name' => 'required|max_length[60]',
'contacts.friends.*.name' => 'required|max_length[60]',
]);

0 comments on commit 3266127

Please sign in to comment.