From a67bfc127446c48520113009e044c52d057e08cc Mon Sep 17 00:00:00 2001 From: Chris Reichel Date: Thu, 10 Apr 2025 10:34:06 +0200 Subject: [PATCH 1/2] array union in controllers ! fix yii\web\Controller::bindActionParams behaviour for union types o some refactoring in the process --- framework/web/Controller.php | 159 +++++++++++++++----- tests/framework/web/ControllerTest.php | 48 ++++++ tests/framework/web/FakePhp80Controller.php | 5 + 3 files changed, 178 insertions(+), 34 deletions(-) diff --git a/framework/web/Controller.php b/framework/web/Controller.php index 2177a7f0bfc..463dacc86ed 100644 --- a/framework/web/Controller.php +++ b/framework/web/Controller.php @@ -133,41 +133,15 @@ public function bindActionParams($action, $params) $name = $param->getName(); if (array_key_exists($name, $params)) { $isValid = true; - $isArray = ($type = $param->getType()) instanceof \ReflectionNamedType && $type->getName() === 'array'; - if ($isArray) { - $params[$name] = (array)$params[$name]; - } elseif (is_array($params[$name])) { - $isValid = false; - } elseif ( - PHP_VERSION_ID >= 70000 - && ($type = $param->getType()) !== null - && method_exists($type, 'isBuiltin') - && $type->isBuiltin() - && ($params[$name] !== null || !$type->allowsNull()) - ) { - $typeName = PHP_VERSION_ID >= 70100 ? $type->getName() : (string)$type; - - if ($params[$name] === '' && $type->allowsNull()) { - if ($typeName !== 'string') { // for old string behavior compatibility - $params[$name] = null; - } - } else { - switch ($typeName) { - case 'int': - $params[$name] = filter_var($params[$name], FILTER_VALIDATE_INT, FILTER_NULL_ON_FAILURE); - break; - case 'float': - $params[$name] = filter_var($params[$name], FILTER_VALIDATE_FLOAT, FILTER_NULL_ON_FAILURE); - break; - case 'bool': - $params[$name] = filter_var($params[$name], FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE); - break; - } - if ($params[$name] === null) { - $isValid = false; - } - } + $type = $param->getType(); + if ($type instanceof \ReflectionNamedType) { + [$result, $isValid] = $this->filterSingleTypeActionParam($params[$name], $type); + $params[$name] = $result; + } elseif (class_exists('\ReflectionUnionType') && $type instanceof \ReflectionUnionType) { + [$result, $isValid] = $this->filterUnionTypeActionParam($params[$name], $type); + $params[$name] = $result; } + if (!$isValid) { throw new BadRequestHttpException( Yii::t('yii', 'Invalid data received for parameter "{param}".', ['param' => $name]) @@ -211,6 +185,123 @@ public function bindActionParams($action, $params) return $args; } + /** + * The logic for [[bindActionParam]] to validate whether a given parameter matches the action's typing + * if the function parameter has a single named type. + * @param mixed $param The parameter value. + * @param \ReflectionNamedType $type + * @return array{0: mixed, 1: bool} The resulting parameter value and a boolean indicating whether the value is valid. + */ + private function filterSingleTypeActionParam($param, $type) + { + $isArray = $type->getName() === 'array'; + if ($isArray) { + return [(array)$param, true]; + } + + if (is_array($param)) { + return [$param, false]; + } + + if (PHP_VERSION_ID >= 70000 + && method_exists($type, 'isBuiltin') + && $type->isBuiltin() + && ($param !== null || !$type->allowsNull())) { + $typeName = PHP_VERSION_ID >= 70100 ? $type->getName() : (string)$type; + if ($param === '' && $type->allowsNull()) { + if ($typeName !== 'string') { // for old string behavior compatibility + return [null, true]; + } + return ['', true]; + } + + $filterResult = $this->filterParamByType($param, $typeName); + return [$filterResult, $filterResult !== null]; + } + return [$param, true]; + } + + /** + * The logic for [[bindActionParam]] to validate whether a given parameter matches the action's typing + * if the function parameter has a union type. + * @param mixed $param The parameter value. + * @param \ReflectionUnionType $type + * @return array{0: mixed, 1: bool} The resulting parameter value and a boolean indicating whether the value is valid. + */ + private function filterUnionTypeActionParam($param, $type) + { + $types = $type->getTypes(); + if ($param === '' && $type->allowsNull()) { + // check if type can be string for old string behavior compatibility + foreach ($types as $partialType) { + if ($partialType === null + || !method_exists($partialType, 'isBuiltin') + || !$partialType->isBuiltin() + ) { + continue; + } + $typeName = PHP_VERSION_ID >= 70100 ? $partialType->getName() : (string)$partialType; + if ($typeName === 'string') { + return ['', true]; + } + } + return [null, true]; + } + // if we found a built-in type but didn't return out, its validation failed + $foundBuiltinType = false; + // we save returning out an array or string for later because other types should take precedence + $canBeArray = false; + $canBeString = false; + foreach ($types as $partialType) { + if ($partialType === null + || !method_exists($partialType, 'isBuiltin') + || !$partialType->isBuiltin() + ) { + continue; + } + $foundBuiltinType = true; + $typeName = PHP_VERSION_ID >= 70100 ? $partialType->getName() : (string)$partialType; + $canBeArray |= $typeName === 'array'; + $canBeString |= $typeName === 'string'; + if (is_array($param)) { + if ($canBeArray) { + break; + } + continue; + } + + $filterResult = $this->filterParamByType($param, $typeName); + if ($filterResult !== null) { + return [$filterResult, true]; + } + } + if (!is_array($param) && $canBeString) { + return [$param, true]; + } + if ($canBeArray) { + return [(array)$param, true]; + } + return [$param, $canBeString || !$foundBuiltinType]; + } + + /** + * Run the according filter_var logic for teh given type. + * @param string $param The value to filter. + * @param string $typeName The type name. + * @return mixed|null The resulting value, or null if validation failed or the type can't be validated. + */ + private function filterParamByType(string $param, string $typeName) { + switch ($typeName) { + case 'int': + return filter_var($param, FILTER_VALIDATE_INT, FILTER_NULL_ON_FAILURE); + case 'float': + return filter_var($param, FILTER_VALIDATE_FLOAT, FILTER_NULL_ON_FAILURE); + case 'bool': + return filter_var($param, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE); + } + return null; + } + /** * {@inheritdoc} */ diff --git a/tests/framework/web/ControllerTest.php b/tests/framework/web/ControllerTest.php index e4b79cd4b77..1e930399bc3 100644 --- a/tests/framework/web/ControllerTest.php +++ b/tests/framework/web/ControllerTest.php @@ -10,6 +10,7 @@ use RuntimeException; use Yii; use yii\base\InlineAction; +use yii\web\BadRequestHttpException; use yii\web\NotFoundHttpException; use yii\web\Response; use yii\web\ServerErrorHttpException; @@ -329,5 +330,52 @@ public function testUnionBindingActionParams() $args = $this->controller->bindActionParams($injectionAction, $params); $this->assertSame('test', $args[0]); $this->assertSame(1, $args[1]); + + // test that a value PHP parsed to a string but that should be an int becomes one + $params = ['arg' => 'test', 'second' => '1']; + + $args = $this->controller->bindActionParams($injectionAction, $params); + $this->assertSame('test', $args[0]); + $this->assertSame(1, $args[1]); + } + + public function testUnionBindingActionParamsWithArray() + { + if (PHP_VERSION_ID < 80000) { + $this->markTestSkipped('Can not be tested on PHP < 8.0'); + return; + } + // Use the PHP80 controller for this test + $this->controller = new FakePhp80Controller('fake', new \yii\web\Application([ + 'id' => 'app', + 'basePath' => __DIR__, + 'components' => [ + 'request' => [ + 'cookieValidationKey' => 'wefJDF8sfdsfSDefwqdxj9oq', + 'scriptFile' => __DIR__ . '/index.php', + 'scriptUrl' => '/index.php', + ], + ], + ])); + + $this->mockWebApplication(['controller' => $this->controller]); + + $injectionAction = new InlineAction('array-or-int', $this->controller, 'actionArrayOrInt'); + $params = ['foo' => 1]; + + try { + $args = $this->controller->bindActionParams($injectionAction, $params); + $this->assertSame(1, $args[0]); + } catch (BadRequestHttpException $e) { + $this->fail('Failed to bind int param for array|int union type!'); + } + + try { + $paramsArray = ['foo' => [1, 2, 3, 4]]; + $args = $this->controller->bindActionParams($injectionAction, $paramsArray); + $this->assertSame([1, 2, 3, 4], $args[0]); + } catch (BadRequestHttpException $e) { + $this->fail('Failed to bind array param for array|int union type!'); + } } } diff --git a/tests/framework/web/FakePhp80Controller.php b/tests/framework/web/FakePhp80Controller.php index 8f8fc20fe7b..f3bf91c4c8e 100644 --- a/tests/framework/web/FakePhp80Controller.php +++ b/tests/framework/web/FakePhp80Controller.php @@ -17,4 +17,9 @@ public function actionInjection(int|string $arg, int|string $second) { } + + public function actionArrayOrInt(array|int $foo) + { + + } } From d29998c564de5bb4b62259a5cdf818d8f67b45ab Mon Sep 17 00:00:00 2001 From: Chris Reichel Date: Thu, 10 Apr 2025 10:39:29 +0200 Subject: [PATCH 2/2] array union in controllers o cleanup, changelog --- framework/CHANGELOG.md | 1 + framework/web/Controller.php | 15 ++++++++++----- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/framework/CHANGELOG.md b/framework/CHANGELOG.md index 2186e40a15d..395f64009be 100644 --- a/framework/CHANGELOG.md +++ b/framework/CHANGELOG.md @@ -8,6 +8,7 @@ Yii Framework 2 Change Log - Bug #20329: pgsql: Column Schema doesn't recognize PG type cast (arkhamvm) - Bug #8298: Loading fixtures does not update table sequence for Postgresql database (mtangoo) - Bug #20347: Fix compatibility with PHP 8.4: remove usage of `session.use_trans_sid` and `session.use_only_cookies` (tehmaestro) +- Bug #20351: Fix behavior for `yii\web\Controller::bindActionParams` around union types (chriscpty) 2.0.52 February 13, 2025 diff --git a/framework/web/Controller.php b/framework/web/Controller.php index 463dacc86ed..5c0c5860a5a 100644 --- a/framework/web/Controller.php +++ b/framework/web/Controller.php @@ -203,10 +203,12 @@ private function filterSingleTypeActionParam($param, $type) return [$param, false]; } - if (PHP_VERSION_ID >= 70000 + if ( + PHP_VERSION_ID >= 70000 && method_exists($type, 'isBuiltin') && $type->isBuiltin() - && ($param !== null || !$type->allowsNull())) { + && ($param !== null || !$type->allowsNull()) + ) { $typeName = PHP_VERSION_ID >= 70100 ? $type->getName() : (string)$type; if ($param === '' && $type->allowsNull()) { if ($typeName !== 'string') { // for old string behavior compatibility @@ -234,7 +236,8 @@ private function filterUnionTypeActionParam($param, $type) if ($param === '' && $type->allowsNull()) { // check if type can be string for old string behavior compatibility foreach ($types as $partialType) { - if ($partialType === null + if ( + $partialType === null || !method_exists($partialType, 'isBuiltin') || !$partialType->isBuiltin() ) { @@ -253,7 +256,8 @@ private function filterUnionTypeActionParam($param, $type) $canBeArray = false; $canBeString = false; foreach ($types as $partialType) { - if ($partialType === null + if ( + $partialType === null || !method_exists($partialType, 'isBuiltin') || !$partialType->isBuiltin() ) { @@ -290,7 +294,8 @@ private function filterUnionTypeActionParam($param, $type) * @param string $typeName The type name. * @return mixed|null The resulting value, or null if validation failed or the type can't be validated. */ - private function filterParamByType(string $param, string $typeName) { + private function filterParamByType(string $param, string $typeName) + { switch ($typeName) { case 'int': return filter_var($param, FILTER_VALIDATE_INT, FILTER_NULL_ON_FAILURE);