Hi,
I was testing few things today and I released one bug in the serialize
function. The serialize function returns a corrupted value when it is
serializing an object that contains a __sleep magic method and this method
return some key that are not string. Ie:
If the __sleep returns a string that is not a property it is fine, it will
give the notice and the generated response is a valid serialized data (in
another words you can use unserialize to retrieve the object back). Ie,
http://viper-7.com/H9ooff
I can try to make a patch to solve it, but before that I would like how the
behavior should be. Some options:
- Give the notice saying the field doesn't exist and do not include on the
serialized response - Give the notice saying the field doesn't exist and convert the value to
string (ie, on my example the int(1) would be transformed to string(1)) - Give a warning and return false once the input from __sleep is invalid
- None of the above alternative
- Don't change it
Thanks,
Juan Basso
No one?
Hi,
I was testing few things today and I released one bug in the serialize
function. The serialize function returns a corrupted value when it is
serializing an object that contains a __sleep magic method and this method
return some key that are not string. Ie:If the __sleep returns a string that is not a property it is fine, it will
give the notice and the generated response is a valid serialized data (in
another words you can use unserialize to retrieve the object back). Ie,
http://viper-7.com/H9ooffI can try to make a patch to solve it, but before that I would like how
the behavior should be. Some options:
- Give the notice saying the field doesn't exist and do not include on
the serialized response- Give the notice saying the field doesn't exist and convert the value to
string (ie, on my example the int(1) would be transformed to string(1))- Give a warning and return false once the input from __sleep is invalid
- None of the above alternative
- Don't change it
Thanks,
Juan Basso
Juan Basso wrote on 05/02/2015 14:07:
No one?
Hi,
I was testing few things today and I released one bug in the serialize
function. The serialize function returns a corrupted value when it is
serializing an object that contains a __sleep magic method and this method
return some key that are not string. Ie:If the __sleep returns a string that is not a property it is fine, it will
give the notice and the generated response is a valid serialized data (in
another words you can use unserialize to retrieve the object back). Ie,
http://viper-7.com/H9ooffI can try to make a patch to solve it, but before that I would like how
the behavior should be. Some options:
- Give the notice saying the field doesn't exist and do not include on
the serialized response- Give the notice saying the field doesn't exist and convert the value to
string (ie, on my example the int(1) would be transformed to string(1))- Give a warning and return false once the input from __sleep is invalid
- None of the above alternative
- Don't change it
Playing around with existing behaviour, if you return something other
than an array, you get a Notice and a serialized null ('N;') - e.g.
http://3v4l.org/rm9rs
I think it would be reasonable for the same to happen if the array
contains something other than a string, particularly given that the
message would still describe the fix quite readily ( "__sleep should
return an array only containing the names of instance-variables to
serialize").
Personally, I'd have made this a Warning rather than a Notice, since
it's clearly discarding data, but it should be consistent either way.
Regards,
Rowan Collins
[IMSoP]
Rowan,
It is happening and giving the same message, but the problem is the
generate value is corrupted because it generates N; without the key. Ie,
when you have a valid field and an invalid field it
generates O:1:"C":2:{s:1:"a";b:1;N;} (note that "a" is the property name, 1
is the boolean value and after that it is just null without the key,
causing the unserialize to not be able to parse it).
If you try to unserialize it gives you the notice "unserialize(): Error at
offset 25 of 26 bytes in XXX".
On Thu, Feb 5, 2015 at 9:57 AM, Rowan Collins rowan.collins@gmail.com
wrote:
Juan Basso wrote on 05/02/2015 14:07:
No one?
Hi,
I was testing few things today and I released one bug in the serialize
function. The serialize function returns a corrupted value when it is
serializing an object that contains a __sleep magic method and this
method
return some key that are not string. Ie:If the __sleep returns a string that is not a property it is fine, it
will
give the notice and the generated response is a valid serialized data (in
another words you can use unserialize to retrieve the object back). Ie,
http://viper-7.com/H9ooffI can try to make a patch to solve it, but before that I would like how
the behavior should be. Some options:
- Give the notice saying the field doesn't exist and do not include on
the serialized response- Give the notice saying the field doesn't exist and convert the value
to
string (ie, on my example the int(1) would be transformed to string(1))- Give a warning and return false once the input from __sleep is invalid
- None of the above alternative
- Don't change it
Playing around with existing behaviour, if you return something other than
an array, you get a Notice and a serialized null ('N;') - e.g.
http://3v4l.org/rm9rsI think it would be reasonable for the same to happen if the array
contains something other than a string, particularly given that the message
would still describe the fix quite readily ( "__sleep should return an
array only containing the names of instance-variables to serialize").Personally, I'd have made this a Warning rather than a Notice, since it's
clearly discarding data, but it should be consistent either way.Regards,
Rowan Collins
[IMSoP]
> On Thu, Feb 5, 2015 at 9:57 AM, Rowan Collins
>
> Playing around with existing behaviour, if you return something
> other than an array, you get a Notice and a serialized null ('N;')
> - e.g. http://3v4l.org/rm9rs
>
> I think it would be reasonable for the same to happen if the array
> contains something other than a string, particularly given that
> the message would still describe the fix quite readily ( "__sleep
> should return an array only containing the names of
> instance-variables to serialize").
>
> Personally, I'd have made this a Warning rather than a Notice,
> since it's clearly discarding data, but it should be consistent
> either way.
>
Juan Basso wrote on 05/02/2015 15:20:
> Rowan,
>
> It is happening and giving the same message, but the problem is the
> generate value is corrupted because it generates N; without the key.
> Ie, when you have a valid field and an invalid field it
> generates O:1:"C":2:{s:1:"a";b:1;N;} (note that "a" is the property
> name, 1 is the boolean value and after that it is just null without
> the key, causing the unserialize to not be able to parse it).
>
> If you try to unserialize it gives you the notice "unserialize():
> Error at offset 25 of 26 bytes in XXX".
Yes, I understand the problem. What I'm saying is that if you return,
say, a string, the result is a validly serialized Null *for the whole
object*.
So:
class A { public function __sleep() { return 'bad value'; } }
var_dump( serialize(new A) );
// Result: string(2) "N;"
I'm suggesting the same behaviour for the currently broken case, so:
class B { public function __sleep() { return ['a', 42]; } }
var_dump( serialize(new B) );
// Current PHP, invalid: string(24) "O:1:"B":2:{s:1:"a";N;N;}"
// Suggested, consistent with other error cases: string(2) "N;"
Interestingly, HHVM generates string(29) "O:1:"B":2:{s:1:"a";N;i:42;N;}"
and will convert the integer key to a string when it is *unserialized*.
Adopting this as standard would be another option, I guess. See
http://3v4l.org/S8lRA
PS I fixed your top posting, because I know people don't like it. :)
Regards,
--
Rowan Collins
[IMSoP]
Thanks for changing the top posting. :)
About the solution, I like the way HHVM solves that. It is also more
consistent with the case where you add the invalid value as string on the
__sleep (ie, on http://3v4l.org/kupOg I changed the 1 from integer to
string).
If you return "N;" is hard to detected if you did serialize(null) or if it
is an error.
Should I try to solve like HHVM for the consistency with different engine
and when the invalid attribute is a string?
On Thu, Feb 5, 2015 at 10:51 AM, Rowan Collins rowan.collins@gmail.com
wrote:
On Thu, Feb 5, 2015 at 9:57 AM, Rowan Collins <rowan.collins@gmail.com
mailto:rowan.collins@gmail.com> wrote:
Playing around with existing behaviour, if you return something other than an array, you get a Notice and a serialized null ('N;') - e.g. http://3v4l.org/rm9rs I think it would be reasonable for the same to happen if the array contains something other than a string, particularly given that the message would still describe the fix quite readily ( "__sleep should return an array only containing the names of instance-variables to serialize"). Personally, I'd have made this a Warning rather than a Notice, since it's clearly discarding data, but it should be consistent either way.Juan Basso wrote on 05/02/2015 15:20:
Rowan,
It is happening and giving the same message, but the problem is the
generate value is corrupted because it generates N; without the key. Ie,
when you have a valid field and an invalid field it generates
O:1:"C":2:{s:1:"a";b:1;N;} (note that "a" is the property name, 1 is the
boolean value and after that it is just null without the key, causing the
unserialize to not be able to parse it).If you try to unserialize it gives you the notice "unserialize(): Error
at offset 25 of 26 bytes in XXX".Yes, I understand the problem. What I'm saying is that if you return, say,
a string, the result is a validly serialized Null for the whole object.So:
class A { public function __sleep() { return 'bad value'; } }
var_dump( serialize(new A) );
// Result: string(2) "N;"I'm suggesting the same behaviour for the currently broken case, so:
class B { public function __sleep() { return ['a', 42]; } }
var_dump( serialize(new B) );
// Current PHP, invalid: string(24) "O:1:"B":2:{s:1:"a";N;N;}"
// Suggested, consistent with other error cases: string(2) "N;"Interestingly, HHVM generates string(29) "O:1:"B":2:{s:1:"a";N;i:42;N;}"
and will convert the integer key to a string when it is unserialized.
Adopting this as standard would be another option, I guess. See
http://3v4l.org/S8lRAPS I fixed your top posting, because I know people don't like it. :)
Regards,
Rowan Collins
[IMSoP]
Hi!
I can try to make a patch to solve it, but before that I would like how
the behavior should be. Some options:
- Give the notice saying the field doesn't exist and do not include on
the serialized response- Give the notice saying the field doesn't exist and convert the value to
string (ie, on my example the int(1) would be transformed to string(1))
I'd try (2) if it doesn't work (which probably would be the case unless
your object happens to have property named "42" somehow) then produce a
notice and omit it. If __sleep returns wrong value completely - e.g.
false, I'd produce a warning and serialize it as null.
Stas Malyshev
smalyshev@gmail.com
Hi,
Thanks for the feedback.
I opened the pull request (https://github.com/php/php-src/pull/1057). Is
that correct?
On Thu, Feb 5, 2015 at 12:30 PM, Stanislav Malyshev smalyshev@gmail.com
wrote:
Hi!
I can try to make a patch to solve it, but before that I would like how
the behavior should be. Some options:
- Give the notice saying the field doesn't exist and do not include on
the serialized response- Give the notice saying the field doesn't exist and convert the value
to
string (ie, on my example the int(1) would be transformed to string(1))I'd try (2) if it doesn't work (which probably would be the case unless
your object happens to have property named "42" somehow) then produce a
notice and omit it. If __sleep returns wrong value completely - e.g.
false, I'd produce a warning and serialize it as null.Stas Malyshev
smalyshev@gmail.com