Edit report at https://bugs.php.net/bug.php?id=64196&edit=1
ID: 64196 Patch added by: krak...@php.net Reported by: krak...@php.net Summary: __clone recursion stack overflow Status: Open Type: Bug Package: Reproducible crash Operating System: Any PHP Version: Irrelevant Block user comment: N Private report: N New Comment: The following patch has been added/updated: Patch Name: __clone-2.patch Revision: 1360710727 URL: https://bugs.php.net/patch-display.php?bug=64196&patch=__clone-2.patch&revision=1360710727 Previous Comments: ------------------------------------------------------------------------ [2013-02-12 21:39:24] ni...@php.net Not sure in what way the new patch resolves the clash. Doesn't it just move it from "$foo->__clone" towards "$foo->{'$__clone'}"? ------------------------------------------------------------------------ [2013-02-12 20:05:02] krak...@php.net __clone-2.patch addresses the clash ... and ensures proper functionality in all situations, not just basic examples. we don't seem to be able to agree on whether such checks should be made, but at least now there are no clashes and the patch is correct ... ------------------------------------------------------------------------ [2013-02-12 20:03:42] krak...@php.net The following patch has been added/updated: Patch Name: __clone-2.patch Revision: 1360699422 URL: https://bugs.php.net/patch-display.php?bug=64196&patch=__clone-2.patch&revision=1360699422 ------------------------------------------------------------------------ [2013-02-12 14:45:15] ni...@php.net zend_object.guards is for property guards. Wouldn't you be clashing with the guard for $__clone here? Also, I'm not convinced we need to add this check at all. Recursion is a valid means of programming and as long as there is some termination condition everything's okay. Arguably with "clone" recursion makes rather little sense, but as it stands now we are open to recursion everywhere and I don't think we should go down the patch of saying "recursion is okay here, but it's not okay here". I mean, "include" for example can also be used recursively, even though you might argue that that's nearly as useless. Should we be adding checks everywhere, where we think recursion makes too little sense? I don't think so. The only (calls) that currently use recursion guarding are __get/__set and friends. For those it makes sense because the recursion guarding gives access to the underlying property, so it has some actual function (rather than just forbidding a programming pattern). ------------------------------------------------------------------------ [2013-02-12 14:27:47] krak...@php.net Description: ------------ This patch avoids stack overflows where recursion is present in __clone. Test script: --------------- <?php class Cloneable { public function __clone(){ return clone $this; } } $c = new Cloneable(); $a = clone $c; ?> Expected result: ---------------- Stack overflow Actual result: -------------- E_ERROR ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=64196&edit=1