Issues

ZF-2234: Invalid use of optional parameters

Description

There are some methods (full list below) in Zend_Acl class, where optional parameters are used incorrectly - they are not placed on the end of the parameters list, so they are followed by mandatory parameters, which actually makes optional parameters implicitly mandatory. Looks like this is done intentionally to allow passing NULL's instead of actual classes, but has a huge side effect when using Reflection on Zend_Acl.

Here is an example of the wrong behaviour when reflecting the Zend_Acl::_roleDFSVisitAllPrivileges() method:

$method = new ReflectionMethod('Zend_Acl', '_roleDFSVisitAllPrivileges'); $params = $method->getParameters(); echo $params[1]->isOptional() ? 'true' : 'false'; // is $resource parameter optional?

The output is 'false', though 'true' is expected.

I'm not actually sure if this is a ReflectionMethod implementation bug, or a Zend_Acl bug, but it looks like a controversial design to me.

Here is the list of affected methods I discovered: _roleDFSVisitAllPrivileges _roleDFSOnePrivilege _roleDFSVisitOnePrivilege

Comments

Please categorize/fix as needed.

I don´t think the mentioned method signatures should be changed, because there is some kind of parameter convention in all Acl methods like:

"function ($role, $resource, ...)"

except in "_getRules()" and "_getRuleTypes". If any parameter orders should be changed, then those two.

So I´d vote to not fix this issue in favor of a consistent api, that would be more useful to the users.

Regards

These methods are protected and AFAIK Zend_Acl is not intended to be extended widely, so personally I don't see any issues with API inconsistency that can affect users. On the other side, having marginally-valid PHP code in enterprise-level framework such as ZF is IMO unacceptable.

Hmm, but why do you want to reflect these methods when you don't want to extend them/change behavior?

Just curious on your use case, because I spend some time extending it and want to see what others make of the ACL functionality :)

To be honest, my use case is bogus and I would never recommend anyone doing something I did, not even mentioning expecting ZF to be compatible with it :) I discovered this issue while experimenting with a highly bug-prone proprietary unit testing framework.

Nevertheless, the problem is not in the inability to properly reflect Zend_Acl, but in invalid (or marginally-valid) use of optional parameters in PHP, regardless of their context. Maybe I'm wrong and such use of optional parameters is perfectly valid - that is for ZF team to decide.

Fixed in trunk with SVN r9417.

Fixed for 1.5.2 with SVN r9419.