php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Request #51527 is_callable() returning true for non-static callbacks
Submitted: 2010-04-10 16:07 UTC Modified: 2021-02-18 11:03 UTC
Votes:4
Avg. Score:4.5 ± 0.9
Reproduced:3 of 3 (100.0%)
Same Version:1 (33.3%)
Same OS:3 (100.0%)
From: weierophinney@php.net Assigned: danack (profile)
Status: Closed Package: Class/Object related
PHP Version: 5.3.2 OS: Linux
Private report: No CVE-ID: None
 [2010-04-10 16:07 UTC] weierophinney@php.net
Description:
------------
is_callable() is returning a false-positive for callbacks that reference non-
static methods. As an example, if I have defined a class 'Foo' with an instance 
(i.e., non-static) method 'bar', and define the callback "array('Foo', 'bar')", 
is_callable() will falsely return true.

Additionally, if you then pass this callback to call_user_func(), this latter 
function will actually try to call the method statically -- and raise an 
E_STRICT about the callback being invalid. If the method has any references to 
$this, it then fails with an E_FATAL, but otherwise it will run the method as if 
it were static.

This behavior is unexpected, and unintuitive. Calling non-static methods as if 
they were static, even when they do not reference $this, violates visibility. I 
would expect is_callable() to return false in these instances, and for 
call_user_func() to immediately raise an E_FATAL if the method is not defined as 
static.

Test script:
---------------
class Foo
{
    public function bar()
    {
        return 'foo bar';
    }
}

$callback = array('Foo', 'bar');
if (is_callable($callback)) {
    echo call_user_func($callback);
}

Expected result:
----------------
No output.

Actual result:
--------------
PHP Strict Standards:  call_user_func() expects parameter 1 to be a valid 
callback, non-static method Foo::bar() should not be called statically

Strict Standards: call_user_func() expects parameter 1 to be a valid callback, 
non-static method Test\Foo::bar() should not be called statically
foo bar

Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2010-04-10 16:29 UTC] pajoye@php.net
It is actually callable. But calling it statically breaks the strict standards, but the strict standards will break BC in many situations.

I would suggest to make this change in trunk only.
 [2010-04-10 16:43 UTC] weierophinney@php.net
@pajoye: Yes, this particular example was callable. However, if the method is 
actually an instance method, and has references to $this, is_callable() still 
returns true -- making it an invalid test to use to determine whether or not it's 
safe to then call call_user_func(). Instead, to really determine if the callback 
is valid, you have to start doing a bunch of reflection -- checking to see if the 
method is defined and static, or if there is a __callStatic defined that would 
intercept the call.

That's the more serious implication of the behavior, and why it needs to be 
fixed.
 [2010-04-10 17:55 UTC] johannes@php.net
-Status: Open +Status: Bogus
 [2010-04-10 17:55 UTC] johannes@php.net
The method can always be called statically. The access to $this might be forbidden but the method itself can be called. is_callable() doesn't mean it will execute properly. (the $this error is basically the same as a call to an undefined funciton in there or such)

To change this the only way would be to forbid calling non-static methods statically. Maybe this can be done nowadays (we needed the behavior for PHP 4 compatibility) but that's no bug but requires a RFC and discussion on internals as this might break quite a few applications (many PEAR-based things, many legacy applications not fully "converted" to PHP 5 ....)
 [2010-04-10 20:19 UTC] weierophinney@php.net
@johannes Perhaps an optional "strict" flag to is_callable() would address this? 
That would keep BC, while allowing for better checking for >= PHP-5 apps. Usage 
would be:

$callback = array('Foo', 'bar');

if (is_callable($callback, true)) {
    // Passed strict check
} else {
    // failed strict check
}

Thoughts?
 [2010-04-12 11:50 UTC] pajoye@php.net
-Status: Bogus +Status: Open
 [2010-04-12 11:50 UTC] pajoye@php.net
I don't like the optional flag idea. You can disable strictness using error_reporting already.

However I would like to see this change in trunk, can you come up with a RFC pls?
 [2010-05-05 07:03 UTC] philduby at phriendly dot net
Another variation that actually (unexpectedly) works:
Calling is_callable and call_user_func from inside an instance (non-static) method using any of: 'self::otherInstanceMethod', array('self','otherInstanceMethod'), array(self,'otherInstanceMethod') succeed.  It appears that (the context for) '$this' is carried over from the original method, even though the calls are being done statically.

Calling self::otherInstanceMethod() directly also succeeds.  It appears that methods called from an instance method 'inherit' the context for $this.  A bit unexpected, but *reasonable*.
Win XP SP3
PHP 5.3.1 (xampp)
 [2010-05-07 16:39 UTC] crrodriguez at opensuse dot org
philduby at phriendly dot net : what you mention seems to be  another 
bug/misfeature.
 [2010-07-20 05:43 UTC] hnzksq at gmail dot com
<?php

/**
 * @author zhouw
 * @copyright 2010
 */

class Foo
{
    public function bar()
    {
        return 'foo bar';
    }
}

$callback = array('Foo', 'bar');
if (is_callable($callback)) {
    echo call_user_func($callback);
}

?>

我测试可以用的。
 [2012-02-23 20:29 UTC] stas@php.net
-Type: Bug +Type: Feature/Change Request
 [2012-02-23 20:29 UTC] stas@php.net
Let's first make important note that it is not a bug - it is an intended 
functionality, is_callable is supposed to return true on this data and as far as 
I can see, has always done so at least since 5.2 (maybe earlier, just don't have 
binary to check). 

Secondly, Foo::Bar is not always a static call. Consider this code:

 class Foo
    {
        private $number = 42;
        public function bar()
        {
           var_dump($this->number);
           return __METHOD__;
        }
    }

class Bar extends Foo {
        public function callme($callback) {
                echo call_user_func($callback);
        }
}

$callback = array('Foo', 'Bar');
var_dump(is_callable($callback));
$bar = new Bar();
$bar->callme($callback);

This is a bit convoluted, but everything works just fine. Changing is_callable 
and the engine to prohibit this case would cause massive code breakage, and as a 
lot of code uses this pattern to call parent ctors, it's probably not feasible. 
There's a difference between "true static call" and "parent method call that 
looks like static call" and unfortunately, this difference exists only in 
runtime when the actual call is made, is_callable would not be able to predict 
it.
 [2016-01-16 02:23 UTC] danack@php.net
-Assigned To: +Assigned To: danack
 [2016-01-16 02:23 UTC] danack@php.net
Assigning to myself. I have an RFC relevant to this: https://wiki.php.net/rfc/consistent_callables
 [2021-02-18 11:03 UTC] nikic@php.net
-Status: Assigned +Status: Closed
 [2021-02-18 11:03 UTC] nikic@php.net
In PHP 8.0 calling non-static methods statically is (finally) prohibited. This has also fixed the is_callable() issue as a side effect.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Sat Nov 23 23:01:26 2024 UTC