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
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If you forgot your password, you can retrieve your password here.
Password:
Status:
Package:
Bug Type:
Summary:
From: weierophinney@php.net
New email:
PHP Version: OS:

 

 [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: Sun Nov 24 01:01:29 2024 UTC