[Draft] fix: Validation passes if key does not exist when using asterisk. #8079

ping-yee · 2023-10-23T03:47:08Z

Description
See #8006
But This PR still is draft, I need to discussion and find out how to fix this problem.

Checklist:

Securely signed commits
Component(s) with PHPDoc blocks, only if necessary or adds value
Unit testing, with >80% coverage
User guide updated
Conforms to style guide

ping-yee · 2023-10-23T18:07:58Z

@kenjis Do you have any idea about this problem?
I write what I thought in the comment out of the commit.

kenjis · 2023-10-24T00:47:03Z

The following tests show the current behaviors for single field.
I think devs expect the same behaviors for multiple fields.

    public function testRunRequiredSingleFieldEmptyData(): void
    {
        $config     = new ValidationConfig();
        $validation = new Validation($config, Services::renderer());

        $validation->setRules([
            'name' => 'required',
        ]);

        $data   = [];
        $result = $validation->run($data);

        $this->assertFalse($result);

        $errors = $validation->getErrors();

        $this->assertSame($errors, ['name' => 'The name field is required.']);
    }

    public function testRunAlphaSingleFieldEmptyData(): void
    {
        $config     = new ValidationConfig();
        $validation = new Validation($config, Services::renderer());

        $validation->setRules([
            'name' => 'alpha',
        ]);

        $data   = [];
        $result = $validation->run($data);

        $this->assertFalse($result);

        $errors = $validation->getErrors();

        $this->assertSame(
            $errors,
            ['name' => 'The name field may only contain alphabetical characters.']
        );
    }

kenjis · 2023-10-24T00:53:44Z

~~Therefore, if the following data comes,~~

        $data = [
            'contacts' => [
                'friends' => [
                    ['name' => 'Fred Flinstone', 'age' => 20],
                    ['age' => 21], // 'name' key does not exist
                ]
            ]
        ];

~~it seems we need to change it to:~~

        $data = [
            'contacts' => [
                'friends' => [
                    ['name' => 'Fred Flinstone', 'age' => 20],
                    [
                        'name' => null, // add 'name' key
                        'age'  => 21,
                    ],
                ],
            ],
        ];

ping-yee · 2023-10-30T08:00:59Z

There are some problem I should figure out first:

So should we pre-process the data first and fill in non-existent fields until they are aligned?
Is this above process also work in other rules? or does it only work in required rule scenario?

kenjis · 2023-10-30T08:37:40Z

https://codeigniter4.github.io/CodeIgniter4/libraries/validation.html#setting-rules-for-array-data
I was thinking, if we are validating against an array, the data to be validated must be in the same format.
In the above example, all elements should have a 'name' key, and data without it should cause a validation error.
So I think it is necessary to first check if the keys are present in all elements.

I sent a PR #8123 that is related to this topic.

kenjis · 2023-10-30T09:02:14Z

https://codeigniter4.github.io/CodeIgniter4/libraries/validation.html#setting-rules-for-array-data

/*
 * The data to test:
 * [
 *     'contacts' => [
 *        'name' => 'Joe Smith',
 *         'friends' => [
 *             [
 *                 'name' => 'Fred Flinstone',
 *             ],
 *             [
 *                 'name' => 'Wilma',
 *             ],
 *         ]
 *     ]
 * ]
 */

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

The contacts.name does not have name. So the example should raise the validation error?

Validation using wildcards (*) may be unclear or inconsistent with the specification.

ping-yee · 2023-10-30T09:45:29Z

I was thinking, if we are validating against an array, the data to be validated must be in the same format.
So I think it is necessary to first check if the keys are present in all elements.

I am agree with this, the before check is neccessary.
Weather this issue can be solved after adding the before check?

ping-yee · 2023-10-30T10:03:50Z

Validation using wildcards (*) may be unclear or inconsistent with the specification.

Yes, I also agree with this. The caption of user guide make me so confused. 😖
But it seems it will be accepted no matter how many layer it is.

public function index(): string
    {
        // Extend the user guide case and add one more layer.
        $requestData = [
            'contacts' => [
                'name' => 'Joe Smith',
                'just' => [
                    'friends' => [
                        [
                            'name' => 'Fred Flinstone',
                        ],
                        [
                            'name' => 'Wilma',
                        ],
                    ]
                ]
            ]
        ];


        $this->validator = \Config\Services::validation();
        $this->validator->setRules([
            'contacts.*.name' => 'required|max_length[60]',
        ]);


        dd($this->validator->run($requestData), $this->validator->getErrors());
    }

Output

$values array (2)
contacts.just.friends.0.name => string (14) "Fred Flinstone"
contacts.just.friends.1.name => string (5) "Wilma"

kenjis · 2023-10-30T10:18:53Z

    public function index(): string
    {
        // Extend the user guide case and add one more layer.
        $requestData = [
            'contacts' => [
                'name' => 'Joe Smith',
                'just' => [
                    'friends' => [
                        [
                            'name' => 'Fred Flinstone',
                        ],
                        [
                            'name' => 'Wilma',
                        ],
                    ],
                ],
            ],
        ];

        $this->validator = \Config\Services::validation();
        $this->validator->setRules([
            'contacts.*.name' => 'required|max_length[60]',
        ]);

        dd(
            $this->validator->run($requestData),
            $this->validator->getErrors(),
            $this->validator->getValidated()
        );
    }

$this->validator->run(...) boolean true
$this->validator->getErrors() array (0)
$this->validator->getValidated() array (0)
⧉ Called from .../app/Controllers/Home.php:34 [dd()]

kenjis · 2023-10-30T10:23:32Z

    public function index(): string
    {
        // Extend the user guide case and add one more layer.
        $requestData = [
            'contacts' => [
                'name' => 'Joe Smith',
                'just' => [
                    'friends' => [
                        [
                            'name' => 'Fred Flinstone',
                        ],
                        [
                            'name' => 'Wilma',
                        ],
                    ],
                ],
            ],
        ];

        $this->validator = \Config\Services::validation();
        $this->validator->setRules([
            'contacts.*.name' => 'required|max_length[1]',
        ]);

        dd(
            $this->validator->run($requestData),
            $this->validator->getErrors(),
            $this->validator->getValidated()
        );
    }

$this->validator->run(...) boolean false
⧉⌕$this->validator->getErrors() array (2)
    ⇄contacts.just.friends.0.name => string (63) "The contacts.*.name field cannot exceed 1 characters in length."
    ⇄contacts.just.friends.1.name => string (63) "The contacts.*.name field cannot exceed 1 characters in length."
$this->validator->getValidated() array (0)
⧉ Called from .../app/Controllers/Home.php:34 [dd()]

kenjis · 2023-10-30T10:26:00Z

But it seems it will be accepted no matter how many layer it is.

That seems to be a bug.
First of all, the key in the user guide should be contacts.friends.*.name.

kenjis · 2023-10-30T11:11:57Z

This looks good.

    public function index(): string
    {
        $requestData = [
            'contacts' => [
                'name'    => 'Joe Smith',
                'friends' => [
                    [
                        'name' => 'Fred Flinstone',
                    ],
                    [
                        'name' => 'Wilma',
                    ],
                ],
            ],
        ];

        $this->validator = \Config\Services::validation();
        $this->validator->setRules([
            'contacts.friends.*.name' => 'required|max_length[60]',
        ]);

        dd(
            $this->validator->run($requestData),
            $this->validator->getErrors(),
            $this->validator->getValidated()
        );
    }

$this->validator->run(...) boolean true
$this->validator->getErrors() array (0)
⧉⌕$this->validator->getValidated() array (1)
    ⇄⧉contacts => array (1)
        ⇄⧉friends => array (2)
            ⇄⧉0 => array (1
                ⇄name => string (14) "Fred Flinstone"
            ⇄⧉1 => array (1)
                ⇄name => string (5) "Wilma"
⧉ Called from .../app/Controllers/Home.php:31 [dd()]

kenjis · 2023-10-30T11:18:02Z

The second example in https://codeigniter4.github.io/CodeIgniter4/libraries/validation.html#setting-rules-for-array-data
Is this also just a mistake? I think we cannot get two values "Fred Flintsone & Wilma" without *.

        $this->validator->setRules([
            'contacts.friends.name' => 'required|max_length[60]',
        ]);

$this->validator->run(...) boolean false
⧉⌕$this->validator->getErrors() array (1)
    ⇄contacts.friends.name => string (44) "The contacts.friends.name field is required."
$this->validator->getValidated() array (0)
⧉ Called from .../app/Controllers/Home.php:31 [dd()]

ping-yee · 2023-10-30T13:09:06Z

Is this also just a mistake? I think we cannot get two values "Fred Flintsone & Wilma" without *.

I think so.. and it should be like this:

public function index(): string
    {
        $requestData = [
            'contacts' => [
                'name'    => 'Joe Smith',
                'friends' => [
                    'name' => 'Fred Flinstone',
                ],
            ],
        ];

        $this->validator = \Config\Services::validation();
        $this->validator->setRules([
            'contacts.friends.name' => 'required|max_length[60]',
        ]);

        dd(
            $this->validator->run($requestData),
            $this->validator->getErrors(),
            $this->validator->getValidated()
        );
    }

$this->validator->run(...) boolean true
$this->validator->getErrors() array (0)
⧉⌕$this->validator->getValidated() array (1)
⇄⧉contacts => array (1)
⇄⧉friends => array (1)
⇄name => string (14) "Fred Flinstone"

kenjis · 2023-10-30T22:00:22Z

I created issue #8128

ping-yee · 2023-10-31T01:35:08Z

Is there any thing that I need to do for this PR? @kenjis

kenjis · 2023-10-31T02:00:31Z

This bug has not yet been fixed, but this PR should be closed at once.

After #8128 and #8123 are completed, we can discuss again how to fix it.

ping-yee · 2023-10-31T02:02:11Z

Okay and thanks!

kenjis · 2023-10-31T02:06:29Z

Your comment #8079 (comment) was very helpful!

kenjis · 2023-10-31T06:36:10Z

I send PR #8131 to add method to check array key with dot array syntax.

ping-yee marked this pull request as draft October 23, 2023 03:47

kenjis added the bug Verified issues on the current code behavior or pull requests that will fix them label Oct 27, 2023

ping-yee mentioned this pull request Oct 30, 2023

docs: Fix the validation with asterisk example #8126

Closed

5 tasks

chore: Write out where the problem occur and the preliminary solution.

fbe9df3

ping-yee force-pushed the 231023_validation branch from f1dac23 to fbe9df3 Compare October 30, 2023 12:52

kenjis mentioned this pull request Oct 30, 2023

Bug: Validation rule with * gets incorrect values as dot array syntax #8128

Closed

ping-yee closed this Oct 31, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Draft] fix: Validation passes if key does not exist when using asterisk. #8079

[Draft] fix: Validation passes if key does not exist when using asterisk. #8079

ping-yee commented Oct 23, 2023

ping-yee commented Oct 23, 2023 •

edited

Loading

kenjis commented Oct 24, 2023

kenjis commented Oct 24, 2023 •

edited

Loading

ping-yee commented Oct 30, 2023 •

edited

Loading

kenjis commented Oct 30, 2023 •

edited

Loading

kenjis commented Oct 30, 2023

ping-yee commented Oct 30, 2023

ping-yee commented Oct 30, 2023

kenjis commented Oct 30, 2023 •

edited

Loading

kenjis commented Oct 30, 2023

kenjis commented Oct 30, 2023

kenjis commented Oct 30, 2023

kenjis commented Oct 30, 2023 •

edited

Loading

ping-yee commented Oct 30, 2023

kenjis commented Oct 30, 2023

ping-yee commented Oct 31, 2023 •

edited

Loading

kenjis commented Oct 31, 2023

ping-yee commented Oct 31, 2023

kenjis commented Oct 31, 2023

kenjis commented Oct 31, 2023

[Draft] fix: Validation passes if key does not exist when using asterisk. #8079

[Draft] fix: Validation passes if key does not exist when using asterisk. #8079

Conversation

ping-yee commented Oct 23, 2023

ping-yee commented Oct 23, 2023 • edited Loading

kenjis commented Oct 24, 2023

kenjis commented Oct 24, 2023 • edited Loading

ping-yee commented Oct 30, 2023 • edited Loading

kenjis commented Oct 30, 2023 • edited Loading

kenjis commented Oct 30, 2023

ping-yee commented Oct 30, 2023

ping-yee commented Oct 30, 2023

kenjis commented Oct 30, 2023 • edited Loading

kenjis commented Oct 30, 2023

kenjis commented Oct 30, 2023

kenjis commented Oct 30, 2023

kenjis commented Oct 30, 2023 • edited Loading

ping-yee commented Oct 30, 2023

kenjis commented Oct 30, 2023

ping-yee commented Oct 31, 2023 • edited Loading

kenjis commented Oct 31, 2023

ping-yee commented Oct 31, 2023

kenjis commented Oct 31, 2023

kenjis commented Oct 31, 2023

ping-yee commented Oct 23, 2023 •

edited

Loading

kenjis commented Oct 24, 2023 •

edited

Loading

ping-yee commented Oct 30, 2023 •

edited

Loading

kenjis commented Oct 30, 2023 •

edited

Loading

kenjis commented Oct 30, 2023 •

edited

Loading

kenjis commented Oct 30, 2023 •

edited

Loading

ping-yee commented Oct 31, 2023 •

edited

Loading