[TYPO3-core] RFC #10454: t3lib_div::makeInstanceClassName() allows to bypass checks for t3lib_Singleton

[Jeff Segars]

Ingo Renner wrote:
> Hi all,
> 
> This is a SVN patch request
> 
> Type: feature
> 
> Branches: trunk
> 
> BT reference: http://bugs.typo3.org/view.php?id=10454
> 
> Problem:
> When marking a class as singleton by implementing t3lib_Singleton one 
> can assure that at any time there's only one instance of that class when 
> consistently using t3lib_div::makeInstance(). However, one can 
> circumvent this and still create multiple instances of the class by 
> using t3lib_div::makeInstanceClassName() together with the new operator.
> 
> The same problem arises when implementing t3lib_Singleton and requiring 
> arguments to be passed to the constructor of that class. This class can 
> only be instanciated by using t3lib_div::makeInstanceClassName() and the 
> new operator, and thus again bypassing the check for the singleton.
> 
> This can have multiple implications with performance and also behavior.
> 
> 
> Solution:
> * make t3lib_div::makeInstance() aware of constructor arguments in the 
> first step.
> * make the functionality of t3lib_div::makeInstanceClassName() available 
> as a protected internal function. This functionality is still needed to 
> recursively resolve xclassed class names.
> * mark t3lib_div::makeInstanceClassName() as deprecated with a hint for 
> t3lib_div::makeInstance()
> * replace all occurances of t3lib_div::makeInstanceClassName() in the 
> core with t3lib_div::makeInstance() calls. (separate issue)
> * later, with TYPO3 4.5, remove t3lib_div::makeInstanceClassName() 
> eventually enforcing the check for singletons
> 
> 
> How to reproduce:
> The last hunk in the patch acts as an example of how the code is cleaned 
> up and still works. This hunk is not "part" of the actual patch and is 
> not intended for the commit with this issue.
> 
> 
> Notes:
> We can of course not prevent someone from directly using the new 
> operator with the class name, but this is out of scope for this issue as 
> it's considered unclean anyways as xclasses aren't taken into account 
> this way.
> 
> With this patch you can finally do this: 
> t3lib_div::makeInstance('myClass', $arg1, $arg2,  ..., $argN);
> 
> 
> 
> all the best
> Ingo
> 

Thanks for the patch Ingo!  makeInstanceClassName has always annoyed me :)

Here's my +1 on reading and testing, with one item that may be open to 
some discussion.

The call to newInstanceArgs() throws a fatal error if a developer passes 
arguments to a class without a constructor. On the one hand, this the 
developer's fault but at the the same time "new myclass(arg1, arg2)" 
doesn't throw a fatal error when there's no constructor.

> Fatal error: Uncaught exception 'ReflectionException' with message 'Class language does not have a constructor, so you cannot pass any constructor arguments'

Thanks,
Jeff