Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:47061
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain seld.be designates 209.85.220.223 as permitted sender)
Message-ID: <4B8A97AF.3060608@seld.be>
Date: Sun, 28 Feb 2010 17:19:59 +0100
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.1.7) Gecko/20100111 Lightning/1.0b2pre Thunderbird/3.0.1
MIME-Version: 1.0
To: internals@lists.php.net
Content-Type: multipart/signed; micalg=pgp-sha1;
 protocol="application/pgp-signature";
 boundary="------------enig78BEAB94FEC21E04948F480F"
Subject: Unserialize is broken
From: j.boggiano@seld.be (Jordi Boggiano)

--------------enig78BEAB94FEC21E04948F480F
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable

Heya,

This bug was closed as Bogus http://bugs.php.net/bug.php?id=3D51173 and
Pierre told me to bring the discussion here since I was ranting on irc.

Johannes argued that the fact unserialize doesn't check the access level
of properties before generating object is good because it allows hackish
features, and that this is intended behavior. Great, but I don't get
what it ultimately allows you to do.

To show this I have expanded my reproduce code and reversed the example:

Run this first to populate the session with an object with a protected
property.

> session_start();
>=20
> class testClass {
>   protected $property;
>   public function __construct($val) {
>     $this->property =3D $val;
>   }
>   public function getProperty() {
>     return $this->property;
>   }
> }
>=20
> $_SESSION['obj'] =3D new testClass('value');

After that you run this script, where the property has been changed to
public, the object that is unserialized from the session has become
completely unusable.

> session_start();
>=20
> class testClass {
>   public $property;
>   public function __construct($val) {
>     $this->property =3D $val;
>   }
>   public function getProperty() {
>     return $this->property;
>   }
> }
>=20
> if (!isset($_SESSION['obj'])) {
>   $_SESSION['obj'] =3D new testClass('value');
> }
>=20
> var_dump($_SESSION['obj']);
> var_dump($_SESSION['obj']->property);
> var_dump($_SESSION['obj']->getProperty());

If you don't want to run it, the output is the following :

object(testClass)[1]
  public 'property' =3D> null
  protected 'property' =3D> string 'value' (length=3D5)
null
null

The "value" you had stored in the object when you serialized it is now
gone and it's impossible to retrieve it, either from inside or outside
the object. If you do it reversed (switch public to protected),
accessing value from outside the object results in a fatal error, even
though there is a "public 'property' =3D> string 'value'" in the var_dump=

output.

Imo unserialize should check, when applying public or protected values,
if either exists on the object, and apply it to the one that exists.
Sure it's gonna cost some performance, but at least changing the
prototype of your class while stuff is running isn't going to kill your
code anymore. For private it's a bit different since private vars belong
to every "class" and not the object as a whole, I guess those should be
unserialized back in their place. I don't see any potential BC break
since doing this atm breaks your code, but people always amazed me with
their ability to create code that relies on broken behavior, so maybe I
overlooked something ?

Cheers,
Jordi


--------------enig78BEAB94FEC21E04948F480F
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename="signature.asc"

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkuKl7UACgkQQxhJaNG8+idcwACcC5rE3mhsHK/p2IS50nCT68Au
picAnAvFaQij5fEQD0AhPSVlTi7D0GhW
=cmQv
-----END PGP SIGNATURE-----

--------------enig78BEAB94FEC21E04948F480F--