yiisoft / yii2

Yii 2: The Fast, Secure and Professional PHP Framework
http://www.yiiframework.com
BSD 3-Clause "New" or "Revised" License
14.24k stars 6.91k forks source link

Add a Php wrapper for is_callable #12711

Open dynasource opened 8 years ago

dynasource commented 8 years ago

The fact that calling callbacks by is_callable could be hazardous was new to me https://github.com/yiisoft/yii/issues/3492 http://www.yiiframework.com/news/78/yii-1-1-15-is-released-security-fix/

Because of the, it was chosen to change all classes with checking against instanceof Closure, going at cost of flexibility (see https://github.com/yiisoft/yii2/pull/12614).

This makes it impossible to override Yiis core classes with callback arrays or to pass these as configuration. This is a pity, because flexibity is reduced while there are alternatives for is_callable($mixed,true) which still enable the use of callback array.

Another thing about this is consistency. Frameworkwide there isnt any. Sometimes, there is only checked on null (see https://github.com/yiisoft/yii2/blob/master/framework/grid/GridView.php#L466), and sometimes on Closure (see https://github.com/yiisoft/yii2/blob/master/framework/grid/GridView.php#L466)

I dont like the inconsistency as you can see at https://github.com/yiisoft/yii2/pull/12614/commits ;) but I was also surprised that arrays were not supported because of https://github.com/yiisoft/yii/issues/3492.

To have best of all worlds including safety, I would suggest 1 way of dealing with this, preferably by using a method in newly to create 'PhpHelper'. To give an example:


namespace yii\helpers;

class PhpHelper
{

    /**
     * Returns a boolean indicating if a callable may be called
     * @see https://github.com/yiisoft/yii/issues/3492
     * @param Closure|array
     * @return bool
     */
    public static function mayCall($callable)
    {
        if($mixed === null){
            return false;
        }

        if ($mixed instanceof \Closure || (is_array($mixed) && is_callable($mixed))){
            return true;
        }

        return false;
    }
}
rob006 commented 8 years ago

I would rather see helper for converting callable to closures, like https://wiki.php.net/rfc/closurefromcallable

samdark commented 8 years ago

@rob006 that's separate issue: https://github.com/yiisoft/yii2/issues/8797#issuecomment-112083165

rob006 commented 8 years ago

I was thinking about something simpler: https://gist.github.com/rintaun/2773168#gistcomment-1876685