Skip to content

PHPC-274: Fix zval_to_bson() encoding of BSON\Serializable objects #40

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 4, 2015
Merged

PHPC-274: Fix zval_to_bson() encoding of BSON\Serializable objects #40

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9e59cee
PHPC-274: Fix zval_to_bson() encoding of BSON\Serializable objects
jmikola Apr 25, 2015
047bea5
PHPC-275: object_to_bson() handling for invalid bsonSerialize() retval
jmikola Apr 26, 2015
56e5567
PHPC-274: Regression test for root BSON\Serializable encoding
jmikola Apr 26, 2015
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
82 changes: 49 additions & 33 deletions src/bson.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -607,34 +607,43 @@ void object_to_bson(zval *object, php_phongo_bson_flags_t flags, const char *key

if (instanceof_function(Z_OBJCE_P(object), php_phongo_type_ce TSRMLS_CC)) {
if (instanceof_function(Z_OBJCE_P(object), php_phongo_serializable_ce TSRMLS_CC)) {
zval *retval = NULL;
zval *obj_data = NULL;
bson_t child;
HashTable *tmp_ht;

zend_call_method_with_0_params(&object, NULL, NULL, BSON_SERIALIZE_FUNC_NAME, &retval);
if (retval) {
bson_t child;
HashTable *tmp_ht;
zend_call_method_with_0_params(&object, NULL, NULL, BSON_SERIALIZE_FUNC_NAME, &obj_data);

convert_to_array_ex(&retval);
tmp_ht = HASH_OF(retval);
if (!obj_data) {
/* zend_call_method() failed */
return;
}

if (tmp_ht) {
tmp_ht->nApplyCount++;
}
if (Z_TYPE_P(obj_data) != IS_ARRAY) {
phongo_throw_exception(PHONGO_ERROR_RUNTIME TSRMLS_CC, "Expected %s() to return an array, %s given", BSON_SERIALIZE_FUNC_NAME, zend_get_type_by_const(Z_TYPE_P(obj_data)));
Copy link
Member Author

Choose a reason for hiding this comment

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

This should be an UnexpectedValueException.

Copy link
Contributor

Choose a reason for hiding this comment

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

Change this to UnexpectedValueException please

Copy link
Member Author

Choose a reason for hiding this comment

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

#43

zval_ptr_dtor(&obj_data);

bson_append_document_begin(bson, key, key_len, &child);
if (instanceof_function(Z_OBJCE_P(object), php_phongo_persistable_ce TSRMLS_CC)) {
if (flags & PHONGO_BSON_ADD_CHILD_ODS) {
bson_append_binary(&child, PHONGO_ODM_FIELD_NAME, -1, 0x80, (const uint8_t *)Z_OBJCE_P(object)->name, strlen(Z_OBJCE_P(object)->name));
}
}
zval_to_bson(retval, flags, &child, NULL TSRMLS_CC);
bson_append_document_end(bson, &child);
return;
Copy link
Member Author

Choose a reason for hiding this comment

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

For PHPC-275: this conditional and the preceding (!obj_data) conditional (when zend_call_method() fails) are the relevant changes for the bson-encode_error-002.phpt test case.

}

if (tmp_ht) {
tmp_ht->nApplyCount--;
tmp_ht = HASH_OF(obj_data);

if (tmp_ht) {
tmp_ht->nApplyCount++;
}

bson_append_document_begin(bson, key, key_len, &child);
if (instanceof_function(Z_OBJCE_P(object), php_phongo_persistable_ce TSRMLS_CC)) {
if (flags & PHONGO_BSON_ADD_CHILD_ODS) {
bson_append_binary(&child, PHONGO_ODM_FIELD_NAME, -1, 0x80, (const uint8_t *)Z_OBJCE_P(object)->name, strlen(Z_OBJCE_P(object)->name));
}
zval_ptr_dtor(&retval);
}
zval_to_bson(obj_data, flags, &child, NULL TSRMLS_CC);
bson_append_document_end(bson, &child);

if (tmp_ht) {
tmp_ht->nApplyCount--;
}
zval_ptr_dtor(&obj_data);
return;
}

Expand Down Expand Up @@ -763,23 +772,30 @@ PHONGO_API void zval_to_bson(zval *data, php_phongo_bson_flags_t flags, bson_t *

switch(Z_TYPE_P(data)) {
case IS_OBJECT:
if (instanceof_function(Z_OBJCE_P(data), php_phongo_persistable_ce TSRMLS_CC)) {
if (instanceof_function(Z_OBJCE_P(data), php_phongo_serializable_ce TSRMLS_CC)) {
Copy link
Member Author

Choose a reason for hiding this comment

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

For PHPC-274: this swap of php_phongo_persistable_ce to php_phongo_serializable_ce is the relevant change for satisfying the non-embedded test cases in bson-encode-002.phpt (the embedded test cases already passed before this).

zend_call_method_with_0_params(&data, NULL, NULL, BSON_SERIALIZE_FUNC_NAME, &obj_data);

if (flags & PHONGO_BSON_ADD_ODS) {
bson_append_binary(bson, PHONGO_ODM_FIELD_NAME, -1, 0x80, (const uint8_t *)Z_OBJCE_P(data)->name, strlen(Z_OBJCE_P(data)->name));
if (!obj_data) {
/* zend_call_method() failed */
return;
Copy link
Member Author

Choose a reason for hiding this comment

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

Instead of returning here and after an exception below, we should break and then change the if (ht_data && ht_data->nApplyCount > 1) conditional later to also return if ht_data is NULL. Additionally, ensure we initialize ht_data to NULL at the top of this function.

Copy link
Member Author

Choose a reason for hiding this comment

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

#43

}

zend_call_method_with_0_params(&data, NULL, NULL, BSON_SERIALIZE_FUNC_NAME, &obj_data);
if(obj_data) {
if (Z_TYPE_P(obj_data) == IS_ARRAY) {
ht_data = HASH_OF(obj_data);
} else {
phongo_throw_exception(PHONGO_ERROR_RUNTIME TSRMLS_CC, "%s", "Return value expected to be array");
zval_ptr_dtor(&obj_data);
}
if (Z_TYPE_P(obj_data) != IS_ARRAY) {
phongo_throw_exception(PHONGO_ERROR_RUNTIME TSRMLS_CC, "Expected %s() to return an array, %s given", BSON_SERIALIZE_FUNC_NAME, zend_get_type_by_const(Z_TYPE_P(obj_data)));
Copy link
Member Author

Choose a reason for hiding this comment

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

This should also be an UnexpectedValueException.

zval_ptr_dtor(&obj_data);

break;
return;
}

ht_data = HASH_OF(obj_data);

if (instanceof_function(Z_OBJCE_P(data), php_phongo_persistable_ce TSRMLS_CC)) {
Copy link
Member Author

Choose a reason for hiding this comment

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

For PHPC-274: since we swapped the instanceof check above, we still want to respect the PHONGO_BSON_ADD_ODS flag, so this conditional has been added.

if (flags & PHONGO_BSON_ADD_ODS) {
bson_append_binary(bson, PHONGO_ODM_FIELD_NAME, -1, 0x80, (const uint8_t *)Z_OBJCE_P(data)->name, strlen(Z_OBJCE_P(data)->name));
}
}

break;
}
/* break intentionally omitted */

Expand Down
138 changes: 102 additions & 36 deletions tests/bson/bson-encode-002.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,65 +6,130 @@ BSON encoding: Encoding objects into BSON representation
<?php
require_once __DIR__ . "/../utils/basic.inc";

class MyClass implements BSON\Serializable, BSON\Unserializable {
function bsonSerialize() {
return array(
"random" => "class",
"data"
);
class AssociativeArray implements BSON\Serializable, BSON\Unserializable
{
public function bsonSerialize()
{
return array("random" => "class", "data");
}
function bsonUnserialize(array $data) {
var_dump(__METHOD__, $data);

public function bsonUnserialize(array $data)
{
echo __METHOD__, "() was called with data:\n";
var_dump($data);
}
}
class MyClass2 implements BSON\Serializable, BSON\Unserializable {
function bsonSerialize() {
return array(
1, 2, 3,
);

class NumericArray implements BSON\Serializable, BSON\Unserializable
{
public function bsonSerialize()
{
return array(1, 2, 3);
}
function bsonUnserialize(array $data) {
var_dump(__METHOD__, $data);

public function bsonUnserialize(array $data)
{
echo __METHOD__, "() was called with data:\n";
var_dump($data);
}
}

$tests = array(
array("stuff" => new MyClass),
array("stuff" => new MyClass2),
);
echo "Testing top-level AssociativeArray:\n";
$bson = BSON\fromArray(new AssociativeArray);
echo BSON\toJSON($bson), "\n";
echo "Encoded BSON:\n";
hex_dump($bson);
$value = BSON\toArray($bson, array("document" => 'AssociativeArray'));
echo "Decoded BSON:\n";
var_dump($value);

echo "\nTesting embedded AssociativeArray:\n";
$bson = BSON\fromArray(array('embed' => new AssociativeArray));
echo BSON\toJSON($bson), "\n";
echo "Encoded BSON:\n";
hex_dump($bson);
$value = BSON\toArray($bson, array("document" => 'AssociativeArray'));
echo "Decoded BSON:\n";
var_dump($value);

echo "\nTesting top-level NumericArray:\n";
$bson = BSON\fromArray(new NumericArray);
echo BSON\toJSON($bson), "\n";
echo "Encoded BSON:\n";
hex_dump($bson);
$value = BSON\toArray($bson, array("document" => 'NumericArray'));
echo "Decoded BSON:\n";
var_dump($value);

echo "\nTesting embedded NumericArray:\n";
$bson = BSON\fromArray(array('embed' => new NumericArray));
echo BSON\toJSON($bson), "\n";
echo "Encoded BSON:\n";
hex_dump($bson);
$value = BSON\toArray($bson, array("document" => 'NumericArray'));
echo "Decoded BSON:\n";
var_dump($value);

foreach($tests as $n => $test) {
$s = BSON\fromArray($test);
echo "Test#{$n} ", BSON\toJSON($s), "\n";
hex_dump($s);
$obj = BSON\toArray($s, array("document" => get_class($test["stuff"])));
var_dump($obj);
}
?>
===DONE===
<?php exit(0); ?>
--EXPECTF--
Test#0 { "stuff" : { "random" : "class", "0" : "data" } }
0 : 2f 00 00 00 03 73 74 75 66 66 00 23 00 00 00 02 [/....stuff.#....]
Testing top-level AssociativeArray:
{ "random" : "class", "0" : "data" }
Encoded BSON:
0 : 23 00 00 00 02 72 61 6e 64 6f 6d 00 06 00 00 00 [#....random.....]
10 : 63 6c 61 73 73 00 02 30 00 05 00 00 00 64 61 74 [class..0.....dat]
20 : 61 00 00 [a..]
Decoded BSON:
array(2) {
["random"]=>
string(5) "class"
[0]=>
string(4) "data"
}

Testing embedded AssociativeArray:
{ "embed" : { "random" : "class", "0" : "data" } }
Encoded BSON:
0 : 2f 00 00 00 03 65 6d 62 65 64 00 23 00 00 00 02 [/....embed.#....]
10 : 72 61 6e 64 6f 6d 00 06 00 00 00 63 6c 61 73 73 [random.....class]
20 : 00 02 30 00 05 00 00 00 64 61 74 61 00 00 00 [..0.....data...]
string(24) "MyClass::bsonUnserialize"
AssociativeArray::bsonUnserialize() was called with data:
array(2) {
["random"]=>
string(5) "class"
[0]=>
string(4) "data"
}
Decoded BSON:
array(1) {
["stuff"]=>
object(MyClass)#%d (0) {
["embed"]=>
object(AssociativeArray)#%d (0) {
}
}
Test#1 { "stuff" : { "0" : 1, "1" : 2, "2" : 3 } }
0 : 26 00 00 00 03 73 74 75 66 66 00 1a 00 00 00 10 [&....stuff......]

Testing top-level NumericArray:
{ "0" : 1, "1" : 2, "2" : 3 }
Encoded BSON:
0 : 1a 00 00 00 10 30 00 01 00 00 00 10 31 00 02 00 [.....0......1...]
10 : 00 00 10 32 00 03 00 00 00 00 [...2......]
Decoded BSON:
array(3) {
[0]=>
int(1)
[1]=>
int(2)
[2]=>
int(3)
}

Testing embedded NumericArray:
{ "embed" : { "0" : 1, "1" : 2, "2" : 3 } }
Encoded BSON:
0 : 26 00 00 00 03 65 6d 62 65 64 00 1a 00 00 00 10 [&....embed......]
10 : 30 00 01 00 00 00 10 31 00 02 00 00 00 10 32 00 [0......1......2.]
20 : 03 00 00 00 00 00 [......]
string(25) "MyClass2::bsonUnserialize"
NumericArray::bsonUnserialize() was called with data:
array(3) {
[0]=>
int(1)
Expand All @@ -73,9 +138,10 @@ array(3) {
[2]=>
int(3)
}
Decoded BSON:
array(1) {
["stuff"]=>
object(MyClass2)#%d (0) {
["embed"]=>
object(NumericArray)#%d (0) {
}
}
===DONE===
42 changes: 42 additions & 0 deletions tests/bson/bson-encode_error-001.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
--TEST--
BSON encoding error when bsonSerialize() for root document does not return an array
--SKIPIF--
<?php require __DIR__ . "/../utils/basic-skipif.inc"?>
--FILE--
<?php
require_once __DIR__ . "/../utils/basic.inc";

class MyClass implements BSON\Serializable
{
private $value;

public function __construct($value)
{
$this->value = $value;
}

public function bsonSerialize()
{
return $this->value;
}
}

$invalidValues = array(new stdClass, 'foo', 1, true);

foreach ($invalidValues as $invalidValue) {
try {
BSON\fromArray(new MyClass($invalidValue));
} catch (MongoDB\Driver\Exception\RuntimeException $e) {
echo $e->getMessage(), "\n";
}
}

?>
===DONE===
<?php exit(0); ?>
--EXPECT--
Expected bsonSerialize() to return an array, object given
Expected bsonSerialize() to return an array, string given
Expected bsonSerialize() to return an array, integer given
Expected bsonSerialize() to return an array, boolean given
===DONE===
42 changes: 42 additions & 0 deletions tests/bson/bson-encode_error-002.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
--TEST--
BSON encoding error when bsonSerialize() for embedded document does not return an array
--SKIPIF--
<?php require __DIR__ . "/../utils/basic-skipif.inc"?>
--FILE--
<?php
require_once __DIR__ . "/../utils/basic.inc";

class MyClass implements BSON\Serializable
{
private $value;

public function __construct($value)
{
$this->value = $value;
}

public function bsonSerialize()
{
return $this->value;
}
}

$invalidValues = array(new stdClass, 'foo', 1, true);

foreach ($invalidValues as $invalidValue) {
try {
$bson = BSON\fromArray(array('embed' => new MyClass($invalidValue)));
} catch (MongoDB\Driver\Exception\RuntimeException $e) {
echo $e->getMessage(), "\n";
}
}

?>
===DONE===
<?php exit(0); ?>
--EXPECT--
Expected bsonSerialize() to return an array, object given
Expected bsonSerialize() to return an array, string given
Expected bsonSerialize() to return an array, integer given
Expected bsonSerialize() to return an array, boolean given
===DONE===
Loading