Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:98838 Return-Path: Mailing-List: contact internals-help@lists.php.net; run by ezmlm Delivered-To: mailing list internals@lists.php.net Received: (qmail 57826 invoked from network); 21 Apr 2017 13:51:25 -0000 Received: from unknown (HELO lists.php.net) (127.0.0.1) by localhost with SMTP; 21 Apr 2017 13:51:25 -0000 Authentication-Results: pb1.pair.com header.from=nikita.ppv@gmail.com; sender-id=pass Authentication-Results: pb1.pair.com smtp.mail=nikita.ppv@gmail.com; spf=pass; sender-id=pass Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.223.175 as permitted sender) X-PHP-List-Original-Sender: nikita.ppv@gmail.com X-Host-Fingerprint: 209.85.223.175 mail-io0-f175.google.com Received: from [209.85.223.175] ([209.85.223.175:33119] helo=mail-io0-f175.google.com) by pb1.pair.com (ecelerity 2.1.1.9-wez r(12769M)) with ESMTP id 2F/BB-61625-C5E0AF85 for ; Fri, 21 Apr 2017 09:51:24 -0400 Received: by mail-io0-f175.google.com with SMTP id k87so120646660ioi.0 for ; Fri, 21 Apr 2017 06:51:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=GgoiMNggScWaeLnor1zalQkpkjrbJhJW7y3oCo14tEw=; b=QTl0DRnFhhNvD1Z5fQHXIJwS6Fsci93AoE4w+iEUPg4WwS8Sc1ebsRDZ8fxfXZdRt2 IHd/7q+wTntNCawCAv0Iw32SgDeJo5YlqZAOakheMNSVdPTyvIdFyuJmfgcHUkBcPG2B 28CAW+UTVef1V1zONOr+7TypdyQOIZS8x+8GsneUTDL5ESiz90vYlClCTCft60fcIU8+ BrLZVqkymeYjx+Iic7CaChXMj/4QloCGbsjGyW+d3jcqdHexrueCh3Dtea6rsx6VO7Ow zEfuDex9AR18Ld6mmbuwkeCkoBq/9YB6HlQVcTC3fqHhYINwlQs+CXpZB12qcYtFPYxa oNwA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=GgoiMNggScWaeLnor1zalQkpkjrbJhJW7y3oCo14tEw=; b=Hq1R3GWG4MUAgOiYDDbGC9VW9MLNTQNoS2PKlq1OwNR+ZlZ1fJO2M0yL//hTdI2Qug dbtimCrWloqNWjNUpC2VC6h2+Yh/Ij1U0TpeVlvCD7609uYysBx+R9g17YO7Gn2pPjqm rWP2IXueYfke+sGDrBw+ps09qSGBXX/hxTmiJsGTBdtP+m5i1hmfZGOjZr92htw71QY+ 8bIwXXBywCtyHF8CTqpBRSXC+GXS2LlS9mb5I7NAPuH+McKTAtYe/dpNCaYDwa/Hq1kS zmYMedgXbeZuu5wODk05FEAtNR5rN4ceC/0oytqXpklvI2CRc3FhPZiLRVOUWX9hFnYo 2Tow== X-Gm-Message-State: AN3rC/7bkjfExlG4qqQLf2bKWE2dTJcmU2A4S7ijwxbh7AUpFk7FoQ8L /UXldfe9qLAZEIg2npFTxAWo2yuQIg== X-Received: by 10.36.181.20 with SMTP id v20mr5100222ite.73.1492782608956; Fri, 21 Apr 2017 06:50:08 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.9.144 with HTTP; Fri, 21 Apr 2017 06:50:08 -0700 (PDT) In-Reply-To: References: Date: Fri, 21 Apr 2017 15:50:08 +0200 Message-ID: To: =?UTF-8?Q?Micha=C5=82_Brzuchalski?= Cc: PHP Internals List Content-Type: multipart/alternative; boundary=f403045d9c022245b1054dad8900 Subject: Re: [PHP-DEV] A replacement for the Serializable interface From: nikita.ppv@gmail.com (Nikita Popov) --f403045d9c022245b1054dad8900 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On Fri, Apr 21, 2017 at 2:47 PM, Micha=C5=82 Brzuchalski < michal.brzuchalski@gmail.com> wrote: > I know my voice is doesn't mean anything but IMHO interface with magic > methods could bring more inconsistency. > > I know PHP is consistently inconsistent but I would prefer if it is > posdible to fix an issue with present method naming. > > Cheers > Magic methods have a distinct backwards compatibility advantage. They allow you to add __serialize/__unserialize to an existing class that currently uses Serializable. Older PHP versions will then use the Serializable implementation, while never versions will use __serialize/__unserialize. An interface makes this a lot more complicated, because you either have to bump your PHP version requirement (unlikely), or you have to provide a shim interface for older PHP versions. This shim interface would then be part of any library currently implementing Serializable, which seems sub-optimal to me. That's why I think magic methods are better for this case (though I don't strongly care). Also, to answer an OTR question: We cannot simply reuse the Serializable interface by allowing an array return value from Serializable::unserialize(). The array return value is only a means to an end: the important part is that the new serialization mechanism does not share serialization state -- using arrays instead of strings is just a convenient way to achieve this. However, Serializable::unserialize() currently shares the state and we cannot change this without breaking BC -- so we cannot reuse this interface. Nikita > 21.04.2017 11:39 "Nikita Popov" napisa=C5=82(a): > >> Hi internals, >> >> As you are surely aware, serialization in PHP is a big mess. Said mess i= s >> caused by some fundamental issues in the serialization format, and >> exacerbated by the existence of the Serializable interface. Fixing the >> serialization format is likely not possible at this point, but we can >> replace Serializable with a better alternative and I'd like to start a >> discussion on that. >> >> The problem is essentially that Serializable::serialize() is expected to >> return a string, which is generally obtained by recursively calling >> serialize() in the Serializable::serialize() implementation. This >> serialize() call shares state information with the outer serialize(), to >> ensure that two references to the same object (or the same reference) wi= ll >> continue referring to a single object/reference after serialization. >> >> This causes two big issues: >> >> First, the implementation is highly order-dependent. If >> Serializable::serialize() contains multiple calls to serialize(), then >> calls to unserialize() have to be repeated **in the same order** in >> Serializable::unserialize(), otherwise unserialization may fail or be >> corrupted. In particular this means that using parent::serialize() and >> parent::unserialize() is unsafe. (See also >> https://bugs.php.net/bug.php?id=3D66052 and linked bugs.) >> >> Second, the existence of Serializable introduces security issues that we >> cannot fix. Allowing the execution of PHP code during unserialization is >> unsafe, and even innocuous looking code is easily exploited. We have >> recently mitigated __wakeup() based attacks by delaying __wakeup() calls >> until the end of the unserialization. We cannot do the same for >> Serializable::unserialize() calls, as their design strictly requires the >> unserialization context to still be active during the call. Similarly, >> Serializable prevents an up-front validation pass of the serialized >> string, >> as the format used for Serializable objects is user-defined. >> >> The delayed __wakeup() mitigation mentioned in the previous point also >> interacts badly with Serializable, because we have to delay __wakeup() >> calls to the end of the unserialization, which in particular also implie= s >> that Serializable::unserialize() sees objects prior to wakeup. (See also >> https://bugs.php.net/bug.php?id=3D74436.) >> >> In the end, everything comes down to the fact that Serializable requires >> nested serialization calls with context sharing. >> >> The alternative mechanism (__sleep + __wakeup) does not have these issue= s >> (anymore), but it is not sufficiently flexible for general use: Notably, >> __sleep() allows you to limit which properties are serialized, but the >> properties still have to actually exist on the object. >> >> I'd like to propose the addition of a new mechanism which essentially >> works >> the same way as Serializable, but uses arrays instead of strings and doe= s >> not share context. I'm not sure about the naming (RealSerializable, >> anyone?), so I'll just go with magic methods __serialize() and >> __unserialize() for now: >> >> public function __serialize() : array; >> public function __unserialize(array $data) : void; >> >> From a userland perspective the implementation should be the same as for >> Serializable methods, but with interior serialize()/unserialize() calls >> stripped out. Right now Serializable implementations already usually wor= k >> by doing something like "return serialize([ ... ])", this would change i= t >> to just "return [ ... ]" and move the serialize()/unserialize() call int= o >> the engine, where we can perform it safely and robustly. >> >> The new methods should reuse the "O" serialization format, rather than >> introducing a new one. This allows a measure of interoperability with >> previous PHP versions, which can still decode serialized strings from >> newer >> versions using __wakeup(). >> >> If an object has both __wakeup() and __unserialize(), then __unserialize= () >> should be called. If an object implements both Serializable::unserialize= () >> and __unserialize(), then we should invoke one or the other based on >> whether "C" or "O" serialization is used. >> >> Thoughts? >> >> Nikita >> > --f403045d9c022245b1054dad8900--