Do not exit from loadClass() early #5783

Merged
merged 2 commits into from Feb 5, 2014

3 participants

@weierophinney
Zend Framework member

In loadClass(), if the namespace/prefix matches, but the classfile is not
found, we were returning a boolean false immediately. However, if a more
specific namespace/prefix matches, but is later in the stack, we're
essentially ignoring it, making the class unreachable.

This patch removes the return false line in the foreach loop, allowing a
namespace registered later to match and potentially fulfill the autoloading.

There will be a performance impact, which means it's always better to
register the more specific namespace earlier.

@weierophinney weierophinney Do not exit from loadClass() early
In loadClass(), if the namespace/prefix matches, but the classfile is not
found, we were returning a boolean false immediately. However, if a more
specific namespace/prefix matches, but is later in the stack, we're
essentially ignoring it, making the class unreachable.

This patch removes the `return false` line in the `foreach` loop, allowing
a namespace registered later to match and potentially fulfill the autoloading.

There _will_ be a performance impact, which means it's always better to register
the more specific namespace earlier.
4de2950
@Ocramius Ocramius and 1 other commented on an outdated diff Feb 4, 2014
tests/ZendTest/Loader/StandardAutoloaderTest.php
@@ -190,4 +190,12 @@ public function testCanTellAutoloaderToRegisterZendNamespaceAtInstantiation()
$this->assertAttributeEquals($expected, 'namespaces', $loader);
}
+ public function testWillLoopThroughAllNamespacesUntilMatchIsFoundWhenAutoloading()
+ {
+ $loader = new StandardAutoloader();
+ $loader->registerNamespace('ZendTest\Loader\TestAsset\Parent', __DIR__ . '/TestAsset/Parent');
+ $loader->registerNamespace('ZendTest\Loader\TestAsset\Parent\Child', __DIR__ . '/TestAsset/Child');
+ $loader->autoload('ZendTest\Loader\TestAsset\Parent\Child\Subclass');
@Ocramius
Zend Framework member
Ocramius added a note Feb 4, 2014

Can you assert on the return value? Can't really know if it was autoloaded by a different loader, no?

@weierophinney
Zend Framework member

The fact that no PHP errors are raised is indicative of the fact that it did not attempt to include a file it could not. However, I can likely do an assertion against !== false to validate; will update with that shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@EvanDotPro
Zend Framework member

Just to clarify here for anyone who references back to this PR later, the performance impact mentioned by @weierophinney is only applicable if you fall into this scenario where you do in fact have a more specific, overlapping namespace registered later. This is a very, very specific edge-case and unlikely to affect any current applications in practice (they would have simply ran into this bug). 😄

@ralphschindler ralphschindler added a commit that referenced this pull request Feb 5, 2014
@ralphschindler ralphschindler Merge PR #5783 to develop
Merge branch 'hotfix/standard-autoloader-stack' of git://github.com/weierophinney/zf2 into weierophinney-hotfix/standard-autoloader-stack

* 'hotfix/standard-autoloader-stack' of git://github.com/weierophinney/zf2:
  [#5783] Assertion on return value
  Do not exit from loadClass() early
2ae43e3
@ralphschindler ralphschindler added a commit that referenced this pull request Feb 5, 2014
@ralphschindler ralphschindler Merge PR #5783 to develop
Merge branch 'weierophinney-hotfix/standard-autoloader-stack' into develop

* weierophinney-hotfix/standard-autoloader-stack:
  [#5783] Assertion on return value
  Do not exit from loadClass() early
4741ba7
@ralphschindler ralphschindler merged commit 70cd8a5 into zendframework:develop Feb 5, 2014

1 check passed

Details default The Travis CI build passed
@ralphschindler ralphschindler added this to the 2.3.0 milestone Feb 5, 2014
@ralphschindler ralphschindler self-assigned this Feb 5, 2014
@weierophinney weierophinney deleted the weierophinney:hotfix/standard-autoloader-stack branch Feb 5, 2014
@weierophinney weierophinney modified the milestone: 2.3.0 Feb 5, 2014
@weierophinney weierophinney added a commit to zendframework/zend-loader that referenced this pull request May 15, 2015
@weierophinney weierophinney [zendframework/zendframework#5783] Assertion on return value
- per @Ocramius
967f5f7
@gianarb gianarb pushed a commit to zendframework/zend-loader that referenced this pull request May 15, 2015
@ralphschindler ralphschindler Merge PR zendframework/zendframework#5783 to develop
Merge branch 'hotfix/standard-autoloader-stack' of git://github.com/weierophinney/zf2 into weierophinney-hotfix/standard-autoloader-stack

* 'hotfix/standard-autoloader-stack' of git://github.com/weierophinney/zf2:
  [zendframework/zendframework#5783] Assertion on return value
  Do not exit from loadClass() early
3d70932
@gianarb gianarb pushed a commit to zendframework/zend-loader that referenced this pull request May 15, 2015
@ralphschindler ralphschindler Merge PR zendframework/zendframework#5783 to develop
Merge branch 'weierophinney-hotfix/standard-autoloader-stack' into develop

* weierophinney-hotfix/standard-autoloader-stack:
  [zendframework/zendframework#5783] Assertion on return value
  Do not exit from loadClass() early
0f7d635
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment