Unserialize is broken View as Markdown

Heya,

This bug was closed as Bogus http://bugs.php.net/bug.php?id=51173 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();

class testClass {
protected $property;
public function __construct($val) {
$this->property = $val;
}
public function getProperty() {
return $this->property;
}
}

$_SESSION['obj'] = 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();

class testClass {
public $property;
public function __construct($val) {
$this->property = $val;
}
public function getProperty() {
return $this->property;
}
}

if (!isset($_SESSION['obj'])) {
$_SESSION['obj'] = new testClass('value');
}

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' => null
protected 'property' => string 'value' (length=5)
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' => 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