Hi Tim
> Hello internals, > > > > Following the discussion that started at > > https://externals.io/message/128226#128456 I wrote this RFC to > > formalize > > our consensus on the topic. > > > > TL;DR, this is about converting the deprecation of __sleep and __wakeup > > to > > a documentation-based soft deprecation: > > https://wiki.php.net/rfc/soft-deprecate-sleep-wakeup > > Thank you for the RFC. I have some comments: > > 1. > > I disagree with the phrasing that the RFC passed with a “narrow margin”. > While it is technically true, that this is the narrowest margin for > accepting an RFC, the necessary margins are already biased in favor of > not accepting an RFC. That the RFC was accepted means that a significant > majority of voters were in favor of the deprecation. I did not vote, > since I did not have sufficient time to form an opinion on the RFC, but > given the knowledge I've gained as part of the discussion I would now > vote in favor of the RFC. > Jakub addressed that in a sub-thread. 2. > > The examples are biased. As an example, the initial “User” example has a > serialization hook that is completely useless. The other examples try to > replicate `__sleep()`'s broken behavior exactly, which seems to be a > relevant requirement in the real world for only a minority of users. > The quantitative argument is not the only measure of the impact. The RFC explains why - qualitatively - the RFC will have a significant impact. For the quantitative part, we can talk forever before agreeing. > 3. > > Similarly, I believe that the RFC *overstates* the cost of the > deprecation. From my experience a majority of serialization hooks will > just unconditionally throw an exception to prevent serialization. The > truth is probably somewhere in the middle. > I'm not sure why you talk about throwing hooks. Those are not concerned so that's orthogonal to the points made. >From the small but not negligible share of the community I'm working with, having both Doctrine and Symfony impacted means the impact will be significant. Add that to the fact the "fix" is risky (what the RFC explains), and this will be a costly RFC for sure. Note that as Jakub figured out in the latest example he added, not only library-level codebases will need complex fixes: e.g. app-side user objects that use __sleep/wakeup and that are put into a session storage will need something as complex to address the deprecation without disrupting their connected users. > 4. > > The RFC correctly acknowleges that `__sleep()` is broken with regard to > private properties, but at the same time claims that the deprecation > does not fix a “correctness problem”, which is a contradiction. > I clarified this aspect in the RFC this way ("No technical urgency" section): > Some consider that __sleep() is broken because it is incompatible with nested private properties. Yet, this is more of a limitation rather than something being broken: many use cases don't need access to private properties and are just fine with the method. When one needs to overcome this limitation, one can migrate to __serialize, on an opt-in basis. > 5. > > The serialization mechanism is also a security sensitive part of the > language, the fewer moving parts there are, the better. Security is part > of the motivation for me. > Jakub addressed that in a sub-thread also. > ---------------------- > > That all said, as I've said before: I see that replacing __wakeup() by > __unserialize() is non-trivial and I would be okay with deferring that > one until we have some helper (e.g. `**s**et_mangled_object_vars`). But > __sleep() can just go away. > We've added a note in the "Future scope" section, phrased like this: > Consider dealing with __wakeup / __sleep separately as the impact might be specific to each one Cheers, Nicolas
