Skip to content
This repository has been archived by the owner on Jan 31, 2020. It is now read-only.

Removal of Eval and amended unit test #5

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 91 additions & 22 deletions src/Adapter/PhpCode.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,52 +9,121 @@

namespace Zend\Serializer\Adapter;

use Zend\Serializer\Exception;
use Zend\Json\Decoder;
use Zend\Json\Encoder;
use Zend\Stdlib\ErrorHandler;
use Zend\Serializer\Exception;

class PhpCode extends AbstractAdapter
{
/**
* Serialize PHP using var_export
* Encoder/Serialize PHP using Zend\Json\Encoder or serialize
*
* @param mixed $value
* @return string
* @throws Exception\RuntimeException only if the parameter is already serialized
*/
public function serialize($value)
{
return var_export($value, true);
if ($this->isSerialized($value)) {
throw new Exception\RuntimeException('Value is already serialized');
}

if (!is_object($value)) {
return serialize($value);
}

return Encoder::encode($value);
}

/**
* Deserialize PHP string
*
* Warning: this uses eval(), and should likely be avoided.
* @param string $code
* @return mixed (can be string or object)
*/
public function unserialize($code)
{
if (!$this->validateString($code)) {
return unserialize($code);
}

$decoded = @Decoder::decode($code);

if (!is_object($decoded) && $this->isSerialized($decoded)) {
return unserialize($decoded);
}

if (is_object($decoded)) {
$code = $this->extractClass($decoded);
}

return $code;
}

/**
* Validation to check if we can unserialize and/or decode
*
* @param $code
* @return bool
*/
private function validateString($code)
{
if (!$this->isSerialized($code) && @unserialize($code) === $code) {
return false;
}

try {
Decoder::decode($code);
} catch (\Exception $e) {
return false;
}

return true;
}

/**
* @param \StdClass $object
*
* @param string $code
* @return mixed
* @throws Exception\RuntimeException on eval error
* @throws Exception\RuntimeException if no class name is found, then we cannot re-generate the class
*/
public function unserialize($code)
private function extractClass(\StdClass $object)
{
ErrorHandler::start(E_ALL);
$ret = null;
// This suppression is due to the fact that the ErrorHandler cannot
// catch syntax errors, and is intentionally left in place.
$eval = @eval('$ret=' . $code . ';');
$err = ErrorHandler::stop();
if ($object->__className === null || $object->__className === false) {
throw new Exception\RuntimeException('Cannot retrieve class name');
}

// Get original class name
$className = $object->__className;

// Retrieve current class name
$serializedObjClassName = strstr(serialize($object), '"');
$stdClassName = strstr($serializedObjClassName, ':');

if ($eval === false || $err) {
$msg = 'eval failed';
// Replace class name with the original class name
$serializedObject = sprintf('O:%d:"%s"%s', strlen($className), $className, $stdClassName);

// Unserialize back into object
return unserialize($serializedObject);
}

// Error handler doesn't catch syntax errors
if ($eval === false) {
$lastErr = error_get_last();
$msg .= ': ' . $lastErr['message'];
}
/**
* Check if string is already serialized
*
* @param mixed $code
* @return bool
*/
private function isSerialized($code)
{
ErrorHandler::start(E_ALL);
$unserialize = unserialize($code);
$errors = ErrorHandler::stop();

throw new Exception\RuntimeException($msg, 0, $err);
if ($errors === false || $unserialize === 'b:0;') {
return true;
}

return $ret;
return false;
}
}
147 changes: 53 additions & 94 deletions test/Adapter/PhpCodeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,131 +6,90 @@
* @copyright Copyright (c) 2005-2015 Zend Technologies USA Inc. (http://www.zend.com)
* @license http://framework.zend.com/license/new-bsd New BSD License
*/

namespace ZendTest\Serializer\Adapter;

use Zend\Serializer;
use ZendTest\Serializer\TestAsset\Dummy;
use Zend\Json\Encoder;

/**
* @group Zend_Serializer
* @covers Zend\Serializer\Adapter\PhpCode
*/
class PhpCodeTest extends \PHPUnit_Framework_TestCase
{
/**
* @var Serializer\Adapter\PhpCode
*/
/** @var Serializer\Adapter\PhpCode */
private $adapter;

public function setUp()
{
$this->adapter = new Serializer\Adapter\PhpCode();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Import the final class instead the only the namespace. This is the only class imported.

}

public function tearDown()
{
$this->adapter = null;
}

public function testSerializeString()
{
$value = 'test';
$expected = "'test'";

$data = $this->adapter->serialize($value);
$this->assertEquals($expected, $data);
}

public function testSerializeFalse()
{
$value = false;
$expected = 'false';

$data = $this->adapter->serialize($value);
$this->assertEquals($expected, $data);
}

public function testSerializeNull()
{
$value = null;
$expected = 'NULL';

$data = $this->adapter->serialize($value);
$this->assertEquals($expected, $data);
}

public function testSerializeNumeric()
{
$value = 100.12345;
$expected = '100.12345';

$data = $this->adapter->serialize($value);
$this->assertEquals($expected, $data);
}

/**
* Test when serializing a PHP object it matches the
* encode process
*
* Unserialize on PHP objects occur on Zend\Json\Encoder::encode
*/
public function testSerializeObject()
{
$value = new \stdClass();
$expected = "stdClass::__set_state(array(\n))";
$object = new Dummy();
$data = $this->adapter->serialize($object);

$data = $this->adapter->serialize($value);
$this->assertEquals($expected, $data);
$this->assertEquals(Encoder::encode($object), $data);
}

public function testUnserializeString()
/**
* Test when unserializing a PHP object it matches
* the the same instance of original class
*
* Unserialize on PHP objects occur on Zend\Json\Decoder::decode
*/
public function testUnserializeObject()
{
$value = "'test'";
$expected = 'test';
$expected = new Dummy();
$serialized = $this->adapter->serialize($expected);

$data = $this->adapter->unserialize($value);
$this->assertEquals($expected, $data);
}

public function testUnserializeFalse()
{
$value = 'false';
$expected = false;
$data = $this->adapter->unserialize($serialized);

$data = $this->adapter->unserialize($value);
$this->assertEquals($expected, $data);
$this->assertInstanceOf(get_class($expected), $data);
}

public function testUnserializeNull()
{
$value = 'NULL';
$expected = null;

$data = $this->adapter->unserialize($value);
$this->assertEquals($expected, $data);
}

public function testUnserializeNumeric()
/**
* @dataProvider serializedValuesProvider
*/
public function testSerialize($unserialized, $serialized)
{
$value = '100';
$expected = 100;

$data = $this->adapter->unserialize($value);
$this->assertEquals($expected, $data);
$this->assertEquals($serialized, $this->adapter->serialize($unserialized));
}

/* TODO: PHP Fatal error: Call to undefined method stdClass::__set_state()
public function testUnserializeObject()
/**
* @dataProvider serializedValuesProvider
*/
public function testUnserialize($unserialized, $serialized)
{
$value = "stdClass::__set_state(array(\n))";
$expected = new stdClass();

$data = $this->adapter->unserialize($value);
$this->assertEquals($expected, $data);
$this->assertEquals($unserialized, $this->adapter->unserialize($serialized));
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test should work without changes. Otherwise this PR introduces a BC Break.

You'll find why JSON Encoder is not the solution. The adapter emulate php native serialize/unserialize without call the native implementation.

JSON Encoded adapter is this https://github.com/zendframework/zend-serializer/blob/master/src/Adapter/Json.php

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we keep this test and make the changes suggested, from what I see the 'unserialize' will still fail. Mainly due to __set_status: "PHP Fatal error: Call to undefined method stdClass::__set_state()". The testUnserializeObject is skipped because of this.

This was the main reason why I decided to re-work this adapter altogether. What are your thoughts?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the original issue is not resolved. This adapter has as objective serialize and unserialize using the PHP string format and not the JSON format.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we change the format, then this test wouldn't work unless we change the expected value.

*/

public function testUnserializeInvalid()
public function serializedValuesProvider()
{
if (version_compare(PHP_VERSION, '7', 'ge')) {
$this->markTestSkipped('Cannot catch parse errors in PHP 7+');
}
$value = 'not a serialized string';

$this->setExpectedException('Zend\Serializer\Exception\RuntimeException', 'syntax error');
$this->adapter->unserialize($value);
return [
// Description => [unserialized, serialized]
'String' => ['test', serialize('test')],
'true' => [true, serialize(true)],
'false' => [false, serialize(false)],
'null' => [null, serialize(null)],
'int' => [1, serialize(1)],
'float' => [1.2, serialize(1.2)],

// Boolean as string
'"true"' => ['true', serialize('true')],
'"false"' => ['false', serialize('false')],
'"null"' => ['null', serialize('null')],
'"1"' => ['1', serialize('1')],
'"1.2"' => ['1.2', serialize('1.2')],

'PHP Code with tags' => ['<?php echo "test"; ?>', serialize('<?php echo "test"; ?>')]
];
}
}
7 changes: 5 additions & 2 deletions test/TestAsset/Dummy.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,8 @@

namespace ZendTest\Serializer\TestAsset;

class Dummy
{}
class Dummy {

public $test = '1';

}