toc() with invalid/missing "li"-function

Discussions and requests related to new CMSimple features, plugins, templates etc. and how to develop.
Please don't ask for support at this forums!
Post Reply
manu
Posts: 1086
Joined: Wed Jun 04, 2008 12:05 pm
Location: St. Gallen - Schweiz
Contact:

toc() with invalid/missing "li"-function

Post by manu » Mon Feb 23, 2015 3:27 pm

If toc() is called with an invalid/missing "li"-function, toc always returns false i.e. echo toc() shows nothing in template.
How about to harden the error handling slightly in tplfuncs.php line ~209?

Code: Select all

    if (count($ta) > 0) {
        $return = call_user_func($li, $ta, $start);
        if ($return === false) $return = XH_message('fail',"\"$li\"-function missing!");
        return $return;
    } 

cmb
Posts: 14225
Joined: Tue Jun 21, 2011 11:04 am
Location: Bingen, RLP, DE
Contact:

Re: toc() with invalid/missing "li"-function

Post by cmb » Mon Feb 23, 2015 4:24 pm

manu wrote:If toc() is called with an invalid/missing "li"-function, toc always returns false i.e. echo toc() shows nothing in template.
Actually, I consider this a bug. I'm surprised that call_user_func('doesNotExist') only gives a warning; apparently, the manual page doesn't explicitly mention this, even though doesNotExist() would error, as well as $func='doesNotExist';$func(). There's only:
Returns the return value of the callback, or FALSE on error.
Interestingly, call_user_func('trim') returns NULL.

I'm not sure about the fix, however. We might check for is_callable($li) (and report the error manually), or we might simply do $li($start, $ta), what would give a fatal error -- similar to writing tock(...) in the first place.

PS: I have moved this discussion to a separate topic, and put it on the XH 1.6.6 roadmap.
Christoph M. Becker – Plugins for CMSimple_XH

manu
Posts: 1086
Joined: Wed Jun 04, 2008 12:05 pm
Location: St. Gallen - Schweiz
Contact:

Re: toc() with invalid/missing "li"-function

Post by manu » Wed Feb 25, 2015 10:19 am

Ok, this might do:

Code: Select all

    if (count($ta) > 0) {
        if (is_callable($li)) {
            return $li($ta, $start);
        } else {
            return XH_message('fail',"\"$li\"-function failed!");
        }
    } 
Its a help when you play around with xtoc(), toxic_XH and whatsoever. After removing these you often forget the call in the template.

manu
Posts: 1086
Joined: Wed Jun 04, 2008 12:05 pm
Location: St. Gallen - Schweiz
Contact:

Re: toc() with invalid/missing "li"-function

Post by manu » Thu Feb 26, 2015 9:02 am

But what if the function is callable but has another range of parameters, returns anything other than a string as expected?
Per definition li() has to be a function and has to return a string.

Code: Select all

    if (count($ta) > 0) {
        $return = function_exists($li) ? $li($ta, $start) : false ;
        if (!is_string($return)) $return = XH_message('fail',"\"$li\"-function failed!");
        return $return;
    } 
(still puzzled about either the variable function nor call_user_func() dont throw a warning/info when the amount of parameters is wrong.)

cmb
Posts: 14225
Joined: Tue Jun 21, 2011 11:04 am
Location: Bingen, RLP, DE
Contact:

Re: toc() with invalid/missing "li"-function

Post by cmb » Thu Feb 26, 2015 11:05 am

manu wrote:But what if the function is callable but has another range of parameters, returns anything other than a string as expected?
Good point.
manu wrote:still puzzled about either the variable function nor call_user_func() dont throw a warning/info when the amount of parameters is wrong.
PHP is quite liberal when it comes to superfluos function arguments, because these could be used by the callee via func_get_args(). However, missing arguments are supposed to trigger a warning (either with call_user_func() or variable function call). The types of the arguments are usually handled "gracefully" due to PHP's type juggling.
Christoph M. Becker – Plugins for CMSimple_XH

cmb
Posts: 14225
Joined: Tue Jun 21, 2011 11:04 am
Location: Bingen, RLP, DE
Contact:

Re: toc() with invalid/missing "li"-function

Post by cmb » Wed May 27, 2015 7:32 pm

Hm, on what exactly we are going to vote?
manu wrote:

Code: Select all

    if (count($ta) > 0) {
        if (is_callable($li)) {
            return $li($ta, $start);
        } else {
            return XH_message('fail',"\"$li\"-function failed!");
        }
    } 
The <del>following</del><ins>above</ins> is fine for me (we should internationalize the error message, though).

I, personally, wouldn't like to check for the return type of the $li function. That's not typical for PHP, which does type juggling, and even scalar return type declarations coming in PHP 7 won't work so strict (unless strict_types=1 is declared, which I tend to dislike; I would have to check it more thoroughly, though).
Christoph M. Becker – Plugins for CMSimple_XH

cmb
Posts: 14225
Joined: Tue Jun 21, 2011 11:04 am
Location: Bingen, RLP, DE
Contact:

Re: toc() with invalid/missing "li"-function

Post by cmb » Wed Jun 24, 2015 12:42 am

In hindsight, it might have been better to let toc() emit a warning in case a not callable value has been passed as $li parameter. Something like:

Code: Select all

    if (count($ta) > 0) {
        if (!is_callable($li)) {
            trigger_error("\"$li\"-function failed!", E_USER_NOTICE);
        }
        return $li($ta, $start);
    }
Christoph M. Becker – Plugins for CMSimple_XH

manu
Posts: 1086
Joined: Wed Jun 04, 2008 12:05 pm
Location: St. Gallen - Schweiz
Contact:

Re: toc() with invalid/missing "li"-function

Post by manu » Thu Jun 25, 2015 8:43 am

cmb wrote:I'm not sure about the fix, however. We might check for is_callable($li) (and report the error manually), or we might simply do $li($start, $ta), what would give a fatal error -- similar to writing tock(...) in the first place.
After reconsideration of all arguments how to make this error visible I end up to just to call $li() directly. If the call is unsuccessfull it just ends up in a fatal error which can be investigated with debug mode on. IMHO this is the simpliest answer on the initial question.
Done (rev. 1656).

cmb
Posts: 14225
Joined: Tue Jun 21, 2011 11:04 am
Location: Bingen, RLP, DE
Contact:

Re: toc() with invalid/missing "li"-function

Post by cmb » Thu Jun 25, 2015 10:13 am

manu wrote:After reconsideration of all arguments how to make this error visible I end up to just to call $li() directly.
Fine. Thanks for committing.
Christoph M. Becker – Plugins for CMSimple_XH

Post Reply