Req #69959 [Sus]: unserialize() needlessly requires boilerplate

Edit report at https://bugs.php.net/bug.php?id=69959&edit=1 ID: 69959 Updated by: krakjoe@php.net Reported by: lucas at threeamdesign dot com dot au Summary: unserialize() needlessly requires boilerplate Status: Suspended Type: Feature/Change Request Package: *General Issues PHP Version: 7.0.0alpha2 Block user comment: N Private report: N New Comment: I read the proposal. Changing the signature of serialize/unserialize, and or otherwise introducing new behaviour for a core function, even if it is optional behaviour, is exactly the kind of thing that requires proper discussion, by everybody. My suspending the bug was not to end the conversation, rather to encourage you to push for the change using the correct process. Writing up an RFC is easy, defining new behaviour is easy, but to make these kinds of changes, to argue the case for them, requires a hard working champion ... it could be you :) Since we are allowed to throw exceptions from the engine, I personally think that the best solution is to throw exceptions in exceptional circumstances. I also thought, and think, you wouldn't be suggesting that we do something really strange with parameter and return values, if you were only aware that you can actually break BC in the pursuit of more elegant behaviour (we can't do it here on this bug tracker). But my personal opinion doesn't actually matter ... What matters is process, and the opinion of everybody else. Push forward with your idea to improve behaviour, RFC the best solution you can think of - I think it's exceptions - I'll even write the patch for you if you are not able to do it yourself. Just showing you the way is all ... Previous Comments: ------------------------------------------------------------------------ [2016-04-08 23:07:22] lucas at threeamdesign dot com dot au I'm sure you're time poor, but it seems you haven't really read my proposal. I offered a perfectly sound and *completely backwards-compatible* solution. Adding an *optional* second parameter, that is a reference to whether the deserialization was successful, means the function's return value can stay the same, thus no updates are required to userland code. This mirrors the behaviour of exec where the shell return value is not the return value of the function. ------------------------------------------------------------------------ [2016-03-27 06:58:38] krakjoe@php.net Because of the widespread implications of changing the behaviour of unserialize, this discussion must go through the RFC process in order to make any changes. Please see: https://wiki.php.net/rfc/howto ------------------------------------------------------------------------ [2015-10-06 04:27:05] lucas at threeamdesign dot com dot au We currently have this function funserialize($serialized, &$into) { static $sfalse; if (is_string($serialized)) { if ($sfalse === null) { $sfalse = serialize(false); } $into = @unserialize($serialized); return $into !== false || rtrim($serialized) === $sfalse; } $into = false; return false; } but this is convoluted and isn't guaranteed to be future-proof. ------------------------------------------------------------------------ [2015-06-30 01:07:44] lucas at threeamdesign dot com dot au My point is $data = unserialize('something invalid', $unserialized); if (!$unserialized) { //handle error } or if (!unserialize('something invalid', $data)) { //handle error } would be far more reliable, and obviate the need for error handling/suppression. ------------------------------------------------------------------------ [2015-06-29 18:29:11] kalle@php.net We could potentially make it throw an Exception with the whole Engine Exceptions transition going on. But it would break BC as you would have to write code like: try { $s = unserialize('something invalid'); } catch(Exception $e) { echo $e->getMessage(); } vs. if(!($s = @unserialize('something invalid'))) { echo 'Error: Unable to un-serialize'; } ------------------------------------------------------------------------ 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=69959 -- Edit this bug report at https://bugs.php.net/bug.php?id=69959&edit=1