From eafe91b30040af933a6c671e7a319b010220038b Mon Sep 17 00:00:00 2001 From: kenjis Date: Mon, 23 Oct 2023 11:23:25 +0900 Subject: [PATCH 1/6] refactor!: remove param types for traditional validation rules There is no guarantee that a string will be passed, so declaring strict types may result in a TypeError. --- system/Validation/FormatRules.php | 200 ++++++++++++++++++++++++------ system/Validation/Rules.php | 8 +- 2 files changed, 170 insertions(+), 38 deletions(-) diff --git a/system/Validation/FormatRules.php b/system/Validation/FormatRules.php index 5089f5a0961f..8a16c98e4a33 100644 --- a/system/Validation/FormatRules.php +++ b/system/Validation/FormatRules.php @@ -22,10 +22,16 @@ class FormatRules { /** * Alpha + * + * @param string|null $str */ - public function alpha(?string $str = null): bool + public function alpha($str = null): bool { - return ctype_alpha($str ?? ''); + if (! is_string($str)) { + $str = (string) $str; + } + + return ctype_alpha($str); } /** @@ -35,12 +41,16 @@ public function alpha(?string $str = null): bool * * @return bool True if alpha with spaces, else false. */ - public function alpha_space(?string $value = null): bool + public function alpha_space($value = null): bool { if ($value === null) { return true; } + if (! is_string($value)) { + $value = (string) $value; + } + // @see https://regex101.com/r/LhqHPO/1 return (bool) preg_match('/\A[A-Z ]+\z/i', $value); } @@ -49,13 +59,19 @@ public function alpha_space(?string $value = null): bool * Alphanumeric with underscores and dashes * * @see https://regex101.com/r/XfVY3d/1 + * + * @param string|null $str */ - public function alpha_dash(?string $str = null): bool + public function alpha_dash($str = null): bool { if ($str === null) { return false; } + if (! is_string($str)) { + $str = (string) $str; + } + return preg_match('/\A[a-z0-9_-]+\z/i', $str) === 1; } @@ -78,24 +94,40 @@ public function alpha_numeric_punct($str) return false; } + if (! is_string($str)) { + $str = (string) $str; + } + return preg_match('/\A[A-Z0-9 ~!#$%\&\*\-_+=|:.]+\z/i', $str) === 1; } /** * Alphanumeric + * + * @param string|null $str */ - public function alpha_numeric(?string $str = null): bool + public function alpha_numeric($str = null): bool { - return ctype_alnum($str ?? ''); + if (! is_string($str)) { + $str = (string) $str; + } + + return ctype_alnum($str); } /** * Alphanumeric w/ spaces + * + * @param string|null $str */ - public function alpha_numeric_space(?string $str = null): bool + public function alpha_numeric_space($str = null): bool { + if (! is_string($str)) { + $str = (string) $str; + } + // @see https://regex101.com/r/0AZDME/1 - return (bool) preg_match('/\A[A-Z0-9 ]+\z/i', $str ?? ''); + return (bool) preg_match('/\A[A-Z0-9 ]+\z/i', $str); } /** @@ -113,64 +145,106 @@ public function string($str = null): bool /** * Decimal number + * + * @param string|null $str */ - public function decimal(?string $str = null): bool + public function decimal($str = null): bool { + if (! is_string($str)) { + $str = (string) $str; + } + // @see https://regex101.com/r/HULifl/2/ - return (bool) preg_match('/\A[-+]?\d{0,}\.?\d+\z/', $str ?? ''); + return (bool) preg_match('/\A[-+]?\d{0,}\.?\d+\z/', $str); } /** * String of hexidecimal characters + * + * @param string|null $str */ - public function hex(?string $str = null): bool + public function hex($str = null): bool { - return ctype_xdigit($str ?? ''); + if (! is_string($str)) { + $str = (string) $str; + } + + return ctype_xdigit($str); } /** * Integer + * + * @param string|null $str */ - public function integer(?string $str = null): bool + public function integer($str = null): bool { - return (bool) preg_match('/\A[\-+]?\d+\z/', $str ?? ''); + if (! is_string($str)) { + $str = (string) $str; + } + + return (bool) preg_match('/\A[\-+]?\d+\z/', $str); } /** * Is a Natural number (0,1,2,3, etc.) + * + * @param string|null $str */ - public function is_natural(?string $str = null): bool + public function is_natural($str = null): bool { - return ctype_digit($str ?? ''); + if (! is_string($str)) { + $str = (string) $str; + } + + return ctype_digit($str); } /** * Is a Natural number, but not a zero (1,2,3, etc.) + * + * @param string|null $str */ - public function is_natural_no_zero(?string $str = null): bool + public function is_natural_no_zero($str = null): bool { - return $str !== '0' && ctype_digit($str ?? ''); + if (! is_string($str)) { + $str = (string) $str; + } + + return $str !== '0' && ctype_digit($str); } /** * Numeric + * + * @param string|null $str */ - public function numeric(?string $str = null): bool + public function numeric($str = null): bool { + if (! is_string($str)) { + $str = (string) $str; + } + // @see https://regex101.com/r/bb9wtr/2 - return (bool) preg_match('/\A[\-+]?\d*\.?\d+\z/', $str ?? ''); + return (bool) preg_match('/\A[\-+]?\d*\.?\d+\z/', $str); } /** * Compares value against a regular expression pattern. + * + * @param string|null $str */ - public function regex_match(?string $str, string $pattern): bool + public function regex_match($str, string $pattern): bool { + if (! is_string($str)) { + $str = (string) $str; + } + if (strpos($pattern, '/') !== 0) { $pattern = "/{$pattern}/"; } - return (bool) preg_match($pattern, $str ?? ''); + return (bool) preg_match($pattern, $str); } /** @@ -178,10 +252,16 @@ public function regex_match(?string $str, string $pattern): bool * timezone_identifiers_list function. * * @see http://php.net/manual/en/datetimezone.listidentifiers.php + * + * @param string|null $str */ - public function timezone(?string $str = null): bool + public function timezone($str = null): bool { - return in_array($str ?? '', timezone_identifiers_list(), true); + if (! is_string($str)) { + $str = (string) $str; + } + + return in_array($str, timezone_identifiers_list(), true); } /** @@ -189,33 +269,51 @@ public function timezone(?string $str = null): bool * * Tests a string for characters outside of the Base64 alphabet * as defined by RFC 2045 http://www.faqs.org/rfcs/rfc2045 + * + * @param string|null $str */ - public function valid_base64(?string $str = null): bool + public function valid_base64($str = null): bool { if ($str === null) { return false; } + if (! is_string($str)) { + $str = (string) $str; + } + return base64_encode(base64_decode($str, true)) === $str; } /** * Valid JSON + * + * @param string|null $str */ - public function valid_json(?string $str = null): bool + public function valid_json($str = null): bool { - json_decode($str ?? ''); + if (! is_string($str)) { + $str = (string) $str; + } + + json_decode($str); return json_last_error() === JSON_ERROR_NONE; } /** * Checks for a correctly formatted email address + * + * @param string|null $str */ - public function valid_email(?string $str = null): bool + public function valid_email($str = null): bool { + if (! is_string($str)) { + $str = (string) $str; + } + // @see https://regex101.com/r/wlJG1t/1/ - if (function_exists('idn_to_ascii') && defined('INTL_IDNA_VARIANT_UTS46') && preg_match('#\A([^@]+)@(.+)\z#', $str ?? '', $matches)) { + if (function_exists('idn_to_ascii') && defined('INTL_IDNA_VARIANT_UTS46') && preg_match('#\A([^@]+)@(.+)\z#', $str, $matches)) { $str = $matches[1] . '@' . idn_to_ascii($matches[2], 0, INTL_IDNA_VARIANT_UTS46); } @@ -227,10 +325,16 @@ public function valid_email(?string $str = null): bool * * Example: * valid_emails[one@example.com,two@example.com] + * + * @param string|null $str */ - public function valid_emails(?string $str = null): bool + public function valid_emails($str = null): bool { - foreach (explode(',', $str ?? '') as $email) { + if (! is_string($str)) { + $str = (string) $str; + } + + foreach (explode(',', $str) as $email) { $email = trim($email); if ($email === '') { @@ -248,11 +352,16 @@ public function valid_emails(?string $str = null): bool /** * Validate an IP address (human readable format or binary string - inet_pton) * + * @param string|null $ip * @param string|null $which IP protocol: 'ipv4' or 'ipv6' */ - public function valid_ip(?string $ip = null, ?string $which = null): bool + public function valid_ip($ip = null, ?string $which = null): bool { - if (empty($ip)) { + if (! is_string($ip)) { + $ip = (string) $ip; + } + + if ($ip === '') { return false; } @@ -278,13 +387,19 @@ public function valid_ip(?string $ip = null, ?string $which = null): bool * * Warning: this rule will pass basic strings like * "banana"; use valid_url_strict for a stricter rule. + * + * @param string|null $str */ - public function valid_url(?string $str = null): bool + public function valid_url($str = null): bool { if (empty($str)) { return false; } + if (! is_string($str)) { + $str = (string) $str; + } + if (preg_match('/\A(?:([^:]*)\:)?\/\/(.+)\z/', $str, $matches)) { if (! in_array($matches[1], ['http', 'https'], true)) { return false; @@ -301,14 +416,19 @@ public function valid_url(?string $str = null): bool /** * Checks a URL to ensure it's formed correctly. * + * @param string|null $str * @param string|null $validSchemes comma separated list of allowed schemes */ - public function valid_url_strict(?string $str = null, ?string $validSchemes = null): bool + public function valid_url_strict($str = null, ?string $validSchemes = null): bool { if (empty($str)) { return false; } + if (! is_string($str)) { + $str = (string) $str; + } + // parse_url() may return null and false $scheme = strtolower((string) parse_url($str, PHP_URL_SCHEME)); $validSchemes = explode( @@ -322,10 +442,16 @@ public function valid_url_strict(?string $str = null, ?string $validSchemes = nu /** * Checks for a valid date and matches a given date format + * + * @param string|null $str */ - public function valid_date(?string $str = null, ?string $format = null): bool + public function valid_date($str = null, ?string $format = null): bool { - if ($str === null) { + if (! is_string($str)) { + $str = (string) $str; + } + + if ($str === '') { return false; } diff --git a/system/Validation/Rules.php b/system/Validation/Rules.php index b7c585f56c30..04d0222a63db 100644 --- a/system/Validation/Rules.php +++ b/system/Validation/Rules.php @@ -131,9 +131,15 @@ public function in_list(?string $value, string $list): bool * Example: * is_unique[table.field,ignore_field,ignore_value] * is_unique[users.email,id,5] + * + * @param string|null $str */ - public function is_unique(?string $str, string $field, array $data): bool + public function is_unique($str, string $field, array $data): bool { + if (! is_string($str) && $str !== null) { + $str = (string) $str; + } + [$field, $ignoreField, $ignoreValue] = array_pad( explode(',', $field), 3, From 61508ccca6fc971819005cbf37dfe9607bbe9a24 Mon Sep 17 00:00:00 2001 From: kenjis Date: Mon, 23 Oct 2023 11:25:28 +0900 Subject: [PATCH 2/6] refactor: cast param to string --- system/Validation/Validation.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system/Validation/Validation.php b/system/Validation/Validation.php index 0b49fe02c48b..b10711978068 100644 --- a/system/Validation/Validation.php +++ b/system/Validation/Validation.php @@ -337,7 +337,7 @@ protected function processRules( // @phpstan-ignore-next-line $error may be set by rule methods. $this->errors[$field] = $error ?? $this->getErrorMessage( - ($this->isClosure($rule) || $arrayCallable) ? $i : $rule, + ($this->isClosure($rule) || $arrayCallable) ? (string) $i : $rule, $field, $label, $param, From 7f8396a3d672adf4f6cf16bd7841ffc3b1674547 Mon Sep 17 00:00:00 2001 From: kenjis Date: Mon, 23 Oct 2023 11:27:42 +0900 Subject: [PATCH 3/6] test: fix incorrect param value types --- tests/system/Validation/CreditCardRulesTest.php | 2 +- tests/system/Validation/StrictRules/CreditCardRulesTest.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/system/Validation/CreditCardRulesTest.php b/tests/system/Validation/CreditCardRulesTest.php index 397675270eda..a37ac9b825bb 100644 --- a/tests/system/Validation/CreditCardRulesTest.php +++ b/tests/system/Validation/CreditCardRulesTest.php @@ -73,7 +73,7 @@ public function provideValidCCNumber(): iterable ], 'random_test' => [ 'amex', - $this->generateCardNumber('37', 16), + $this->generateCardNumber(37, 16), false, ], 'invalid_type' => [ diff --git a/tests/system/Validation/StrictRules/CreditCardRulesTest.php b/tests/system/Validation/StrictRules/CreditCardRulesTest.php index 2e685c852e55..abc53f4f1642 100644 --- a/tests/system/Validation/StrictRules/CreditCardRulesTest.php +++ b/tests/system/Validation/StrictRules/CreditCardRulesTest.php @@ -74,7 +74,7 @@ public function provideValidCCNumber(): iterable ], 'random_test' => [ 'amex', - $this->generateCardNumber('37', 16), + $this->generateCardNumber(37, 16), false, ], 'invalid_type' => [ From 31f1fa0e4cda6895c0006dbba4d9148e6a406adf Mon Sep 17 00:00:00 2001 From: kenjis Date: Mon, 23 Oct 2023 11:38:18 +0900 Subject: [PATCH 4/6] chore: update phpstan-baseline --- phpstan-baseline.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpstan-baseline.php b/phpstan-baseline.php index 69d6347feeea..b08c01747ec6 100644 --- a/phpstan-baseline.php +++ b/phpstan-baseline.php @@ -3783,7 +3783,7 @@ ]; $ignoreErrors[] = [ 'message' => '#^Construct empty\\(\\) is not allowed\\. Use more strict comparison\\.$#', - 'count' => 4, + 'count' => 3, 'path' => __DIR__ . '/system/Validation/FormatRules.php', ]; $ignoreErrors[] = [ From 530d5e1ef914b8c9472cc2619208af101440fcce Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 27 Oct 2023 13:20:03 +0900 Subject: [PATCH 5/6] docs: add changelog --- user_guide_src/source/changelogs/v4.5.0.rst | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/user_guide_src/source/changelogs/v4.5.0.rst b/user_guide_src/source/changelogs/v4.5.0.rst index fb0ab21a3734..7f1f52fa7ccc 100644 --- a/user_guide_src/source/changelogs/v4.5.0.rst +++ b/user_guide_src/source/changelogs/v4.5.0.rst @@ -79,6 +79,19 @@ Return Type Changes - **Model:** The return type of the ``objectToRawArray()`` method in the ``Model`` and ``BaseModel`` classes has been changed from ``?array`` to ``array``. +Traditional Validation Rules +---------------------------- + +To add ``declare(strict_types=1)`` to the framework codebase, the method parameter +type ``?string`` for a value to validate in the all Traditional Validation rule +classes ``CodeIgniter\Validation\FormatRules`` and ``CodeIgniter\Validation\Rules`` +are removed. + +For example, the method signature changed as follows:: + + Before: public function integer(?string $str = null): bool + After: public function integer($str = null): bool + Others ------ From 776b9cfaa03457bf47d166613a0e3f89ebb3f730 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 27 Oct 2023 13:20:17 +0900 Subject: [PATCH 6/6] docs: update about Traditional Validation Rules --- .../source/libraries/validation.rst | 31 +++++++++++++------ 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/user_guide_src/source/libraries/validation.rst b/user_guide_src/source/libraries/validation.rst index 810573a235ce..a3e6c55d924f 100644 --- a/user_guide_src/source/libraries/validation.rst +++ b/user_guide_src/source/libraries/validation.rst @@ -213,15 +213,29 @@ Traditional and Strict Rules ============================ CodeIgniter 4 has two kinds of Validation rule classes. -The traditional rule classes (**Traditional Rules**) have the namespace ``CodeIgniter\Validation``, -and the new classes (**Strict Rules**) have ``CodeIgniter\Validation\StrictRules``, which provide strict validation. + +The default rule classes (**Strict Rules**) have the namespace +``CodeIgniter\Validation\StrictRules``, and they provide strict validation. + +The traditional rule classes (**Traditional Rules**) have the namespace +``CodeIgniter\Validation``. They are provided for backward compatibility only. +They may not validate non-string values correctly and need not be used in new +projects. .. note:: Since v4.3.0, **Strict Rules** are used by default for better security. +Strict Rules +------------ + +.. versionadded:: 4.2.0 + +The **Strict Rules** don't use implicit type conversion. + Traditional Rules ----------------- -.. warning:: When validating data that contains non-string values, such as JSON data, it is recommended to use **Strict Rules**. +.. warning:: When validating data that contains non-string values, such as JSON data, + you should use **Strict Rules**. The **Traditional Rules** implicitly assume that string values are validated, and the input value may be converted implicitly to a string value. @@ -231,16 +245,13 @@ However, for example, if you use JSON input data, it may be a type of bool/null/ When you validate the boolean ``true``, it is converted to string ``'1'`` with the Traditional rule classes. If you validate it with the ``integer`` rule, ``'1'`` passes the validation. -Strict Rules ------------- - -.. versionadded:: 4.2.0 - -The **Strict Rules** don't use implicit type conversion. - Using Traditional Rules ----------------------- +.. warning:: The **Traditional Rules** are provided for backward compatibility only. + They may not validate non-string values correctly and need not be used in new + projects. + If you want to use traditional rules, you need to change the rule classes in **app/Config/Validation.php**: .. literalinclude:: validation/003.php