Skip to content

Commit

Permalink
Fix yii\base\Controller::bindInjectedParams() to not throw error when…
Browse files Browse the repository at this point in the history
… argument of `ReflectionUnionType` type is passed (#18869)
  • Loading branch information
Bizley committed Sep 6, 2021
1 parent 932806b commit ea60fba
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 2 deletions.
1 change: 1 addition & 0 deletions framework/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ Yii Framework 2 Change Log
- Enh #18783: Add `XmlResponseFormatter::$objectTagToLowercase` option to lowercase object tags (WinterSilence, samdark)
- Bug #18845: Fix duplicating `id` in `MigrateController::addDefaultPrimaryKey()` (WinterSilence, samdark)
- Bug #17119: Fix `yii\caching\Cache::multiSet()` to use `yii\caching\Cache::$defaultDuration` when no duration is passed (OscarBarrett)
- Bug #18842: Fix `yii\base\Controller::bindInjectedParams()` to not throw error when argument of `ReflectionUnionType` type is passed (bizley)


2.0.43 August 09, 2021
Expand Down
4 changes: 4 additions & 0 deletions framework/base/Controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -563,6 +563,10 @@ public function findLayoutFile($view)
*/
final protected function bindInjectedParams(\ReflectionType $type, $name, &$args, &$requestedParams)
{
if (!$type instanceof \ReflectionNamedType || $type->isBuiltin()) {
return;
}

// Since it is not a builtin type it must be DI injection.
$typeName = $type->getName();
if (($component = $this->module->get($name, false)) instanceof $typeName) {
Expand Down
2 changes: 1 addition & 1 deletion framework/console/Controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ public function bindActionParams($action, $params)
}
$args[] = $actionParams[$key] = $params[$key];
unset($params[$key]);
} elseif (PHP_VERSION_ID >= 70100 && ($type = $param->getType()) !== null && !$type->isBuiltin()) {
} elseif (PHP_VERSION_ID >= 70100 && ($type = $param->getType()) !== null) {
try {
$this->bindInjectedParams($type, $name, $args, $requestedParams);
} catch (\yii\base\Exception $e) {
Expand Down
2 changes: 1 addition & 1 deletion framework/web/Controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ public function bindActionParams($action, $params)
}
$args[] = $actionParams[$name] = $params[$name];
unset($params[$name]);
} elseif (PHP_VERSION_ID >= 70100 && ($type = $param->getType()) !== null && !$type->isBuiltin()) {
} elseif (PHP_VERSION_ID >= 70100 && ($type = $param->getType()) !== null) {
try {
$this->bindInjectedParams($type, $name, $args, $requestedParams);
} catch (HttpException $e) {
Expand Down

3 comments on commit ea60fba

@andris-sevcenko
Copy link

@andris-sevcenko andris-sevcenko commented on ea60fba Sep 28, 2021

Choose a reason for hiding this comment

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

This commit breaks running the action in some cases.

The removal of && !$type->isBuiltin() causes the logic for parsing a the signature ?int $revisionId = null to branch here and execute $this->bindInjectedParams($type, $name, $args, $requestedParams);

The first if statement of that method added with this commit causes the function to return immediately for built-in types, and so the default value is never added to the$args array.

In our case, the next parameter has a signature of ?string, so, providing that puts it in the $args array where the ?int arg is expected to be, causing chaos.

@samdark
Copy link
Member

Choose a reason for hiding this comment

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

@andris-sevcenko would you please report it as an issue? It would be definitely lost here.

@andris-sevcenko
Copy link

Choose a reason for hiding this comment

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

Thanks for the tip, @samdark

Please sign in to comment.