Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Validation rule with * gets incorrect values as dot array syntax #8129

Merged
merged 7 commits into from
Nov 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 28 additions & 13 deletions system/Validation/Validation.php
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
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
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
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
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
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
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
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]',
]);
Loading