[RFC] More Appropriate Date/Time Exceptions View as Markdown

If your data fails to
unserialize, the only safe option is to throw it away.

Even given that, you probably want to investigate how it got
corrupted so that you can stop it from happening again. And doing
that would rely on being able to see the data that you attempted
to unserialize.

That's fair, but does not require dedicated subclasses.
Investigating corrupted serialization data is not something that can
be done programmatically, instead an actual human has to look at the
error message and possibly stack trace within the error log / within
Sentry / whatever floats your boat.

I'm not against improving the error messages to more clearly
indicate what part of the serialized data is broken, I'm against
ext-specific or library-specific exception classes for
unserialization failures, because that will lead to assumptions
being made that will not hold in the general case.

I feel like the RFC could use a bit more clarity around what
"unserialize" means. Are we talking about the literal "unserialize()"
function, or are we also talking about unserializing a string into a
DateTimeInterface object? I took it to mean the latter, and if the
latter, then I think putting the original data in the error object is
quite important. It's important for generating user-submitted errors,
reading data from a database/cache, or other messages.

The "Invalid serialiszation data for * object" errors (they are already
Error) are used in __set_state and __wakeup — for for both PHP's
unserialize (the __wakeup) and when calling __set_state with an array
with values.

Currently these do not provide any other context, and although that can
be expanded, it is not what this RFC set out to do. I would say that
improving error/exception/warning messages is out of scope.

I am not saying that we shouldn't do it, but not as part of this RFC.
I'll reflect it to say so, as well as clarify the serialization
messages.

cheers,
Derick

--
https://derickrethans.nl | https://xdebug.org | https://dram.io

Author of Xdebug. Like it? Consider supporting me: https://xdebug.org/support
Host of PHP Internals News: https://phpinternals.news

mastodon: @derickr@phpc.social @xdebug@phpc.social
twitter: @derickr and @xdebug

However, I generally feel that the program will always have access to
the raw data, after all, it is unserializing it.

Not always in a useful way.

function getMeeting(DB $db, int $meeting_id): Meeting
{
$meeting_data = $db->getUserSettings($meeting_id);

return unserialize($meeting_data);

}

If the $meeting_data included invalid DateTime info, the info wouldn't
be in the stacktrace. Investigating that issue would require someone
manually extracting some data from the production DB, which
programmers shouldn't have direct access to.

Tim Düsterhus wrote:

instead an actual human has to look at the error
message and possibly stack trace within the error log / within Sentry /
whatever floats your boat.

When the info isn't completely contained in the exception message
itself, what tickles my fancy is having small functions that format
specific exception types into an easy to understand to comprehend
piece of info, so that no-one has to spend time trying to read a
stack-trace.

I'm against ext-specific or library-specific exception classes
for unserialization failures,

Good point.

I'll raise it again when there is another discussion about an
UnserializeException.

Although it would probably be safe to expose invalid data that was
meant to be turned into a DateTime object, it's not obvious that it
would be safe to increase the exposure of arbitrary data, as it might
include PII data.

cheers
Dan
Ack