Skip to content

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

Closed
ping-yee wants to merge 1 commit into
codeigniter4:developfrom
ping-yee:231023_validation
Closed

[Draft] fix: Validation passes if key does not exist when using asterisk.#8079
ping-yee wants to merge 1 commit into
codeigniter4:developfrom
ping-yee:231023_validation

Conversation

@ping-yee

Copy link
Copy Markdown
Contributor

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
ping-yee marked this pull request as draft October 23, 2023 03:47
@ping-yee

ping-yee commented Oct 23, 2023

Copy link
Copy Markdown
ContributorAuthor

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

@kenjis

Copy link
Copy Markdown
Member

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

publicfunctiontestRunRequiredSingleFieldEmptyData(): void
{
$config = newValidationConfig();
$validation = newValidation($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.']);
}
publicfunctiontestRunAlphaSingleFieldEmptyData(): void
{
$config = newValidationConfig();
$validation = newValidation($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

kenjis commented Oct 24, 2023

Copy link
Copy Markdown
Member

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,
],
],
],
];

@kenjiskenjis added the bug Verified issues on the current code behavior or pull requests that will fix them label Oct 27, 2023
@ping-yee

ping-yee commented Oct 30, 2023

Copy link
Copy Markdown
ContributorAuthor

There are some problem I should figure out first:

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

@kenjis

kenjis commented Oct 30, 2023

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
ContributorAuthor

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

Copy link
Copy Markdown
ContributorAuthor

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.

publicfunctionindex(): 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

kenjis commented Oct 30, 2023

Copy link
Copy Markdown
Member
publicfunctionindex(): 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

Copy link
Copy Markdown
Member
publicfunctionindex(): 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

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Member

This looks good.

publicfunctionindex(): 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

kenjis commented Oct 30, 2023

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
ContributorAuthor

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:

publicfunctionindex(): 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

Copy link
Copy Markdown
Member

I created issue #8128

@ping-yee

ping-yee commented Oct 31, 2023

Copy link
Copy Markdown
ContributorAuthor

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

@kenjis

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
ContributorAuthor

Okay and thanks!

@kenjis

Copy link
Copy Markdown
Member

Your comment #8079 (comment) was very helpful!

@kenjis

Copy link
Copy Markdown
Member

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

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugVerified issues on the current code behavior or pull requests that will fix them

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@ping-yee@kenjis
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
[Draft] fix: Validation passes if key does not exist when using asterisk. by ping-yee · Pull Request #8079 · codeigniter4/CodeIgniter4 · GitHub
Skip to content

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

Closed
ping-yee wants to merge 1 commit into
codeigniter4:developfrom
ping-yee:231023_validation
Closed

[Draft] fix: Validation passes if key does not exist when using asterisk.#8079
ping-yee wants to merge 1 commit into
codeigniter4:developfrom
ping-yee:231023_validation

Conversation

@ping-yee

Copy link
Copy Markdown
Contributor

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
ping-yee marked this pull request as draft October 23, 2023 03:47
@ping-yee

ping-yee commented Oct 23, 2023

Copy link
Copy Markdown
ContributorAuthor

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

@kenjis

Copy link
Copy Markdown
Member

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

publicfunctiontestRunRequiredSingleFieldEmptyData(): void
{
$config = newValidationConfig();
$validation = newValidation($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.']);
}
publicfunctiontestRunAlphaSingleFieldEmptyData(): void
{
$config = newValidationConfig();
$validation = newValidation($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

kenjis commented Oct 24, 2023

Copy link
Copy Markdown
Member

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,
],
],
],
];

@kenjiskenjis added the bug Verified issues on the current code behavior or pull requests that will fix them label Oct 27, 2023
@ping-yee

ping-yee commented Oct 30, 2023

Copy link
Copy Markdown
ContributorAuthor

There are some problem I should figure out first:

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

@kenjis

kenjis commented Oct 30, 2023

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
ContributorAuthor

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

Copy link
Copy Markdown
ContributorAuthor

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.

publicfunctionindex(): 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

kenjis commented Oct 30, 2023

Copy link
Copy Markdown
Member
publicfunctionindex(): 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

Copy link
Copy Markdown
Member
publicfunctionindex(): 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

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Member

This looks good.

publicfunctionindex(): 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

kenjis commented Oct 30, 2023

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
ContributorAuthor

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:

publicfunctionindex(): 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

Copy link
Copy Markdown
Member

I created issue #8128

@ping-yee

ping-yee commented Oct 31, 2023

Copy link
Copy Markdown
ContributorAuthor

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

@kenjis

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
ContributorAuthor

Okay and thanks!

@kenjis

Copy link
Copy Markdown
Member

Your comment #8079 (comment) was very helpful!

@kenjis

Copy link
Copy Markdown
Member

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

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugVerified issues on the current code behavior or pull requests that will fix them

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@ping-yee@kenjis
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' [Draft] fix: Validation passes if key does not exist when using asterisk. by ping-yee · Pull Request #8079 · codeigniter4/CodeIgniter4 · GitHub
Skip to content

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

Closed
ping-yee wants to merge 1 commit into
codeigniter4:developfrom
ping-yee:231023_validation
Closed

[Draft] fix: Validation passes if key does not exist when using asterisk.#8079
ping-yee wants to merge 1 commit into
codeigniter4:developfrom
ping-yee:231023_validation

Conversation

@ping-yee

Copy link
Copy Markdown
Contributor

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
ping-yee marked this pull request as draft October 23, 2023 03:47
@ping-yee

ping-yee commented Oct 23, 2023

Copy link
Copy Markdown
ContributorAuthor

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

@kenjis

Copy link
Copy Markdown
Member

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

publicfunctiontestRunRequiredSingleFieldEmptyData(): void
{
$config = newValidationConfig();
$validation = newValidation($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.']);
}
publicfunctiontestRunAlphaSingleFieldEmptyData(): void
{
$config = newValidationConfig();
$validation = newValidation($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

kenjis commented Oct 24, 2023

Copy link
Copy Markdown
Member

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,
],
],
],
];

@kenjiskenjis added the bug Verified issues on the current code behavior or pull requests that will fix them label Oct 27, 2023
@ping-yee

ping-yee commented Oct 30, 2023

Copy link
Copy Markdown
ContributorAuthor

There are some problem I should figure out first:

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

@kenjis

kenjis commented Oct 30, 2023

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
ContributorAuthor

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

Copy link
Copy Markdown
ContributorAuthor

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.

publicfunctionindex(): 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

kenjis commented Oct 30, 2023

Copy link
Copy Markdown
Member
publicfunctionindex(): 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

Copy link
Copy Markdown
Member
publicfunctionindex(): 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

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Member

This looks good.

publicfunctionindex(): 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

kenjis commented Oct 30, 2023

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
ContributorAuthor

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:

publicfunctionindex(): 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

Copy link
Copy Markdown
Member

I created issue #8128

@ping-yee

ping-yee commented Oct 31, 2023

Copy link
Copy Markdown
ContributorAuthor

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

@kenjis

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
ContributorAuthor

Okay and thanks!

@kenjis

Copy link
Copy Markdown
Member

Your comment #8079 (comment) was very helpful!

@kenjis

Copy link
Copy Markdown
Member

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

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugVerified issues on the current code behavior or pull requests that will fix them

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@ping-yee@kenjis
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' [Draft] fix: Validation passes if key does not exist when using asterisk. by ping-yee · Pull Request #8079 · codeigniter4/CodeIgniter4 · GitHub
Skip to content

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

Closed
ping-yee wants to merge 1 commit into
codeigniter4:developfrom
ping-yee:231023_validation
Closed

[Draft] fix: Validation passes if key does not exist when using asterisk.#8079
ping-yee wants to merge 1 commit into
codeigniter4:developfrom
ping-yee:231023_validation

Conversation

@ping-yee

Copy link
Copy Markdown
Contributor

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
ping-yee marked this pull request as draft October 23, 2023 03:47
@ping-yee

ping-yee commented Oct 23, 2023

Copy link
Copy Markdown
ContributorAuthor

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

@kenjis

Copy link
Copy Markdown
Member

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

publicfunctiontestRunRequiredSingleFieldEmptyData(): void
{
$config = newValidationConfig();
$validation = newValidation($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.']);
}
publicfunctiontestRunAlphaSingleFieldEmptyData(): void
{
$config = newValidationConfig();
$validation = newValidation($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

kenjis commented Oct 24, 2023

Copy link
Copy Markdown
Member

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,
],
],
],
];

@kenjiskenjis added the bug Verified issues on the current code behavior or pull requests that will fix them label Oct 27, 2023
@ping-yee

ping-yee commented Oct 30, 2023

Copy link
Copy Markdown
ContributorAuthor

There are some problem I should figure out first:

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

@kenjis

kenjis commented Oct 30, 2023

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
ContributorAuthor

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

Copy link
Copy Markdown
ContributorAuthor

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.

publicfunctionindex(): 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

kenjis commented Oct 30, 2023

Copy link
Copy Markdown
Member
publicfunctionindex(): 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

Copy link
Copy Markdown
Member
publicfunctionindex(): 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

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Member

This looks good.

publicfunctionindex(): 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

kenjis commented Oct 30, 2023

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
ContributorAuthor

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:

publicfunctionindex(): 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

Copy link
Copy Markdown
Member

I created issue #8128

@ping-yee

ping-yee commented Oct 31, 2023

Copy link
Copy Markdown
ContributorAuthor

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

@kenjis

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
ContributorAuthor

Okay and thanks!

@kenjis

Copy link
Copy Markdown
Member

Your comment #8079 (comment) was very helpful!

@kenjis

Copy link
Copy Markdown
Member

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

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugVerified issues on the current code behavior or pull requests that will fix them

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@ping-yee@kenjis
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' [Draft] fix: Validation passes if key does not exist when using asterisk. by ping-yee · Pull Request #8079 · codeigniter4/CodeIgniter4 · GitHub
Skip to content

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

Closed
ping-yee wants to merge 1 commit into
codeigniter4:developfrom
ping-yee:231023_validation
Closed

[Draft] fix: Validation passes if key does not exist when using asterisk.#8079
ping-yee wants to merge 1 commit into
codeigniter4:developfrom
ping-yee:231023_validation

Conversation

@ping-yee

Copy link
Copy Markdown
Contributor

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
ping-yee marked this pull request as draft October 23, 2023 03:47
@ping-yee

ping-yee commented Oct 23, 2023

Copy link
Copy Markdown
ContributorAuthor

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

@kenjis

Copy link
Copy Markdown
Member

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

publicfunctiontestRunRequiredSingleFieldEmptyData(): void
{
$config = newValidationConfig();
$validation = newValidation($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.']);
}
publicfunctiontestRunAlphaSingleFieldEmptyData(): void
{
$config = newValidationConfig();
$validation = newValidation($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

kenjis commented Oct 24, 2023

Copy link
Copy Markdown
Member

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,
],
],
],
];

@kenjiskenjis added the bug Verified issues on the current code behavior or pull requests that will fix them label Oct 27, 2023
@ping-yee

ping-yee commented Oct 30, 2023

Copy link
Copy Markdown
ContributorAuthor

There are some problem I should figure out first:

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

@kenjis

kenjis commented Oct 30, 2023

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
ContributorAuthor

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

Copy link
Copy Markdown
ContributorAuthor

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.

publicfunctionindex(): 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

kenjis commented Oct 30, 2023

Copy link
Copy Markdown
Member
publicfunctionindex(): 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

Copy link
Copy Markdown
Member
publicfunctionindex(): 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

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Member

This looks good.

publicfunctionindex(): 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

kenjis commented Oct 30, 2023

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
ContributorAuthor

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:

publicfunctionindex(): 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

Copy link
Copy Markdown
Member

I created issue #8128

@ping-yee

ping-yee commented Oct 31, 2023

Copy link
Copy Markdown
ContributorAuthor

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

@kenjis

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
ContributorAuthor

Okay and thanks!

@kenjis

Copy link
Copy Markdown
Member

Your comment #8079 (comment) was very helpful!

@kenjis

Copy link
Copy Markdown
Member

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

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugVerified issues on the current code behavior or pull requests that will fix them

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@ping-yee@kenjis
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' [Draft] fix: Validation passes if key does not exist when using asterisk. by ping-yee · Pull Request #8079 · codeigniter4/CodeIgniter4 · GitHub
Skip to content

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

Closed
ping-yee wants to merge 1 commit into
codeigniter4:developfrom
ping-yee:231023_validation
Closed

[Draft] fix: Validation passes if key does not exist when using asterisk.#8079
ping-yee wants to merge 1 commit into
codeigniter4:developfrom
ping-yee:231023_validation

Conversation

@ping-yee

Copy link
Copy Markdown
Contributor

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
ping-yee marked this pull request as draft October 23, 2023 03:47
@ping-yee

ping-yee commented Oct 23, 2023

Copy link
Copy Markdown
ContributorAuthor

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

@kenjis

Copy link
Copy Markdown
Member

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

publicfunctiontestRunRequiredSingleFieldEmptyData(): void
{
$config = newValidationConfig();
$validation = newValidation($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.']);
}
publicfunctiontestRunAlphaSingleFieldEmptyData(): void
{
$config = newValidationConfig();
$validation = newValidation($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

kenjis commented Oct 24, 2023

Copy link
Copy Markdown
Member

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,
],
],
],
];

@kenjiskenjis added the bug Verified issues on the current code behavior or pull requests that will fix them label Oct 27, 2023
@ping-yee

ping-yee commented Oct 30, 2023

Copy link
Copy Markdown
ContributorAuthor

There are some problem I should figure out first:

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

@kenjis

kenjis commented Oct 30, 2023

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
ContributorAuthor

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

Copy link
Copy Markdown
ContributorAuthor

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.

publicfunctionindex(): 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

kenjis commented Oct 30, 2023

Copy link
Copy Markdown
Member
publicfunctionindex(): 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

Copy link
Copy Markdown
Member
publicfunctionindex(): 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

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Member

This looks good.

publicfunctionindex(): 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

kenjis commented Oct 30, 2023

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
ContributorAuthor

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:

publicfunctionindex(): 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

Copy link
Copy Markdown
Member

I created issue #8128

@ping-yee

ping-yee commented Oct 31, 2023

Copy link
Copy Markdown
ContributorAuthor

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

@kenjis

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
ContributorAuthor

Okay and thanks!

@kenjis

Copy link
Copy Markdown
Member

Your comment #8079 (comment) was very helpful!

@kenjis

Copy link
Copy Markdown
Member

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

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugVerified issues on the current code behavior or pull requests that will fix them

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@ping-yee@kenjis
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); })(); [Draft] fix: Validation passes if key does not exist when using asterisk. by ping-yee · Pull Request #8079 · codeigniter4/CodeIgniter4 · GitHub
Skip to content

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

Closed
ping-yee wants to merge 1 commit into
codeigniter4:developfrom
ping-yee:231023_validation
Closed

[Draft] fix: Validation passes if key does not exist when using asterisk.#8079
ping-yee wants to merge 1 commit into
codeigniter4:developfrom
ping-yee:231023_validation

Conversation

@ping-yee

Copy link
Copy Markdown
Contributor

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
ping-yee marked this pull request as draft October 23, 2023 03:47
@ping-yee

ping-yee commented Oct 23, 2023

Copy link
Copy Markdown
ContributorAuthor

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

@kenjis

Copy link
Copy Markdown
Member

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

publicfunctiontestRunRequiredSingleFieldEmptyData(): void
{
$config = newValidationConfig();
$validation = newValidation($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.']);
}
publicfunctiontestRunAlphaSingleFieldEmptyData(): void
{
$config = newValidationConfig();
$validation = newValidation($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

kenjis commented Oct 24, 2023

Copy link
Copy Markdown
Member

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,
],
],
],
];

@kenjiskenjis added the bug Verified issues on the current code behavior or pull requests that will fix them label Oct 27, 2023
@ping-yee

ping-yee commented Oct 30, 2023

Copy link
Copy Markdown
ContributorAuthor

There are some problem I should figure out first:

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

@kenjis

kenjis commented Oct 30, 2023

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
ContributorAuthor

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

Copy link
Copy Markdown
ContributorAuthor

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.

publicfunctionindex(): 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

kenjis commented Oct 30, 2023

Copy link
Copy Markdown
Member
publicfunctionindex(): 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

Copy link
Copy Markdown
Member
publicfunctionindex(): 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

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Member

This looks good.

publicfunctionindex(): 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

kenjis commented Oct 30, 2023

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
ContributorAuthor

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:

publicfunctionindex(): 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

Copy link
Copy Markdown
Member

I created issue #8128

@ping-yee

ping-yee commented Oct 31, 2023

Copy link
Copy Markdown
ContributorAuthor

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

@kenjis

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
ContributorAuthor

Okay and thanks!

@kenjis

Copy link
Copy Markdown
Member

Your comment #8079 (comment) was very helpful!

@kenjis

Copy link
Copy Markdown
Member

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

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugVerified issues on the current code behavior or pull requests that will fix them

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@ping-yee@kenjis