Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

fix: validation change behavior regex match in integer and numeric to php function #6490

Closed
wants to merge 11 commits into from
Closed

Conversation

ddevsr
Copy link
Collaborator

@ddevsr ddevsr commented Sep 4, 2022

Description
Fixed #6489

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

@ddevsr ddevsr changed the title fix: change behavior regex match to php function fix: validation change behavior regex match in integer and numeric to php function Sep 4, 2022
Copy link
Collaborator

@iRedds iRedds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add tests.

{
return (bool) preg_match('/\A[\-+]?\d+\z/', $str ?? '');
return is_int($str);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All data sent via HTTP is represented as strings and arrays. That is, an integer as a string will not pass validation.

@iRedds
Copy link
Collaborator

iRedds commented Sep 4, 2022

I think that it would be more correct to remove typehint for passed values in all rules. Casting to the appropriate types within the method.

@kenjis kenjis added the breaking change Pull requests that may break existing functionalities label Sep 4, 2022
@kenjis
Copy link
Member

kenjis commented Sep 4, 2022

This PR brings a lot of breaking changes. Probably unacceptable.

There was 1 error:

1) CodeIgniter\Validation\FormatRulesTest::testInteger with data set #0 (1, true)
TypeError: Argument 1 passed to CodeIgniter\Validation\FormatRulesTest::testInteger() must be of the type string or null, int given, called in /home/runner/work/CodeIgniter4/CodeIgniter4/vendor/phpunit/phpunit/src/Framework/TestCase.php on line 1545

/home/runner/work/CodeIgniter4/CodeIgniter4/tests/system/Validation/FormatRulesTest.php:867
phpvfscomposer:///home/runner/work/CodeIgniter4/CodeIgniter4/vendor/phpunit/phpunit/phpunit:97

--

There were 6 failures:

1) CodeIgniter\Validation\FormatRulesTest::testIntegerWithInvalidTypeData with data set "bool true" (true, true)
Failed asserting that false is identical to true.

/home/runner/work/CodeIgniter4/CodeIgniter4/tests/system/Validation/FormatRulesTest.php:812
phpvfscomposer:///home/runner/work/CodeIgniter4/CodeIgniter4/vendor/phpunit/phpunit/phpunit:97

2) CodeIgniter\Validation\FormatRulesTest::testInteger with data set #2 ('42', true)
Failed asserting that false is identical to true.

/home/runner/work/CodeIgniter4/CodeIgniter4/tests/system/Validation/FormatRulesTest.php:877
phpvfscomposer:///home/runner/work/CodeIgniter4/CodeIgniter4/vendor/phpunit/phpunit/phpunit:97

3) CodeIgniter\Validation\FormatRulesTest::testInteger with data set #3 ('-1', true)
Failed asserting that false is identical to true.

/home/runner/work/CodeIgniter4/CodeIgniter4/tests/system/Validation/FormatRulesTest.php:877
phpvfscomposer:///home/runner/work/CodeIgniter4/CodeIgniter4/vendor/phpunit/phpunit/phpunit:97

4) CodeIgniter\Validation\StrictRules\FormatRulesTest::testInteger with data set #0 ('0', true)
Failed asserting that false is identical to true.

/home/runner/work/CodeIgniter4/CodeIgniter4/tests/system/Validation/StrictRules/FormatRulesTest.php:876
phpvfscomposer:///home/runner/work/CodeIgniter4/CodeIgniter4/vendor/phpunit/phpunit/phpunit:97

5) CodeIgniter\Validation\StrictRules\FormatRulesTest::testInteger with data set #1 ('42', true)
Failed asserting that false is identical to true.

/home/runner/work/CodeIgniter4/CodeIgniter4/tests/system/Validation/StrictRules/FormatRulesTest.php:876
phpvfscomposer:///home/runner/work/CodeIgniter4/CodeIgniter4/vendor/phpunit/phpunit/phpunit:97

6) CodeIgniter\Validation\StrictRules\FormatRulesTest::testInteger with data set #2 ('-1', true)
Failed asserting that false is identical to true.

/home/runner/work/CodeIgniter4/CodeIgniter4/tests/system/Validation/StrictRules/FormatRulesTest.php:876
phpvfscomposer:///home/runner/work/CodeIgniter4/CodeIgniter4/vendor/phpunit/phpunit/phpunit:97

@kenjis
Copy link
Member

kenjis commented Sep 5, 2022

I think that it would be more correct to remove typehint for passed values in all rules. Casting to the appropriate types within the method.

Yes. When we introduce declare(strict_types=1), many TypeError would occur.

@ddevsr
Copy link
Collaborator Author

ddevsr commented Sep 5, 2022

I think that it would be more correct to remove typehint for passed values in all rules. Casting to the appropriate types within the method.

Yes. When we introduce declare(strict_types=1), many TypeError would occur.
image

@kenjis this PR stuck on cache test php unit, i already adjusted in integerProvider

@ddevsr ddevsr marked this pull request as draft September 5, 2022 03:43
@kenjis kenjis added stale Pull requests with conflicts needs rework Changes requested by reviewer that are still pending labels Oct 17, 2022
@ddevsr ddevsr closed this Nov 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Pull requests that may break existing functionalities needs rework Changes requested by reviewer that are still pending stale Pull requests with conflicts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: "integer" validation rule 500 error
3 participants