PHP 8 support? Named parameter $whatever overwrites previous argument

[neekfenwick]

The following GET handler works fine on PHP 7, but on PHP 8 produces a fatal Error:

/**
 * @uri /path/to/resource/{whatever}
 */
class MyResource extends Resource {
    public function get($whatever) {
        /* body */
    }
}

The Error is raised on the line in Resource.php trying to invoke the get function:

$response = Response::create(call_user_func_array(array($this, $methodName), $this->params));

The error looks like:

Fatal error: Uncaught Error: Named parameter $model overwrites previous argument in /home/me/project/lib/tonic/src/Tonic/Resource.php on line 153
Error: Named parameter $model overwrites previous argument in /home/me/project/lib/tonic/src/Tonic/Resource.php on line 153
Call Stack: 0.0003 385808
1. {main}() /home/me/project/restTonic.php:0 0.1378 2369704
2. Tonic\Resource->exec() /home/me/project/restTonic.php:19 0.1380 2370944
3. call_user_func_array:{/home/me/project/lib/tonic/src/Tonic/Resource.php:153}

This looks like normal PHP 8 behaviour. Would tonic need a rewrite to support PHP 8 or is there some simpler workaround we can employ? (or move to a completely different library)

[neekfenwick]

I've found what I think is a great description of the problem at https://www.drupal.org/project/rules/issues/3210303#comment-14070697 .. essentially Tonic is overloading the 'params' member of Request with both integer-indexed entries and named entries, which then causes call_user_func_array to raise an Error.

For example in my simple example above, calling /path/to/resource/foobar results in the params array containing:

[
    0 => 'foobar',
    'whatever' => 'foobar'
]

The whatever member is added in Application::route where it does this (edited for readability):

preg_match($uriRegex, $request->getUri(), $params)
// $params now has [ 0 => '/path/to/resource/foobar', 1 => 'foobar' ]
array_shift($params);
// $params now has [ 0 => 'foobar' ]
$uriParams = $resourceMetadata->getUriParams($index);
if ($uriParams) { // has params within URI
    foreach ($uriParams as $key => $name) {
        $params[$name] = $params[$key];
        // $params now has [ 0 => 'foobar', 'whatever' => 'foobar' ]
    }
}

I imagine the aim of adding the named parameter as well as the integer-indexed one is to support handlers that have multiple arguments that are provided out-of-order compared to their URI, for example:

@uri /path/to/resource/{firstarg}/{secondarg}/
public function get($secondarg, $firstarg) { }

With the named parameters firstarg and secondarg in $params, they would be assigned correctly before PHP 8. If I comment out the $params[$name] = $params[$key]; line, then my handler is called successfully, because $params only then contains an integer-indexed value which is assigned to the first argument to the function.

One solution to this would be to replace $params with only named parameters if the URI seems to be one with named parameters, instead of adding to it. i.e.

preg_match($uriRegex, $request->getUri(), $params)
// $params now has [ 0 => '/path/to/resource/foobar', 1 => 'foobar' ]
array_shift($params);
// $params now has [ 0 => 'foobar' ]
$uriParams = $resourceMetadata->getUriParams($index);
if ($uriParams) { // has params within URI
    $namedParams = []; // restart with an empty params array
    foreach ($uriParams as $key => $name) {
        $namedParams[$name] = $params[$key];
        // $namedParams now has [ 'whatever' => 'foobar' ]
    }
    $params = $namedParams; // replace $params
}

As I'm new to this problem and don't know much about this library's history, any feedback would be great to hear. I also believe this project is unmaintained, so I may be forking it to maintain our own PHP 8 compatible version, anyone else who's done this would be good to hear from :)

[PBillodeau]

@neekfenwick, I'm also new to this library, so be skeptical :) That said, my solution is available here: #210.

From my understanding named parameters were silently ignored by the call_user_func_array function before PHP8, so my solution was to simply filter out the named parameters when it's used by the call_user_func_array function. Everything else stays the same.

Did you run into any unintentional side-effects with your solution, or has it run smoothly?

[neekfenwick]

Did you run into any unintentional side-effects with your solution, or has it run smoothly?

My fix is working well on my PHP 8.1 development system. Since this project appears dead and I doubt any PR will be accepted, I've made a Release on my fork of the project at https://github.com/neekfenwick/tonic/releases - calling it alpha just while I test things but it's going well so far.

I believe my fix is more elegant than yours, as it builds the $params array in only one way, either with numerical indices or named parameters, and I believe yours is broken, I'll feedback on your PR.