php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #55475 is_a() triggers autoloader
Submitted: 2011-08-22 08:16 UTC Modified: 2011-11-09 05:27 UTC
Votes:29
Avg. Score:4.5 ± 1.0
Reproduced:25 of 27 (92.6%)
Same Version:17 (68.0%)
Same OS:11 (44.0%)
From: mads at gartneriet dot dk Assigned: dmitry
Status: Closed Package: Scripting Engine problem
PHP Version: 5.3.7 OS:
Private report: No CVE-ID: 2011-3379
 [2011-08-22 08:16 UTC] mads at gartneriet dot dk
Description:
------------
When calling is_a() with a first argument that is not an object, then __autoload() is triggered:



Test script:
---------------
<?php

function __autoload($class) {
	echo "Would load: " . $class . PHP_EOL;
}

$var = "test";
var_dump(is_a($var, 'B'));

$obj = new Stdclass;
var_dump(is_a($obj, 'C'));

?>

Expected result:
----------------
bool(false)
bool(false)

Actual result:
--------------
Would load: test
bool(false)
bool(false)


Patches

final_patch_for_5_4_and_HEAD_v2 (last revision 2011-11-08 09:24 UTC) by alan_k@php.net)
is_a_5.4_alternative (last revision 2011-10-17 13:15 UTC) by jbondc at openmv dot com)
final_patch_for_5_4_and_HEAD (last revision 2011-10-13 07:36 UTC) by alan_k@php.net)
is_a_with_warning.txt (last revision 2011-09-25 09:32 UTC) by alan_k@php.net)
Is_a_with_allow_string_argument_v3 (last revision 2011-09-22 23:31 UTC) by alan_k@php.net)
Is_a_with_allow_string_argument_v2 (last revision 2011-09-22 23:26 UTC) by alan_k@php.net)
Is_a_with_allow_string_argument (last revision 2011-09-22 23:24 UTC) by alan_k@php.net)
is_class_of.diff (last revision 2011-09-20 21:32 UTC) by alan_k@php.net)
is_class_of.txt (last revision 2011-09-20 21:25 UTC) by alan_k@php.net)
revert.is_a.behaviour.to.ignoring.strings.diff (last revision 2011-08-25 02:37 UTC) by alan at akbkhome dot com)
bug55475 (last revision 2011-08-22 10:30 UTC) by kalle@php.net)

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2011-08-22 08:19 UTC] pajoye@php.net
-Status: Open +Status: Assigned -Assigned To: +Assigned To: dmitry
 [2011-08-22 08:19 UTC] pajoye@php.net
Related to change for the #53727 fix.

http://svn.php.net/viewvc/php/php-
src/branches/PHP_5_3/Zend/zend_builtin_functions.c?r1=307522&r2=312904

Assigned to Dmitry
 [2011-08-22 08:57 UTC] konstantin dot leboev at gmail dot com
I guess it's not a bug. The first argument can be a class name, what will be 
loaded only on calling this function.
 [2011-08-22 09:15 UTC] colder@php.net
1) The underlying implementation is shared between is_a and is_subclass_of.
2) Previously, strings as first argument was not permitted by is_a but
was for is_subclass_of,
3) is_subclass_of always triggered autoload in such cases.
4) Following a fix from Dmitry, the underlying implementation now
allows a string as first argument for is_a as well.

Conclusion: it is now consistent, but if you wrongly used is_a with a
string before, it now triggers autoload because it actually accepts
it.
 [2011-08-22 10:12 UTC] zeev@php.net
Discussed with Dmitry, the current functionality appears to be correct.

is_a('foo', 'bar') *can* be true even if class foo isn't loaded, and we'll only 
know that if we try to load 'foo'.

This is different from is_a($obj, 'non_existent_class'), which we can resolve as 
'false' in case non_existent_class isn't loaded without trying to load it (there 
can't be an instance of a class that doesn't exist).
 [2011-08-22 10:29 UTC] kalle@php.net
Zeev, although the functionality might appear as it should be then we should not make sudden changes like that in the middle of a stable branch, we should patch up 5.3 and keep the behaviour in 5.4 if its indeed intended atleast to comply with previous versions.

The fix for 5.3 is simple, attached
 [2011-08-22 10:30 UTC] kalle@php.net
The following patch has been added/updated:

Patch Name: bug55475
Revision:   1314009019
URL:        https://bugs.php.net/patch-display.php?bug=55475&patch=bug55475&revision=1314009019
 [2011-08-22 11:00 UTC] dmitry@php.net
Before the patch, is_a() didn't accept string as the first argument at all, so it always returned "false" and never triggered __autoload(). The proposed patch didn't revert to original behavior. It just disables autoloading and may lead to wrong result.

class a {}
class b extends a {}
var_dump(is_a("b", "a")); // it was false before 5.3.7, now it's true

I would say that the old behaviour was wrong, especially because "instanceof" and is_subclass_of() already implemented support for string arguments.
 [2011-08-22 13:31 UTC] colder@php.net
It seems correct to me as well to trigger autoload in this case. It does and 
always did so for is_subclass_of(), I don't see any reason for a condition of 
"subclasses_only" to yes or no trigger the autoload.
 [2011-08-22 13:41 UTC] kalle@php.net
I'm not arguing that the new behaviour is wrong, I believe it is the desired too but I don't agree to break BC in the middle of a stable release series nor as much as I would like to myself to achieve the right behaviour.
 [2011-08-22 14:27 UTC] colder@php.net
But what BC break are you talking about exactly?

It went from not-working (returning always false for strings as first argument) 
to working with autoload.
 [2011-08-22 14:40 UTC] kalle@php.net
I'm talking about the usual procedure we have about changing behaviour, a function suddenly returns the oppersite of what it used to in the middle of a stable series is very unlike to do, even for PHP.

I knnow it went from not working to working, but I don't on the fact that such a commonly used function will change behaviour like that. What we should do is to make a big fat warning in the migration guide for 5.3.x -> 5.4.x about it, and in the manual.

It would be the same if we changed substr() to be case insensitive in the middle of a release series, lets just not venture in such dark corners.
 [2011-08-22 14:44 UTC] kalle@php.net
... the behaviour I'm talking about is obvious the return value and the fact that the autoloader now is called.
 [2011-08-22 14:49 UTC] colder@php.net
Well, we have 3 options here:

1) keep it like it is since 5.3.7
2) reverting it to how it worked before 5.3.7
3) change it even more to not use autoload, so that it neither works like <5.3.6 nor 5.3.7

Apparently through your proposed fix you're advocating for (3). If so, I can't see how it would improve the situation in 
any way.

Personally, given that the BC change is minimal, and that we're only adding functionality, (1) seems fine.
Correct code existing before 5.3.6 will work just fine anyway.
 [2011-08-22 15:46 UTC] johannes@php.net
is_a()'s first argument is documented to be an object. If called with a string, following the documentation, I would actually expect a "Warning: is_a() expects parameter 1 to be object, string given" and return NULL.

That aside and looking at the actual behavior: Previously is_a() could be used to check whether the parameter is an object AND of a specific type in one go. This can't be done anymore. In $a = "test"; is_a($a, "foo"); test might be an existing class and might be of type foo. Now people have to do is_object() && is_a().

I don't like having such behavior change in bug fix versions as I don't like going back and forth which is annoying for documentation and confusing for users. I would love to keep it out of 5.3.8 to have that as low risk quick release for the hash issue. Which means a rollback to the old way is even harder to do. (two versions with the new behavior out)
 [2011-08-22 18:36 UTC] stas@php.net
This is not a bug. If first argument is a string, it is interpreted as a class 
name and autoloader is called for it. Actually, IIRC, one the reasons why is_a was 
un-deprecated is that it can work with strings.
 [2011-08-22 19:17 UTC] mads at gartneriet dot dk
Maybe not a bug, but it is behaving different ind 5.3.7 than in the previous versions, which makes some of the code from PEAR that i use, give errors.
 [2011-08-22 21:46 UTC] colder@php.net
What code? Do you have some example?
 [2011-08-23 05:17 UTC] mads at gartneriet dot dk
DB_DataObject uses is_a() to check if a variable is both an object and an instance of a particular object.
PEAR::isError() does too.

This just gives warnings in my code, and I could of course easily fix these two places in my local pear-code. But then it will bite me the next time I upgrade those packages from PEAR.
 [2011-08-23 06:26 UTC] alan at akbkhome dot com
From the manual.

"Returns TRUE if the object is of this class or has this class as one of its 
parents, FALSE otherwise."

note the "FALSE otherwise" ...

Defiantly a bug..
 [2011-08-23 08:25 UTC] alan at akbkhome dot com
@stas - the point of un-depreciating and working with a string was the second 
argument, not the first one. 

eg.

is_a($something, 'might_be_not_loaded_class') 
rather than
$something instanceof might_be_not_loaded_class
 [2011-08-23 14:24 UTC] jha dot rajeev at gmail dot com
This also affects HTML_Template_Flexy pear package that uses is_a to check 
returned object against PEAR_Error class. No matter what behavior is right it 
looks broken to me because I am patching this pear packages files right now!
 [2011-08-24 01:59 UTC] alan at akbkhome dot com
For reference: 
The workaround is to do this 

if (is_a($a, 'B')) {

becomes

if (is_object($a) && is_a($a, 'B')) {
 [2011-08-24 05:16 UTC] jha dot rajeev at gmail dot com
I have a question re. the correct behavior of custom __autoload() functions when 
called from is_a() in 5.3.7. How do we handle/report missing classes? is is_a() 
prepared to handle any sort of exceptions or does it assume that __autoload will 
return TRUE/FALSE only?

what if I just did something like is_a("",ABCD)?
 [2011-08-26 10:24 UTC] kkaminski at itens dot pl
+1 for reverting change in 5.3 branch and implementing it in 5.4 (or giving up as it really CHANGES BEHAVIOR)
Currently __autoload throws Exceptions to break code execution on some frameworks. This is clean solution as if developer makes a typo, code still can handle missing class and for instance - display a dedicated error report.

Unfortunately, with your latest 'fix' all PEAR packages are now broken on frameworks with __autoload + exceptions - due to isError implementation.

Are you really sure is it MY duty to rewrite / repatch all code (external) code to work around your 'fix' ?
How I am supposed to handle missing classes in this case? With exceptions I can catch everything and handle myself. Whats the other way?
 [2011-08-29 07:15 UTC] tyrael@php.net
"note the "FALSE otherwise" ..."

note "if the object" ...

Tyrael
 [2011-09-07 06:30 UTC] vchernoivan at gmail dot com
I guess it is no use to argue if the behaviuor is correct or not, or how precise 
the manual is. Since IT IS BREAKING EXISTING CODE, for me, too.
Before the change
   if (is_a($date,"DateTime"))
       return $date->format(...);
   /// some code handling datetime strings
worked just fine. Now it triggers __autoload and results in completely broken 
page. 
For sure, personally I can  change every piece of MY OWN code. 
But consider users of tons of PHP libraries! 
What do you think, how long will it take to update every piece of them?
Vote for reverting to prior-5.7 behavior until 5.4
 [2011-09-15 09:58 UTC] dmitry@php.net
Automatic comment from SVN on behalf of dmitry
Revision: http://svn.php.net/viewvc/?view=revision&amp;revision=316810
Log: Fixed bug #55475 (is_a() triggers autoloader). (alan at akbkhome dot com)
 [2011-09-15 10:00 UTC] dmitry@php.net
-Status: Assigned +Status: Closed
 [2011-09-15 10:00 UTC] dmitry@php.net
I've committed the revert.is_a.behaviour.to.ignoring.strings.diff by alan at akbkhome dot com into 5.3.

5.4 is going to support string argument.
 [2011-09-15 10:59 UTC] dmitry@php.net
Automatic comment from SVN on behalf of dmitry
Revision: http://svn.php.net/viewvc/?view=revision&amp;revision=316811
Log: Reverted the fix for #55475 (is_a() triggers autoloader) before the common decision
 [2011-09-15 11:00 UTC] dmitry@php.net
-Status: Closed +Status: Assigned
 [2011-09-15 11:00 UTC] dmitry@php.net
Reverted before the common decision.
 [2011-09-20 21:25 UTC] alan_k@php.net
The following patch has been added/updated:

Patch Name: is_class_of.txt
Revision:   1316553958
URL:        https://bugs.php.net/patch-display.php?bug=55475&patch=is_class_of.txt&revision=1316553958
 [2011-09-20 21:30 UTC] alan_k@php.net
Attached now is a patch that fixes this by adding

is_class_of

Which behaves the same as is_subclass_of, (autoload/ accepts strings)

It also fixes the documentation on is_subclass_of and reverts the behaviour of 
is_a

Note: the is_a change is now a security bug as sending url's to is_a may trigger 
remote code execution now.

Note: I'm not sure you can classify a 2-3 developers comments as the "common 
decision" there where objections to the original change from core developers, 
this patch gives everybody what they want....
 [2011-09-20 21:32 UTC] alan_k@php.net
The following patch has been added/updated:

Patch Name: is_class_of.diff
Revision:   1316554378
URL:        https://bugs.php.net/patch-display.php?bug=55475&patch=is_class_of.diff&revision=1316554378
 [2011-09-22 23:24 UTC] alan_k@php.net
The following patch has been added/updated:

Patch Name: Is_a_with_allow_string_argument
Revision:   1316733848
URL:        https://bugs.php.net/patch-display.php?bug=55475&patch=Is_a_with_allow_string_argument&revision=1316733848
 [2011-09-22 23:26 UTC] alan_k@php.net
The following patch has been added/updated:

Patch Name: Is_a_with_allow_string_argument_v2
Revision:   1316733980
URL:        https://bugs.php.net/patch-display.php?bug=55475&patch=Is_a_with_allow_string_argument_v2&revision=1316733980
 [2011-09-22 23:31 UTC] alan_k@php.net
The following patch has been added/updated:

Patch Name: Is_a_with_allow_string_argument_v3
Revision:   1316734303
URL:        https://bugs.php.net/patch-display.php?bug=55475&patch=Is_a_with_allow_string_argument_v3&revision=1316734303
 [2011-09-23 09:51 UTC] rasmus@php.net
Automatic comment from SVN on behalf of rasmus
Revision: http://svn.php.net/viewvc/?view=revision&amp;revision=317183
Log: Re-committing Alan's is_a revert/fix for bug #55475
Dmitry had done so earlier, but reverted pending discussion.
It is completely clear that this should never have been changed in the
5.3 branch in the first place giving the number of things that broke
because of it.
 [2011-09-24 09:22 UTC] henri at nerv dot fi
Has someone requested CVE-identifier for this issue? I can do it if not.
 [2011-09-24 13:13 UTC] cipri@php.net
Yes, I contacted the CVE yesterday to request a CVE-ID and  I'll update it here as soon as I receive one.
 [2011-09-25 09:32 UTC] alan_k@php.net
The following patch has been added/updated:

Patch Name: is_a_with_warning.txt
Revision:   1316943145
URL:        https://bugs.php.net/patch-display.php?bug=55475&patch=is_a_with_warning.txt&revision=1316943145
 [2011-09-26 19:38 UTC] togos00 at gmail dot com
Even if the new behavior is not a bug, per se, it is definitely surprising.  
is_a( $string, $className ) returning true would imply that $string is an 
instance of $className, which obviously it is not, as it is a string and not even 
an object.  Having a separate function such as is_subclass_of( $className1, 
$className2 ) has the dual benefits of being more intuitive and not breaking old 
code.
 [2011-09-26 19:45 UTC] pajoye@php.net
@cipri

Please contact security@php.net prior to request a CVE, to avoid double requests 
or confusing information. or mark a bug as security issue so we will catch it (and 
the sec guys of the linux distro as well) :)
 [2011-09-26 19:54 UTC] henri at nerv dot fi
CVE already requested with A LOT of conversation: 
http://www.openwall.com/lists/oss-security/2011/09/24/2
 [2011-09-26 19:57 UTC] pajoye@php.net
that's what I meant.
 [2011-09-27 09:35 UTC] alan_k@php.net
Automatic comment from SVN on behalf of alan_k
Revision: http://svn.php.net/viewvc/?view=revision&amp;revision=317382
Log: document fix for #55475 in NEWS
 [2011-09-27 18:36 UTC] pajoye@php.net
-CVE-ID: +CVE-ID: 2011-3379
 [2011-09-27 18:36 UTC] pajoye@php.net
Add CVE #
 [2011-10-03 07:30 UTC] alan_k@php.net
Any comments on 5.4.*

It seems like applying the 5.3 fix to 5.4 is the only option here, as there is no 
'reasonable' way to flag the previous behavior as E_DEPRECIATED that works well 
as both forward and backward compatible.
 [2011-10-13 07:36 UTC] alan_k@php.net
The following patch has been added/updated:

Patch Name: final_patch_for_5_4_and_HEAD
Revision:   1318491419
URL:        https://bugs.php.net/patch-display.php?bug=55475&patch=final_patch_for_5_4_and_HEAD&revision=1318491419
 [2011-11-08 09:24 UTC] alan_k@php.net
The following patch has been added/updated:

Patch Name: final_patch_for_5_4_and_HEAD_v2
Revision:   1320744263
URL:        https://bugs.php.net/patch-display.php?bug=55475&patch=final_patch_for_5_4_and_HEAD_v2&revision=1320744263
 [2011-11-09 05:27 UTC] stas@php.net
Automatic comment from SVN on behalf of stas
Revision: http://svn.php.net/viewvc/?view=revision&amp;revision=318938
Log: fix bug #55475 - implement is_a BC solution
 [2011-11-09 05:27 UTC] stas@php.net
This bug has been fixed in SVN.

Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.

 For Windows:

http://windows.php.net/snapshots/
 
Thank you for the report, and for helping us make PHP better.


 [2011-11-09 05:27 UTC] stas@php.net
-Status: Assigned +Status: Closed
 [2012-01-09 08:27 UTC] counterpoint at aliro dot org
It's worrying that something that appears to have been accidentally introduced is then justified as how things should work.  Little attention seems to be paid to how people may have been using the mechanism that has changed.  The whole thrust in this area was for is_a to test something that is supposed to be an instance.  Indeed, the at one time intended replacement for is_a is called "instanceof" and a string is not an instance of anything.  Changing an operation called "instanceof" to accept a class name ahead of the operator would seem perverse in the extreme.  Clearly the altered behavior of is_a may break any autoloader that assumes that it will only ever receive strings that can be assumed to be class names.  In particular, consider the common case where a set of class names can be mapped to a set of file names: in this situation, it is not unreasonable for the autoloader to terminate immediately if it receives a class name that contains ".." because this is very likely a hack attempt.
 [2012-04-18 09:47 UTC] laruence@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=10f20585fcbd524016e439c17bf01a0fd5191107
Log: fix bug #55475 - implement is_a BC solution
 [2012-07-02 10:30 UTC] rmc1134 at gmail dot com
I don't get it: why should is_a() accept a string as its first argument?

This function is supposed to be a check on AN OBJECT and is_a('stdClass', 
'stdClass') SHOULD NOT EVER RETURN any truthy value.

Calling __autoload() to check whether some string might be some object is 
something a programmer should do, not the language.
 [2012-07-24 23:39 UTC] rasmus@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=10f20585fcbd524016e439c17bf01a0fd5191107
Log: fix bug #55475 - implement is_a BC solution
 [2013-01-28 17:09 UTC] martijn dot niji at gmail dot com
I must agree with the people who say that is_a('stdClass', 'stdClass') should not return true.

The is_a() function is intended as a function that checks a certain condition is true, that condition being that a certain variable/object is of a certain type/class.

Having is_a() try to automatically load a class by calling autoload is bad behaviour at best and a code breaking security flaw at worst.
 [2013-11-17 09:35 UTC] laruence@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=10f20585fcbd524016e439c17bf01a0fd5191107
Log: fix bug #55475 - implement is_a BC solution
 
PHP Copyright © 2001-2014 The PHP Group
All rights reserved.
Last updated: Wed Apr 23 07:02:14 2014 UTC