php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #74372 autoloading file with syntax error uses next autoloader, may hide parse error
Submitted: 2017-04-04 20:31 UTC Modified: 2017-12-06 22:39 UTC
From: lightnb at bellsouth dot net Assigned: nikic (profile)
Status: Closed Package: SPL related
PHP Version: 7.0.17 OS: Ubuntu 16.04
Private report: No CVE-ID: None
 [2017-04-04 20:31 UTC] lightnb at bellsouth dot net
Description:
------------
Attempting to autoload a class file with a syntax error causes PHP to attempt to use the next autoloader rather than throwing the syntax error.

Test script:
---------------
// Use built in autoloader for 99% of loads
spl_autoload_register(); 
// If that fails, see if it's a rare special case
spl_autoload_register('AutoLoadFallback');

function AutoLoadFallback($ClassName){
    // ... try stuff...
    throw new Exception('Could not autoload the class definition for "'.$ClassName.'"');
}


// Try to load a new widget object
$oWidget = new Widget();

Expected result:
----------------
In this case, assume that the Widget object has a file that the default autoloader can find, but the file contains a syntax error.

The expected result would be a syntax error thrown for the faulty file.

But what actually happens is, when an autoload file is found by the default autoload but a syntax error exists in it, PHP calls the next autoloader in the chain AutoLoadFallback, which of course throws an exception because it only handles cases that the default autoloader can't.

The result is getting an exception thrown by the AutoLoadFallback function, rather than a parse error being thrown when the default autoloader tries to load the class. This blocks me from seeing the real error and line number.

If I comment out the spl_autoload_register('AutoLoadFallback'); line, it throws a parse error per normal.


Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2017-04-04 21:01 UTC] requinix@php.net
-Summary: autoloading file with syntax error attempts to use next autoloader +Summary: autoloading file with syntax error uses next autoloader, may hide parse error -Status: Open +Status: Feedback
 [2017-04-04 21:01 UTC] requinix@php.net
I'm seeing the parse error just fine. https://3v4l.org/NSaTZ

Are you catching the exception? Are you making sure to check for previous exceptions? https://3v4l.org/DXGtj
 [2017-04-04 21:16 UTC] lightnb at bellsouth dot net
-Status: Feedback +Status: Open
 [2017-04-04 21:16 UTC] lightnb at bellsouth dot net
The first call to spl_autoload_register(); should have no arguments to indicate the default loader should be tried first.

You'll probably need to have an actual script with an actual second file that gets autoloaded in order to reproduce this. The issue does not occur if all calls to spl_autoload_register() have a function argument. It only occurs if you want to use the default autoloader in conjunction with a fallback handler.
 [2017-04-04 21:33 UTC] nikic@php.net
@requinix: I'd consider that a bug. If the first autoloader throws, the second one should not be executed. But ... it looks like this behavior was intentionally introduced some nine years ago: https://github.com/php/php-src/commit/e4869828a7905af350afe1870c64651578497a6c At this point this bug has probably become a "feature" :/
 [2017-04-04 23:12 UTC] lightnb at bellsouth dot net
It seems like PHP should differentiate between a user-thrown exception and a parse error in a found file. (Although I'm still not sure what use-case would require autoload execution to continue after an exception is thrown, especially an exception throw by PHP's built-in autoloader.)
 [2017-04-05 06:09 UTC] requinix@php.net
-Status: Open +Status: Feedback
 [2017-04-05 06:09 UTC] requinix@php.net
@lightnb: I had tried with the standard autoloader first. Same thing happens. The issue is in the autoloading process, not spl_autoload() itself.

Fatal error: Uncaught ParseError: syntax error, unexpected end of file, expecting function (T_FUNCTION) in /root/php/PHP-7.0.17/widget.php:4
Stack trace:
#0 [internal function]: spl_autoload('Widget')
#1 /root/php/PHP-7.0.17/bug.php(15): spl_autoload_call('Widget')
#2 {main}

Next Exception: Could not autoload the class definition for "Widget" in /root/php/PHP-7.0.17/bug.php:10
Stack trace:
#0 [internal function]: AutoLoadFallback('Widget')
#1 /root/php/PHP-7.0.16/bug.php(15): spl_autoload_call('Widget')
#2 {main}
  thrown in /root/php/PHP-7.0.17/bug.php on line 10


Bundling the exceptions is a bit unusual for PHP - normally it would be one error at a time - but I don't think there's any harm in it being there. An exception still gets thrown if one occurs, with the difference being that autoloading continues and therefore the class could still be loaded before the exception goes up the call stack.
https://3v4l.org/IlUiA
 [2017-04-05 14:18 UTC] lightnb at bellsouth dot net
-Status: Feedback +Status: Open
 [2017-04-05 14:18 UTC] lightnb at bellsouth dot net
If that's the intended behavior, there should be a warning on the documentation page (http://php.net/manual/en/function.spl-autoload-register.php) that informs developers they need to have a custom error handler that shows all exceptions not just the last exception if they want to know what actually went wrong when using multiple autoloaders.

The examples on PHP's exception page (http://php.net/manual/en/language.exceptions.php), along with most other examples online, also don't mention the necessity for getPrevious() to find out what went wrong.

As far as I know, the last exception is always the "real" problem in all other cases.

> An exception still gets thrown if one occurs, with the difference being that autoloading 
> continues and therefore the class could still be loaded before the exception goes up the call 
> stack.


If the class file has a parse error, it's unlikely that another auto-loader would be able to load it instead.

I'm still not sure why anyone would want errors ignored and another autoloader tried when an error occurs. If that was their intent, they could always use a try/catch in a custom autoload function and suppress the error themselves. But it seems like the default behavior should be to stop on errors since that would be the consistent and expected behavior.
 [2017-12-06 22:37 UTC] nikic@php.net
Automatic comment on behalf of nikita.ppv@gmail.com
Revision: http://git.php.net/?p=php-src.git;a=commit;h=9cdd547ecad81fb4ea499db2f9a62be55b54390f
Log: Fixed bug #74372
 [2017-12-06 22:37 UTC] nikic@php.net
-Status: Open +Status: Closed
 [2017-12-06 22:39 UTC] nikic@php.net
-Assigned To: +Assigned To: nikic
 [2017-12-06 22:39 UTC] nikic@php.net
I've decided to fix this bug in master, despite the minor BC impact. An exception is an exception and it better behave like one.
 
PHP Copyright © 2001-2018 The PHP Group
All rights reserved.
Last updated: Thu Dec 13 22:01:26 2018 UTC