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

From: Date: Sat, 09 Apr 2016 10:57:53 +0000
Subject: Req #69959 [Sus]: unserialize() needlessly requires boilerplate
References: 1  Groups: php.bugs 
Request: Send a blank email to php-bugs+get-200453@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 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: Another quick note ... Backward compatibility isn't the only kind of compatibility we have to consider. Say we implemented your idea to have a truly optional parameter, this can break BC for anyone reflecting on the function, but let's ignore that completely. A forward compatibility problem is looming: What if we want to change serialize/unserialize to throw exceptions in the future, do we still have to populate these pointless parameters with state ? Do we remove the pointless parameters and break more code ? It is rarely as simple as "let's just add this optional parameter" ... I want you to know that I thought about it before changing the status of the bug, and it really wasn't to end the conversation, at all. I appreciate you don't have to be here, like you appreciate I don't have to be here. Previous Comments: ------------------------------------------------------------------------ [2016-04-09 10:41:51] krakjoe@php.net 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 ... ------------------------------------------------------------------------ [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. ------------------------------------------------------------------------ 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

« previous php.bugs (#200453) next »