Zend Framework

Zend_Loader_Autoloader_Resource::autoload() should return false if no match is found

Details

  • Type: Bug Bug
  • Status: Resolved Resolved
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.9.5
  • Fix Version/s: 1.9.6
  • Component/s: Zend_Loader
  • Labels:
    None

Description

Currently (r19141 of the standard trunk), the resource autoloader can produce some unexpected PHP warnings if it accidentally attempts to autoload something that doesn't exist. E.g.:

1. Create and configure an instance of the resource autoloader; for example:

require_once 'Zend/Loader/Autoloader.php';
require_once 'Zend/Loader/Autoloader/Resource.php';
$autoloader = new Zend_Loader_Autoloader_Resource(array(
    'namespace' => 'My',
    'basePath' => '/path/to/my',
));$autoloader->addResourceType('resource', 'resources', 'Resource');

2. Attempt to instantiate a class that doesn't exist, but would be in the configured namespace:

$obj = new My_Resource_DoesNotExist();

At this point you'll get two PHP errors. The first is an E_WARNING message indicating that you tried to include a file that doesn't exist; the second is an E_FATAL message indicating that you tried to instantiate a class that doesn't exist.

However, if the behavior of Zend_Loader_Autoloader::autoload() is any indicator, Zend_Loader_Autoloader_Resource::autoload() should instead return false if the expected class cannot be found. This will get rid of the first E_WARNING message and enable a bit more flexible error handling.

For an example of why this would be helpful, please take a look at http://www.doctrine-project.org/jira/browse/DC-137 : Doctrine has a feature that will generate a certain class on the fly if it can't be autoloaded; if the current ZF resource autoloader is registered, this will still work correctly ...but not without generating the E_WARNING message mentioned earlier.

Thanks!
Adam

  1. library.diff
    20/Nov/09 9:26 PM
    0.8 kB
    Adam Jensen
  2. varname.diff
    23/Nov/09 2:31 AM
    0.3 kB
    Erik Wegner

Activity

Hide
Adam Jensen added a comment -

On further inspection, I realized it's not the return value of autoload() that's causing this ...in fact, it's returning false for classes-not-found, just as documented.

However, I do think it would be worth avoiding the E_WARNING messages. Developers will still get enough error information out of the E_FATAL message that follows it when a non-existing class is being instantiated ...and in the meantime, other situations (like Doctrine's use of class_exists()) would still function gracefully without emitting any errors.

I should be attaching a patch shortly; if this sounds like a good idea, it'd be a pretty easy fix.

Thanks!
Adam

Show
Adam Jensen added a comment - On further inspection, I realized it's not the return value of autoload() that's causing this ...in fact, it's returning false for classes-not-found, just as documented. However, I do think it would be worth avoiding the E_WARNING messages. Developers will still get enough error information out of the E_FATAL message that follows it when a non-existing class is being instantiated ...and in the meantime, other situations (like Doctrine's use of class_exists()) would still function gracefully without emitting any errors. I should be attaching a patch shortly; if this sounds like a good idea, it'd be a pretty easy fix. Thanks! Adam
Hide
Adam Jensen added a comment -

OK, I've attached my patch ...I made this from the library dir of the standard trunk; is that the right way to do that?

In any case, this should prevent any superfluous E_WARNINGs by avoiding the include call unless the path is valid.

Thanks!
Adam

Show
Adam Jensen added a comment - OK, I've attached my patch ...I made this from the library dir of the standard trunk; is that the right way to do that? In any case, this should prevent any superfluous E_WARNINGs by avoiding the include call unless the path is valid. Thanks! Adam
Hide
Adam Jensen added a comment -

Found a couple of other issues that seem to be related to the same problem:

http://framework.zend.com/issues/browse/ZF-6607
http://framework.zend.com/issues/browse/ZF-6798

If my patch is accepted here, it may be worth double-checking if those two issues are resolved as well; I believe they will be.

Show
Adam Jensen added a comment - Found a couple of other issues that seem to be related to the same problem: http://framework.zend.com/issues/browse/ZF-6607 http://framework.zend.com/issues/browse/ZF-6798 If my patch is accepted here, it may be worth double-checking if those two issues are resolved as well; I believe they will be.
Hide
Adam Jensen added a comment -

Better patch ...in my first version, I rather unthinkingly left it calling getClassPath() twice; this one doesn't make that mistake.

Show
Adam Jensen added a comment - Better patch ...in my first version, I rather unthinkingly left it calling getClassPath() twice; this one doesn't make that mistake.
Hide
Adam Jensen added a comment -

Turns out the previously-attached patch didn't really solve the problem, because getClassPath() is apparently only returning false when the class name isn't in any of the autoloader's registered namespaces ...it still doesn't check to see if the file actually exists.

Seems this could be resolved by adding a file_exists() or Zend_Loader::isReadable() call at the end of getClassPath(); that way, the autoloader can simply fail silently for any files it can't figure out, allowing other autoloaders to take care of it if they can ...and if no registered autoloaders can handle it, the standard E_FATAL "class does not exist" error will still be emitted.

I've attached a fresh patch that should accomplish all this nicely enough.

Thanks!
Adam

Show
Adam Jensen added a comment - Turns out the previously-attached patch didn't really solve the problem, because getClassPath() is apparently only returning false when the class name isn't in any of the autoloader's registered namespaces ...it still doesn't check to see if the file actually exists. Seems this could be resolved by adding a file_exists() or Zend_Loader::isReadable() call at the end of getClassPath(); that way, the autoloader can simply fail silently for any files it can't figure out, allowing other autoloaders to take care of it if they can ...and if no registered autoloaders can handle it, the standard E_FATAL "class does not exist" error will still be emitted. I've attached a fresh patch that should accomplish all this nicely enough. Thanks! Adam
Hide
Marco Kaiser added a comment -

fixed in r19187 please check an close if finally fixed

Show
Marco Kaiser added a comment - fixed in r19187 please check an close if finally fixed
Hide
Erik Wegner added a comment -

Maybe the wrong variable name is used in line 195? It currently reads

return $path;

but shouldn't it be

return $classPath;

?

Show
Erik Wegner added a comment - Maybe the wrong variable name is used in line 195? It currently reads return $path; but shouldn't it be return $classPath; ?
Hide
Erik Wegner added a comment -

see comment

Show
Erik Wegner added a comment - see comment
Hide
Daniel Berstein added a comment -

Please commit Erik's patch, as it currently stands ZF autoloader is broken and you get errors/warnings like these:

Notice: Undefined variable: path in ...<path>/lib/Zend/Loader/Autoloader/Resource.php on line 195

Warning: Zend_Loader_Autoloader_Resource::include() [function.include]: Failed opening '' for inclusion (include_path='...<path>/lib:.:/usr/share/php') in ...<path>/lib/Zend/Loader/Autoloader/Resource.php on line 195

Fatal error: Class 'Default_Model_Abstract' not found in ...<path> on line 38

Thanks.

Show
Daniel Berstein added a comment - Please commit Erik's patch, as it currently stands ZF autoloader is broken and you get errors/warnings like these: Notice: Undefined variable: path in ...<path>/lib/Zend/Loader/Autoloader/Resource.php on line 195 Warning: Zend_Loader_Autoloader_Resource::include() [function.include]: Failed opening '' for inclusion (include_path='...<path>/lib:.:/usr/share/php') in ...<path>/lib/Zend/Loader/Autoloader/Resource.php on line 195 Fatal error: Class 'Default_Model_Abstract' not found in ...<path> on line 38 Thanks.
Hide
Marco Kaiser added a comment -

fixed the small typo in r19189

Show
Marco Kaiser added a comment - fixed the small typo in r19189

People

Vote (0)
Watch (3)

Dates

  • Created:
    Updated:
    Resolved: