Re: Re: com php-src: Add optional second arg to unserialize(): ext/standard/basic_functions.c
ext/standard/tests/serialize/serialization_error_001.phpt ext/standard/tests/serialize/unserialize_consumed.phpt ext/standard/var.c

On Tue, Jun 10, 2014 at 4:51 PM, Julien Pauli <jpauli@php.net> wrote: > On Mon, Jun 9, 2014 at 7:35 PM, Ferenc Kovacs <tyra3l@gmail.com> wrote: > > On Mon, Jun 9, 2014 at 7:32 PM, Ferenc Kovacs <tyra3l@gmail.com> wrote: > > > >> > >> > >> > >> On Sun, Apr 13, 2014 at 2:52 AM, Ferenc Kovacs <tyra3l@gmail.com> > wrote: > >> > >>> > >>> > >>> > >>> On Fri, Jul 5, 2013 at 5:43 AM, Stas Malyshev <smalyshev@sugarcrm.com> > >>> wrote: > >>> > >>>> Hi! > >>>> > >>>> > Please add a note to UPGRADING as well. > >>>> > > >>>> > Thanks! > >>>> > >>>> I understand UPGRADING still not updated? Could you please update it? > >>>> > >>>> > >>> hi, > >>> > >>> When fixing https://bugs.php.net/bug.php?id=66568 > >>> today I found out > that > >>> this is still not mentioned in NEWS or UPDATING. > >>> Maybe our mails not getting through to Sara? Let's see if using her > other > >>> address help. > >>> > >>> -- > >>> Ferenc Kovács > >>> @Tyr43l - http://tyrael.hu > >>> > >> > >> managed to reach Sara through twitter( > >> https://twitter.com/Tyr43l/status/456777526908837888), > >> but still not > done. > >> almost commited the missing info to UPGRADING, but then I got some > second > >> thoughts. > >> Assuming that only SplDoublyLinkedList uses this streamed serialize > format > >> and given how that class has it's own serialize/unserialize methods, I > >> think that there is no reason to legalize the usage such strings. > >> I think that unserialize should raise a warning when there are still > >> unconsumed data in the string after finding the end of the serialized > >> format, so that the user is aware that he is feeding corrupt data to the > >> unserialize and if there are cases when we internally construct such > >> strings we should review and either eliminate those, or introduce this > >> format as a first-class citizen with some kind of stream wrapper or > >> iterator, instead of providing the minimal amount of information so that > >> somebody can parse those kind of strings by hand. > >> what do you think? > > I cant get the point of knowing how many bytes of the serialized > format the parser actually consumed. > What would the user be interested in such an information for ? > > Julien Pauli > based on the original commit, and some guessing, I think that Sara bumped into a string serialized through SplDoublyLinkedList::serialize(), which serializes strings a bit differently than serialize(): http://3v4l.org/G3BUY (btw interesting how hhvm introduced yet another format for SplDoublyLinkedList->serialize()) this is probably for performance reasons, you don't have to count the items first to compute the outer wrapper's length(a:2 in this case), but on the other hand it makes data corruption easier to go unnoticed (assume we are storing the serialized data in a database field, mysql using the default settings will silently truncate data when it doesn't fit into the length of the field, and if that happens between two entries that impossible to detect from SplDoublyLinkedList->unserialize() (another interesting thing about hhvm, that it seems to be unable to unserialize even the valid strings, hopefully this is a bug already fixed: http://3v4l.org/bhb4T ) currently unserialize() silently ignores any surplus data at the end of a valid serialized string, so unserializing a SplDoublyLinkedList::serialize()d string will return the first value (which is the serialized value of the internal flags bitmask controlled by the setIteratorMode() call: http://3v4l.org/Ei9ke ) which means that currently you can't really unserialize this string with unserialize() but if unserialize() would be able to tell how much of the data it consumed, you could just get the surplus data based on that offset, and call unserialize() with that data, hence traversing the whole string and gathering the entries into a list/array. and I think that this is an implementation detail of SplDoublyLinkedList and we shouldn't really expose this. I can only guess why Sara haven't used the SplDoublyLinkedList->unserialize() method http://3v4l.org/gWjfK), maybe he wanted to get the values without instanitating an SplDoublyLinkedList and iterate over the list, but if that's the case, and we want to allow these kind of data to be unserialized without the object instanitation and iterating, I would still propose to change SplDoublyLinkedList::unserialize() to allow it to be called statically and return an array containing the values. that would cause only a negligable BC break as SplDoublyLinkedList ->unserialize() currently returns void/null and we would start returning data. -- Ferenc Kovács @Tyr43l - http://tyrael.hu