Re: [PHP-DEV] RFC [Discussion]: Improve unserialize() error handling

2022-10-12 Thread Tim Düsterhus
Hi On 9/28/22 17:42, Tim Düsterhus wrote: If you want to go that route, I'd go all the way to an RCV vote and be done with it. Or else just make an executive decision as the RFC author and let the chips fall where they may. I'm generally not too happy with secondary votes. Sometimes you

Re: [PHP-DEV] RFC [Discussion]: Improve unserialize() error handling

2022-09-28 Thread Tim Düsterhus
Hi On 9/28/22 17:42, Tim Düsterhus wrote: Would the increase of E_NOTICE to E_WARNING for the two cases that currently emit E_NOTICE be something that even requires a vote or is this something that can simply be decided by merging a PR? [1] In that case the second and third vote could be

Re: [PHP-DEV] RFC [Discussion]: Improve unserialize() error handling

2022-09-28 Thread Tim Düsterhus
Hi On 9/28/22 19:53, Nicolas Grekas wrote: I'm not quoting the full text of the discussion because the php ML server refused the message - too long it said. Yeah, that's fine. I also aggressively prune irrelevant parts from whatever I quote; saves traffic and storage, and also makes it

Re: [PHP-DEV] RFC [Discussion]: Improve unserialize() error handling

2022-09-28 Thread Nicolas Grekas
Hi Tim, I'm not quoting the full text of the discussion because the php ML server refused the message - too long it said. > this gist: > > https://gist.github.com/nicolas-grekas/5dd3169f94ed3b4576152605330824fe > > > > > > [...] (see previous messages on the thread for this missing part) > > >>

Re: [PHP-DEV] RFC [Discussion]: Improve unserialize() error handling

2022-09-28 Thread Tim Düsterhus
Hi On 9/28/22 13:41, Christoph M. Becker wrote: Predicting people's second-place choice is risky business. This assumption seems logical on its face, but I'm sure there are people that will buck your expectations. The reasoning is that unless “E_WARNING in 8.x without future decision”

Re: [PHP-DEV] RFC [Discussion]: Improve unserialize() error handling

2022-09-28 Thread Christoph M. Becker
On 27.09.2022 at 22:11, Larry Garfield wrote: > On Tue, Sep 27, 2022, at 3:01 PM, Tim Düsterhus wrote: > >> Thank you, I thought about what to do here and I've adjusted the options >> in the "increase to what" vote to make this a 3-way vote: >> >>

Re: [PHP-DEV] RFC [Discussion]: Improve unserialize() error handling

2022-09-27 Thread Larry Garfield
On Tue, Sep 27, 2022, at 3:01 PM, Tim Düsterhus wrote: > Hi > > On 9/8/22 18:36, Larry Garfield wrote: Either I guess? Honestly we should decide that in advance on the list. :-) E_WARNING+Exception in 9 is what I'd probably favor, with "Exception now" as a second choice. >>

Re: [PHP-DEV] RFC [Discussion]: Improve unserialize() error handling

2022-09-27 Thread Tim Düsterhus
Hi On 9/8/22 18:36, Larry Garfield wrote: Either I guess? Honestly we should decide that in advance on the list. :-) E_WARNING+Exception in 9 is what I'd probably favor, with "Exception now" as a second choice. We've done this kind of two-step thing before; a lot of PHP 8 changes were

Re: [PHP-DEV] RFC [Discussion]: Improve unserialize() error handling

2022-09-21 Thread Tim Düsterhus
Hi On 9/20/22 21:24, Tim Düsterhus wrote: My understanding is that this attempts to gracefully handle broken cache entries and force a rebuild if the payload is invalid. This will fail if the serialized payload contains a malformed DateTimeImmutable, because DateTimeImmutable will throw an

Re: [PHP-DEV] RFC [Discussion]: Improve unserialize() error handling

2022-09-20 Thread Tim Düsterhus
Hi Nicolas On 9/20/22 19:40, Nicolas Grekas wrote: I'm a bit busy with conferences these days... Understood. Enjoy! Are those two examples based on real-world use cases or did you craft them specifically to point out how the proposal would introduce a behavioral change? They were

Re: [PHP-DEV] RFC [Discussion]: Improve unserialize() error handling

2022-09-20 Thread Nicolas Grekas
Hi Tim, I'm a bit busy with conferences these days... On 9/12/22 21:46, Nicolas Grekas wrote:>> unserialize() is a generic > function that will also call arbitrary > >> callbacks. You already have no guarantees whatsoever about the kind of > >> exception that is thrown from it. I am unable to

Re: [PHP-DEV] RFC [Discussion]: Improve unserialize() error handling

2022-09-19 Thread Tim Düsterhus
Hi Nicolas On 9/13/22 19:34, Tim Düsterhus wrote: [long response from a week ago] Did you find the time to read through my response from September, 13th, yet? Best regards Tim Düsterhus -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit:

Re: [PHP-DEV] RFC [Discussion]: Improve unserialize() error handling

2022-09-13 Thread Tim Düsterhus
Hi On 9/12/22 21:46, Nicolas Grekas wrote:>> unserialize() is a generic function that will also call arbitrary callbacks. You already have no guarantees whatsoever about the kind of exception that is thrown from it. I am unable to think of a situation where the input data is well-defined

Re: [PHP-DEV] RFC [Discussion]: Improve unserialize() error handling

2022-09-12 Thread Nicolas Grekas
> On 9/8/22 19:06, Nicolas Grekas wrote: > > From what I understand, there are two things in the RFC: > > > > 1. a proposal to wrap any throwables thrown during unserialization > > inside an UnserializationFailedException > > Correct. > > > 2. a discussion to figure out a way to turn

Re: [PHP-DEV] RFC [Discussion]: Improve unserialize() error handling

2022-09-12 Thread Dan Ackroyd
Hi Nicolas, On Thu, 8 Sept 2022 at 18:06, Nicolas Grekas wrote: > > There needs to be serious justification for it. I think this is covered by the RFC. The current behaviour where people have to setup a custom error handler, and then restore the previous error handler, is pretty 'ungood'. The

Re: [PHP-DEV] RFC [Discussion]: Improve unserialize() error handling

2022-09-12 Thread Dan Ackroyd
Hi Paul, On Sat, 10 Sept 2022 at 12:04, Paul Dragoonis wrote: > > Welcome, Tim :-) > > Do give it more thought regarding the callbacks what Nikolas > said, sleep on it. Although welcoming people is nice, you might want to check that they haven't already had two RFCs passed, and become the

Re: [PHP-DEV] RFC [Discussion]: Improve unserialize() error handling

2022-09-10 Thread Tim Düsterhus
Hi On 9/10/22 13:03, Paul Dragoonis wrote: Do give it more thought regarding the callbacks what Nikolas said, sleep on it. Are you referring to 'unserialize_callback_func' or to the unserialize handlers (i.e. __wakeup, __unserialize, Serializable::unserialize)? If the former, I am

Re: [PHP-DEV] RFC [Discussion]: Improve unserialize() error handling

2022-09-10 Thread Paul Dragoonis
On Thu, 8 Sep 2022, 17:18 Tim Düsterhus, wrote: > Hi > > On 9/7/22 23:44, Larry Garfield wrote: > > Either I guess? Honestly we should decide that in advance on the list. > :-) E_WARNING+Exception in 9 is what I'd probably favor, with "Exception > now" as a second choice. > > > > I'm a new-ish

Re: [PHP-DEV] RFC [Discussion]: Improve unserialize() error handling

2022-09-08 Thread Tim Düsterhus
Hi On 9/8/22 19:06, Nicolas Grekas wrote: From what I understand, there are two things in the RFC: 1. a proposal to wrap any throwables thrown during unserialization inside an UnserializationFailedException Correct. 2. a discussion to figure out a way to turn these

Re: [PHP-DEV] RFC [Discussion]: Improve unserialize() error handling

2022-09-08 Thread Nicolas Grekas
Hi Tim, Thanks for the RFC. I've now written up an RFC as a follow-up for the "What type of > Exception to use for unserialize() failure?" thread [1]: > > > > RFC: Improve unserialize() error handling > https://wiki.php.net/rfc/improve_unserialize_error_handling > > Proof of concept

Re: [PHP-DEV] RFC [Discussion]: Improve unserialize() error handling

2022-09-08 Thread Larry Garfield
On Thu, Sep 8, 2022, at 11:18 AM, Tim Düsterhus wrote: > Hi > > On 9/7/22 23:44, Larry Garfield wrote: >> Either I guess? Honestly we should decide that in advance on the list. :-) >> E_WARNING+Exception in 9 is what I'd probably favor, with "Exception now" as >> a second choice. >> > > I'm a

Re: [PHP-DEV] RFC [Discussion]: Improve unserialize() error handling

2022-09-08 Thread Tim Düsterhus
Hi On 9/7/22 23:44, Larry Garfield wrote: Either I guess? Honestly we should decide that in advance on the list. :-) E_WARNING+Exception in 9 is what I'd probably favor, with "Exception now" as a second choice. I'm a new-ish contributor here in internals, so I don't know how things were

Re: [PHP-DEV] RFC [Discussion]: Improve unserialize() error handling

2022-09-07 Thread Larry Garfield
On Wed, Sep 7, 2022, at 10:37 AM, Tim Düsterhus wrote: > Hi > > On 9/5/22 23:12, Larry Garfield wrote: >>> RFC: Improve unserialize() error handling >>> https://wiki.php.net/rfc/improve_unserialize_error_handling >>> >> Well-explained and well-argued. The only thing I'd add is that we should >>

Re: [PHP-DEV] RFC [Discussion]: Improve unserialize() error handling

2022-09-07 Thread Tim Düsterhus
Hi On 9/5/22 23:12, Larry Garfield wrote: RFC: Improve unserialize() error handling https://wiki.php.net/rfc/improve_unserialize_error_handling Well-explained and well-argued. The only thing I'd add is that we should consider bumping the E_NOTICE to an E_WARNING, *and* slating it to

Re: [PHP-DEV] RFC [Discussion]: Improve unserialize() error handling

2022-09-07 Thread Tim Düsterhus
Hi On 9/7/22 14:41, Côme Chilliet wrote: Le lundi 5 septembre 2022, 19:20:00 CEST Tim Düsterhus a écrit : RFC: Improve unserialize() error handling https://wiki.php.net/rfc/improve_unserialize_error_handling Is the new UnserializationFailedException class extending any other Exception class

Re: [PHP-DEV] RFC [Discussion]: Improve unserialize() error handling

2022-09-07 Thread Côme Chilliet
Le lundi 5 septembre 2022, 19:20:00 CEST Tim Düsterhus a écrit : > RFC: Improve unserialize() error handling > https://wiki.php.net/rfc/improve_unserialize_error_handling Is the new UnserializationFailedException class extending any other Exception class ? This is not explained in the RFC. Côme

Re: [PHP-DEV] RFC [Discussion]: Improve unserialize() error handling

2022-09-06 Thread G. P. B.
On Mon, 5 Sept 2022 at 18:20, Tim Düsterhus wrote: > Hi > > I've now written up an RFC as a follow-up for the "What type of > Exception to use for unserialize() failure?" thread [1]: > > > > RFC: Improve unserialize() error handling >

Re: [PHP-DEV] RFC [Discussion]: Improve unserialize() error handling

2022-09-05 Thread Larry Garfield
On Mon, Sep 5, 2022, at 12:20 PM, Tim Düsterhus wrote: > Hi > > I've now written up an RFC as a follow-up for the "What type of > Exception to use for unserialize() failure?" thread [1]: > > > > RFC: Improve unserialize() error handling >

[PHP-DEV] RFC [Discussion]: Improve unserialize() error handling

2022-09-05 Thread Tim Düsterhus
Hi I've now written up an RFC as a follow-up for the "What type of Exception to use for unserialize() failure?" thread [1]: RFC: Improve unserialize() error handling https://wiki.php.net/rfc/improve_unserialize_error_handling Proof of concept implementation is in: