Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:118816
MIME-Version: 1.0
References: <22177032-fe72-c39b-63fe-fa4368a70852@bastelstu.be>
 <CAOWwgpmuiRzUwGpU3ShQ7HOAVGhcP=Wb8z50acLfERMn_NEbwQ@mail.gmail.com> <c9e61189-7826-844f-56b9-7bb509a0791d@bastelstu.be>
In-Reply-To: <c9e61189-7826-844f-56b9-7bb509a0791d@bastelstu.be>
Date: Sat, 15 Oct 2022 11:06:30 +0200
Message-ID: <CAOWwgpnpBTx9-aLz-bg1xZ9FT5JyWsX44TLwfvanDS_aAW7qdQ@mail.gmail.com>
To: =?UTF-8?Q?Tim_D=C3=BCsterhus?= <tim@bastelstu.be>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="00000000000099f9f005eb0f0f65"
Subject: Re: [PHP-DEV] [VOTE] Improve unserialize() error handling
From: nicolas.grekas+php@gmail.com (Nicolas Grekas)

--00000000000099f9f005eb0f0f65
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

> > Not sure why I didn't think about it before but I just ran the test sui=
te
> > of Symfony after applying the patch proposed in the RFC to change the w=
ay
> > exceptions are handled by unserialize.
> >
> > This change breaks the test suite of 5 separate components. I created
> this
> > gist to list all the failures:
> > https://gist.github.com/nicolas-grekas/3da652a51669baa40c99bd20e4a1b4dd
> >
> > Maybe I wasn't convincing enough during the discussion period, but that
> > doesn't change the fact that the proposed behavior in the RFC is a very
> > clear BC break that will affect userland significantly.
> >
> > I'm therefore voting NO on the proposal.
> >
>
> I'm not surprised by the =E2=80=9Cno=E2=80=9D on the first vote based on =
the previous
> discussion. I am surprised however that you also disagree with raising
> the E_NOTICE to E_WARNING for consistency.
>
> I don't plan on repeating all the previous discussion points with you,
> but as you mention the Symfony tests specifically: Please find the patch
> attached. Do you believe the expectations in this new test would a
> reasonable expectation by a Symfony user?
>

Since the beginning my point is not that the RFC doesn't have merits. It's
that the proposed approach breaks BC in a way that will affect the
community significantly. We have policies that say we should avoid BC
breaks and they should apply here.

To anyone wondering about the reality of the BC break if you're not
convinced there is one because the original code is broken anyway (which is
your main argument Tim IIUC): please have a look at the failures I reported
above and wonder about the changes the RFC would impose. I cannot think
about one that would not imply some sort of "if the version of PHP is >=3D
8.3 then A, else B". This is the fact that highlights the BC break.

Nicolas

--00000000000099f9f005eb0f0f65--