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

From: Date: Fri, 08 Apr 2016 23:07:26 +0000
Subject: Req #69959 [Com]: unserialize() needlessly requires boilerplate
References: 1  Groups: php.bugs 
Request: Send a blank email to php-bugs+get-200447@lists.php.net to get a copy of this message
Edit report at https://bugs.php.net/bug.php?id=69959&edit=1 ID: 69959 Comment by: lucas at threeamdesign dot com dot au 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'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. Previous Comments: ------------------------------------------------------------------------ [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'; } ------------------------------------------------------------------------ [2015-06-29 07:08:24] lucas at threeamdesign dot com dot au Description: ------------ I'm aware that the docs have this to say: --- Warning FALSE is returned both in the case of an error and if unserializing the serialized FALSE value. It is possible to catch this special case by comparing str with serialize(false) or by catching the issued E_NOTICE. --- Using the return value for failure and data is just lazy. This problem could be done away with, very easily if unserialize() were changed to accept a second parameter. This parameter would be a reference variable. If the function was called with the second parameter, the reference would be filled with either the unserialized data, or the success/failure boolean of the process. The return value of the function would then be the other. The latter form would be most backwards-compatible though seems more unintuitive. ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=69959&edit=1

« previous php.bugs (#200447) next »