Skip to content
This repository has been archived by the owner on Feb 6, 2020. It is now read-only.

Undefined $factory notice fix #269

Merged
merged 4 commits into from
Jun 22, 2018

Conversation

demiankatz
Copy link
Contributor

When a plugin manager uses a static class method as a factory (i.e. FactoryClass::factoryMethod), as of release 2.7.10, it causes an undefined variable notice to be thrown due to an uncovered else case. This pull request provides a test case to demonstrate the problem as well as a fix. This problem does not appear to exist in the 3.x code, so it is presumably a side effect of backporting to 2.7. However, for projects depending on earlier versions of the service manager, it can be a significant problem and should be corrected -- that is why I am targeting this PR against the release-2.7 branch.

@demiankatz demiankatz force-pushed the undefined-factory-fix branch from 60d2409 to cf703e8 Compare June 8, 2018 21:56
@@ -355,6 +355,8 @@ protected function createServiceViaCallback($callable, $cName, $rName)
} elseif (is_array($callable)) {
// reset both rewinds and returns the value of the first array element
$factory = reset($callable);
} else {
$factory = null;
Copy link
Member

Choose a reason for hiding this comment

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

Can't it be initialized as null first?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, just a question of style preference. I did it this way to avoid an unnecessary double-assignment, but if you prefer to trim a couple of lines by eliminating the else clause, that works fine too. I'll change it if you wish.

* Zend Framework (http://framework.zend.com/)
*
* @link http://github.com/zendframework/zf2 for the canonical source repository
* @copyright Copyright (c) 2005-2015 Zend Technologies USA Inc. (http://www.zend.com)
Copy link
Member

Choose a reason for hiding this comment

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

2018

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks -- fixed!

@Ocramius Ocramius self-assigned this Jun 22, 2018
@Ocramius Ocramius added the bug label Jun 22, 2018
@Ocramius Ocramius added this to the 2.7.9 milestone Jun 22, 2018
Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

🚢

@Ocramius Ocramius merged commit a5cfced into zendframework:release-2.7 Jun 22, 2018
@Ocramius Ocramius changed the title Undefined $factory notice fix Undefined $factory notice fix Jun 22, 2018
@Ocramius Ocramius modified the milestones: 2.7.9, 2.7.10, 2.7.11 Jun 22, 2018
@Ocramius
Copy link
Member

Thanks @demiankatz! Released in 2.7.11 :-)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants