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

docs: replace types mixed at all classes in Validation. #6607

Conversation

ping-yee
Copy link
Contributor

@ping-yee ping-yee commented Sep 29, 2022

Description
See #6310
I don't modify the mixed at reuquired_without[] comment out in Validation\StrictRules\Rules.php because I already changed it at PR #6589 .

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 changed the title docs: replace types mixed at all of Validation class. docs: replace types mixed at all classes in Validation. Sep 29, 2022
@kenjis
Copy link
Member

kenjis commented Sep 29, 2022

Probably the input value type is array|bool|float|int|object|string|null for all methods.
Because an attacker can send any value that can be used in JSON.

@ping-yee
Copy link
Contributor Author

Honestly, I am not sure about the different between normal rule and strict rule.

Is it just different between having strict mode and not having strict mode?
If it is, there is the fixed type ?string $str paramter of these function at not having strict mode.

Like:

In not having strict mode.

public function differs(?string $str, string $field, array $data): bool

public function equals(?string $str, string $val): bool

In having strict mode.

public function differs($str, string $field, array $data): bool

public function equals($str, string $val): bool

If all of the values need to be changed to array|bool|float|int|object|string|null, probably all of paramters need to be changed from ?string $str to $str at not having strict mode file.

@kenjis
Copy link
Member

kenjis commented Sep 29, 2022

See https://codeigniter4.github.io/CodeIgniter4/libraries/validation.html#traditional-and-strict-rules

Traditional Rules are legacy. These are not designed to validate non-string values.

Strictly speaking, probably traditional rule parameter types should be removed, too.
But it is a breaking changes.

Copy link
Member

@MGatner MGatner left a comment

Choose a reason for hiding this comment

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

I'm afraid it will need to be more nuanced and painstaking than this. For example, the first line of int:

if (is_int($str)) {

... clearly means to support int input.

I think it is a problem with our Validation class in a larger scale, that it is trying to be both Form Validation and more General/Persistence Validation. But until a rework is agreed upon and executed we will need to try to hold these together.

@ping-yee
Copy link
Contributor Author

ping-yee commented Oct 3, 2022

Yeah, it looks like will be a huge work and what I think is too easy before.
But I have no time to handle this huge work recently, I may split this huge work into some smaller PR to contribute.

@ping-yee ping-yee closed this Oct 12, 2022
@ping-yee ping-yee deleted the docs-all-validation-class-type-change branch April 25, 2023 07:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants