ZF-2234: Invalid use of optional parameters

Issue Type: Bug Created: 2007-11-26T06:49:41.000+0000 Last Updated: 2008-05-08T09:31:35.000+0000 Status: Resolved Fix version(s): - 1.5.2 (15/May/08)

Reporter: George Miroshnikov (laggyluke) Assignee: Darby Felton (darby) Tags: - Zend_Acl

Related issues: Attachments:


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


Posted by Wil Sinclair (wil) on 2008-03-25T20:40:00.000+0000

Please categorize/fix as needed.

Posted by Marc Jakubowski (octavian) on 2008-03-27T05:24:06.000+0000

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.


Posted by George Miroshnikov (laggyluke) on 2008-03-27T05:38:24.000+0000

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.

Posted by Marc Jakubowski (octavian) on 2008-04-01T06:10:16.000+0000

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 :)

Posted by George Miroshnikov (laggyluke) on 2008-04-01T07:25:39.000+0000

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.

Posted by Darby Felton (darby) on 2008-05-08T09:29:24.000+0000

Fixed in trunk with SVN r9417.

Posted by Darby Felton (darby) on 2008-05-08T09:31:35.000+0000

Fixed for 1.5.2 with SVN r9419.

Have you found an issue?

See the Overview section for more details.


© 2006-2018 by Zend, a Rogue Wave Company. Made with by awesome contributors.

This website is built using zend-expressive and it runs on PHP 7.