Req #79623 [Opn->Wfx]: method_exists() is too strict in PHP 8
| From: | cmb@php.net | Date: | Wed, 27 May 2020 08:02:48 +0000 |
| Subject: | Req #79623 [Opn->Wfx]: method_exists() is too strict in PHP 8 | ||
| References: | 1 | Groups: | php.bugs |
| Request: | Send a blank email to php-bugs+get-227194@lists.php.net to get a copy of this message | ||
Edit report at https://bugs.php.net/bug.php?id=79623&edit=1
ID: 79623
Updated by: cmb@php.net
Reported by: nicolasgrekas@php.net
Summary: method_exists() is too strict in PHP 8
-Status: Open
+Status: Wont fix
Type: Feature/Change Request
-Package: *General Issues
+Package: Class/Object related
PHP Version: master-Git-2020-05-24 (Git)
-Assigned To:
+Assigned To: cmb
Block user comment: N
Private report: N
New Comment:
> Let's close then.
Okay, fine.
Previous Comments:
------------------------------------------------------------------------
[2020-05-24 10:56:33] nicolasgrekas@php.net
Thanks for the heads up.
For reference, here is the PR to fix the related failures in Symfony:
https://github.com/symfony/symfony/pull/36938
Let's close then.
------------------------------------------------------------------------
[2020-05-24 10:24:16] nikic@php.net
This change was originally made to align the behavior of method_exists() and proprety_exists(),
where the latter already only accepted an object|string argument.
However, I have found this change to have an additional benefit after the fact: The issue is that
method_exists() also accepts a string as first argument, and will treat it as a class name in that
case, which involves invoking the autoloader. This means that if you have code like
method_exists($arbitraryValue, 'method') you must make sure that at least
!is_string($arbitraryValue) holds beforehand -- at which point you might as well just test for
is_object($arbitraryValue). See https://github.com/guzzle/promises/pull/105/files
for an instance of such a bug going unnoticed for a long time. This is a pretty severe issue (those
autoloader invocations are definitely going to hurt performance, but may also impact security), but
the previous behavior of the function made it very easy to make it, because it made it look like
passing an arbitrary value to method_exists() is safe.
Now, looking at the particular warning in that travis log, I see that it is guarded by
"!is_scalar($default) && !method_exists($default, '__toString')" and as
such would not run into this issue. There's definitely false positives. But overall I still
think that the tradeoff here is reasonable.
------------------------------------------------------------------------
[2020-05-24 09:59:17] nicolasgrekas@php.net
Description:
------------
While working on making Symfony compatible with PHP 8, we're noticing a lot of failure that
look like:
TypeError: method_exists(): Argument #1 ($object_or_class) must be of type object|string, XXX given
See e.g. https://travis-ci.org/github/symfony/symfony/jobs/690567745
Can we relax method_exists() and make it accept any value as 1st arg?
None of the changes needed to remove these errors look valuable.
This would lower the cost of migrating to PHP 8 for the community at large.
(Symfony is just an early adopter here.)
Thanks for considering!
------------------------------------------------------------------------
--
Edit this bug report at https://bugs.php.net/bug.php?id=79623&edit=1