Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:118466
Message-ID: <86a071f4-41ee-33e0-92cd-2a76d5b480d8@bastelstu.be>
Date: Thu, 25 Aug 2022 17:06:48 +0200
MIME-Version: 1.0
Content-Language: en-US
To: "G. P. B." <george.banyard@gmail.com>
Cc: PHP internals <internals@lists.php.net>
References: <302000df-5c3f-a86c-a608-2a45d2726ab1@bastelstu.be>
 <CAFPFaMLT0VL+AVnGeRMK2wO+-A_Mf3LCqMKnzqs1iK828GbM9w@mail.gmail.com>
In-Reply-To: <CAFPFaMLT0VL+AVnGeRMK2wO+-A_Mf3LCqMKnzqs1iK828GbM9w@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Subject: Re: [PHP-DEV] What type of Exception to use for unserialize()
 failure?
From: tim@bastelstu.be (=?UTF-8?Q?Tim_D=c3=bcsterhus?=)

Hi

On 8/1/22 14:58, G. P. B. wrote:
>> - Add a new UnserializationFailureException extends Exception (or
>> whatever color the bikeshed should have). Any existing catch blocks
>> should still match, as Exception is more general. It would break for
>> ext/spl, though. Unless UnserializationFailureException extends
>> UnexpectedValueException.
>>
> 
> As we would be breaking BC with ext/date anyway I think having it just
> extend Exception would be fine
> 
> 
>> - Add a helper function that throws this Exception with a consistent
>> wording.
>> - Upgrade unserialize() from Notice to UnserializationFailureException.
>>
> 
> Notice to Exception might be a big jump, E_WARNING definitely an then
> promote to an Exception in 9.0 is probably more in line with what we did
> for 7.x/8.0
> 

I've created a PoC implementation here: 
https://github.com/php/php-src/pull/9425.

- Instead of requiring everyone to adjust their `__unserialize()` 
implementation, the PoC instead wraps any exception that is thrown 
during unserialization into the UnserializationFailureException. That 
way, classes can emit their own meaningful error messages, while 
developers can just catch `UnserializationFailedException` to handle 
everything.

- I've noticed that 'unserialize()' already emits E_WARNING for some 
types of error (e.g. out-of-bounds integers), so users already need to 
be prepared for E_WARNING to be emitted. I've adjusted the 
aforementioned Notice to Warning, but we might be able to directly jump 
to UnserializationFailedException from the existing warnings?

Of course I also plan to create a proper RFC for this, once PHP 8.2 RC 
dust has settled a bit.

Best regards
Tim Düsterhus