Req #69959 [Sus]: unserialize() needlessly requires boilerplate
| From: | krakjoe@php.net | 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