Bug #73816 [Opn->Csd]: Broken eval(anonymous class)

Edit report at https://bugs.php.net/bug.php?id=73816&edit=1 ID: 73816 Updated by: nikic@php.net Reported by: nicolas dot grekas+php at gmail dot com Summary: Broken eval(anonymous class) -Status: Open +Status: Closed Type: Bug Package: Scripting Engine problem PHP Version: 7.1.0 -Assigned To: +Assigned To: nikic Block user comment: N Private report: N New Comment: This has been fixed by https://github.com/php/php-src/commit/0f2cdbf214efd98b4bdaf5ca41728faf00e7c037 in 7.4. I've just added the test case in https://github.com/php/php-src/commit/6f63e053201a5284d5b3ed2d2c369bb72500c345. Previous Comments: ------------------------------------------------------------------------ [2018-10-23 09:21:24] cmb@php.net Related To: Bug #77050 ------------------------------------------------------------------------ [2016-12-27 00:48:22] dave at mudsite dot com It's not really feasible to hash the eval string, as in theory the class could be a small part of the eval string. I do note that HHVM does handle the multiple evals well. I'm trying to think how we could support it simply. It's hard to tell since the file and address of the class is the same, the difference only being the property change. I'd have to look closer at how a class is built up, if it inserts itself before or after creating all the properties. If the latter we could probably create some unique identifier with that knowledge to support it. ------------------------------------------------------------------------ [2016-12-26 21:51:21] nicolas dot grekas+php at gmail dot com 100% agree with this analysis. This still means eval+anonymous classes is kinda broken. I don't know if that would be a proper fix, but can't we use the hash of the evaluated string as part of the generated name? Or at least some nonce? ------------------------------------------------------------------------ [2016-12-26 20:48:22] dave at mudsite dot com I believe you're right in the sense the fix for the referenced bug prevents your example from working, but had you stored the result of the first call to anon() and tried to use it, you'd have hit the bug that was fixed; the reference of the class would have been the second eval'd anon class, rather than the first. I'd leave it to others to say if the previous, or current, behavior is appropriate. If we took your example and ran it with a slight change, storing the return of anon() to variables and var_dumping after you'd expose the bug that's resolved (kinda). You'd see that both c1, and c2 do reference a class-entry, but it'd be the second updated pointer to the entry. https://3v4l.org/RiRcW If you were to then set the prop1 before creating it again, you'd get even weirder output where prop1 wouldn't exist. https://3v4l.org/Eq3Di I'd believe the current (7.0.10, 7.1.0) results ought to be correct. Where an anon class is defined it should be the only entry in the class-table. Since the actual name of the class is something along the lines of: class@anonymous\0/Path/to/file.php(6) : eval()'d code0xdeadbeef Where the only printed value of this name is up to that nul byte, however, the hash into the class table you'd see references the memory location where the 'new class()' is defined. Which, if your eval() line is at the same location the memory location never changes, even though you change the properties in it. ------------------------------------------------------------------------ [2016-12-26 12:48:11] nicolas dot grekas+php at gmail dot com Maybe introduced by the fix for https://bugs.php.net/72594 ? ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at https://bugs.php.net/bug.php?id=73816 -- Edit this bug report at https://bugs.php.net/bug.php?id=73816&edit=1