Published 2011-09-02 00:00:00

Well, for the first time in a very long while I had to post to the PHP core developers list last week, unfortunately the result of which was not particulary usefull.

The key issue was that 5.3.7 accidentally broke is_a() for a reasonably large number of users. Unfortunately the fixup release 5.3.8 did not address this 'mistake', and after a rather fruitless exchange I gave up trying to persuade the group (most people on mailing list), that reverting the change was rather critical (at least pierre supported reverting it in the 5.3.* series).

Anyway, what's this all about, basically if you upgrade to any of these versions and

a) use __autoload() 
or
b) any of your code calls is_a() on a string, 

you will  very likely get strange failures..


The change in detail.


in all versions of PHP since 4.2 the is_a signature looked like this

bool is_a ( object $object , string $class_name )

As a knock on effect from fixing a bug with is_subclass_of, somebody thought it was a good idea to make the two functions signature consistant, so in 5.3.7+ the signature is now

bool is_a ( mixed $object_or_string , string $class_name )

And to make matters worse, that change to the first 'object_or_string', will also call the autoloader if the class is not found.


How is_a() has been used in the past.


On the face of this, it would not seem like a significant change, however, you have to understand the history of is_a(), and why it was introduced. In the early days of PEAR (before PHP 4.2) there was a method called PEAR::isError($mixed), which contained quite a few tests to check if the $mixed was an object, and was an instance of 'PEAR_Error'. A while after PHP 4.2 was released, this was changed to use this new wonderfull feature, and basically became return is_a($mixed, 'PEAR_Error').

Since PEAR existed before exceptions (and is still a reasonable pattern to handle errors), It became quite common practice to have returns from methods which looked like this.

@return {String|PEAR_Error} $mixed  return some data..

So the callee would check the return using PEAR::isError(), or quite often just is_a($ret,'PEAR_Error'), if you knew that the PEAR class might not have been loaded. 

So now comes the change and let's see what happens.

The __autoload() issue.


Personally I never use __autoload, it's the new magic_quotes for me, making code unpredicatable and difficult to follow (read the post about require_once is part of your documentation). But anyway, each to their own, and for the PEAR packages I support I will usually commit any reasonable change that helps out people who are using autoload.

So there are users out there using autoload with my PEAR packages, as I quickly found last week. Quite a few of these packages use the is_a() pattern, and the users who had implemented __autoload() had very smartly decided that calling autoload with a name of a class that could not or did not exist was a serious error condition, and they either died, or threw exceptions.

Unfortunatly, since is_a() was sending all of the string data it got straight into __autoload(), this happened rather a lot. Leading to a run around hunt for all calls to is_a(), and code changes being put it to ensure that it never puts a string in the first argument.


The is_a(string) issue


While I'm not likely to see the autoload issue on my code, I'm not sure I really appreciate having to fix it so quickly without a timetable to change it. The other change that may cause random, undetectable bugs is the accepting a string.

imagine this bit of code

function nextTokString() { 
    if (!is_string($this->tok[$this->pos])) {
return PEAR::raiseError('....')
    }
    return $this->tok[$this->pos++];
}

... some code..
$tok =$this->nextTokString()
if (is_a($tok,'PEAR_Error')) {
    return $tok;
}
... do stuff with string.

Now what happens if the token is 'PEAR_Error', is_a() will now return true. The big issue with this is that unless you know about the is_a() change, this bug is going to be next to impossible to find.. No warning is issued, is_a() just silently returns true, where before it just returned false.

I was hoping that PHP 5.3.9 would go out the door with this reverted, or at least a warning stuck on string usage of is_a(), but nope, none of my efforts of persuasion appear to have worked.

While I do not think the change is particularly necessary (as the use case for the new signature is very rare, and acheivable in other ways), I think reverting this change before PHP 5.3.7+ went into major deployment is rather critical.  (yes it can take months before PHP releases start commonly arriving on servers). Then if it's deemed a necessary change (by vote) then go for it in 5.4... and add  a warning in the next version in the 5.3 series..

  

Anyway the fixes / workaround:


The simplest fix is to prepend tests with is_object

eg. 

if (is_a($tok,'PEAR_Error')) {

becomes

if (is_object($tok) && is_a($tok,'PEAR_Error')) {

if ( $tok instanceof PEAR_Error)) {


While you could start looking at the code and determining if you really need to prefix it with is_object(), the reality is unfortuntaly it may be simpler to stick this extra code in, just in case you start delivering strings where objects where expected.

Update


This has been fixed in 5.3.9, however part of this derives from some confusion over instanceof

When PHP5 was released and added instanceof, doing this when the class did not exist caused a fatal error.

if ( $tok instanceof Unknown_Class ) {

However, this was changed in 5.1 to not cause a fatal error, The documentation is not totally clear on this, especially for anyone who used PHP 5.0.*. 

Unfortunately, since migration times are slow, supporting 5.0-5.1 is a reality of life for anyone writing libraries (actually Most of the libraries I write for still provide support for PHP4). So using any 'new' feature of the language basically prevents you from supporting older version of PHP with new code.

In this case, PHP5 usage has slipped below 0.3% so removing support for this should be fine. 


Comments

Avoiding the autoload-issue in is_a
Hi,

you can even avoid the autoload issue:

if (is_object($tok) && class_exists($class, false) && is_a($tok, $class))
{
..
}


Another thing I would like to add:
it is bad practice to throw an exception, if the class was not found in __autoload. You destroy the whole possibility, to (dynamically) add multiple autoloaders via spl_autoload_register (which comes in quite handy some times, especially if you are using external libraries which only provide an autoloader).
#0 - Jannik ( Link) on 2011-09-02 14:58:11 Delete Comment
instanceof
And whats wrong with using instanceof
#1 - Petah ( Link) on 2011-09-03 04:33:59 Delete Comment
re: instanceof
instanceof relies on the class being tested either

a) existing (already loaded) .. or ..
b) being available via autoload

If you write fast efficient code, you do not want to load in extra code just to test a failure condition. hence is_a() has always been more sensible for a non-compiled language.
#2 - Alan Knowles ( Link) on 2011-09-03 10:51:39 Delete Comment
Thank you!
I found this dumb bug in an old library of xajax. Thank you for your post so that I have something to show people to explain the issue!
#3 - Tom ( Link) on 2011-09-13 02:43:09 Delete Comment
Reverted in 5.3.9
This change has been reverted in 5.3.9 - and a new option $allow_string is an optional argument to is_a() and is_subclass_of()

is_a() - it defaults to off (same as previous behavior)
is_subclass_of() - it defaults to on (same as previous behaviour)
#4 - Alan Knowles ( Link) on 2011-09-25 08:33:30 Delete Comment

Add Your Comment