Skip to content

Commit 58f432f

Browse files
committed
PHPC-948: BSON encoding should throw on circular references
1 parent c8f6bad commit 58f432f

File tree

4 files changed

+128
-87
lines changed

4 files changed

+128
-87
lines changed

src/bson.c

Lines changed: 32 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1019,7 +1019,7 @@ void object_to_bson(zval *object, php_phongo_bson_flags_t flags, const char *key
10191019
zval *obj_data = NULL;
10201020
#endif
10211021
bson_t child;
1022-
HashTable *tmp_ht;
1022+
10231023
#if PHP_VERSION_ID >= 70000
10241024
zend_call_method_with_0_params(object, NULL, NULL, BSON_SERIALIZE_FUNC_NAME, &obj_data);
10251025
#else
@@ -1060,16 +1060,6 @@ void object_to_bson(zval *object, php_phongo_bson_flags_t flags, const char *key
10601060
return;
10611061
}
10621062

1063-
#if PHP_VERSION_ID >= 70000
1064-
tmp_ht = HASH_OF(&obj_data);
1065-
#else
1066-
tmp_ht = HASH_OF(obj_data);
1067-
#endif
1068-
1069-
if (tmp_ht && ZEND_HASH_APPLY_PROTECTION(tmp_ht)) {
1070-
ZEND_HASH_INC_APPLY_COUNT(tmp_ht);
1071-
}
1072-
10731063
/* Persistable objects must always be serialized as BSON documents;
10741064
* otherwise, infer based on bsonSerialize()'s return value. */
10751065
#if PHP_VERSION_ID >= 70000
@@ -1101,9 +1091,6 @@ void object_to_bson(zval *object, php_phongo_bson_flags_t flags, const char *key
11011091
bson_append_array_end(bson, &child);
11021092
}
11031093

1104-
if (tmp_ht && ZEND_HASH_APPLY_PROTECTION(tmp_ht)) {
1105-
ZEND_HASH_DEC_APPLY_COUNT(tmp_ht);
1106-
}
11071094
zval_ptr_dtor(&obj_data);
11081095
return;
11091096
}
@@ -1176,20 +1163,10 @@ void object_to_bson(zval *object, php_phongo_bson_flags_t flags, const char *key
11761163
phongo_throw_exception(PHONGO_ERROR_UNEXPECTED_VALUE TSRMLS_CC, "Unexpected %s instance: %s", ZSTR_VAL(php_phongo_type_ce->name), ZSTR_VAL(Z_OBJCE_P(object)->name));
11771164
return;
11781165
} else {
1179-
HashTable *tmp_ht = HASH_OF(object);
1180-
1181-
if (tmp_ht && ZEND_HASH_APPLY_PROTECTION(tmp_ht)) {
1182-
ZEND_HASH_INC_APPLY_COUNT(tmp_ht);
1183-
}
1184-
11851166
mongoc_log(MONGOC_LOG_LEVEL_TRACE, MONGOC_LOG_DOMAIN, "encoding document");
11861167
bson_append_document_begin(bson, key, key_len, &child);
11871168
phongo_zval_to_bson(object, flags, &child, NULL TSRMLS_CC);
11881169
bson_append_document_end(bson, &child);
1189-
1190-
if (tmp_ht && ZEND_HASH_APPLY_PROTECTION(tmp_ht)) {
1191-
ZEND_HASH_DEC_APPLY_COUNT(tmp_ht);
1192-
}
11931170
}
11941171
}
11951172

@@ -1238,6 +1215,11 @@ static void phongo_bson_append(bson_t *bson, php_phongo_bson_flags_t flags, cons
12381215
bson_t child;
12391216
HashTable *tmp_ht = HASH_OF(entry);
12401217

1218+
if (tmp_ht && ZEND_HASH_GET_APPLY_COUNT(tmp_ht) > 0) {
1219+
phongo_throw_exception(PHONGO_ERROR_UNEXPECTED_VALUE TSRMLS_CC, "Detected recursion for fieldname \"%s\"", key);
1220+
break;
1221+
}
1222+
12411223
if (tmp_ht && ZEND_HASH_APPLY_PROTECTION(tmp_ht)) {
12421224
ZEND_HASH_INC_APPLY_COUNT(tmp_ht);
12431225
}
@@ -1252,9 +1234,25 @@ static void phongo_bson_append(bson_t *bson, php_phongo_bson_flags_t flags, cons
12521234
break;
12531235
}
12541236
/* break intentionally omitted */
1255-
case IS_OBJECT:
1237+
case IS_OBJECT: {
1238+
HashTable *tmp_ht = HASH_OF(entry);
1239+
1240+
if (tmp_ht && ZEND_HASH_GET_APPLY_COUNT(tmp_ht) > 0) {
1241+
phongo_throw_exception(PHONGO_ERROR_UNEXPECTED_VALUE TSRMLS_CC, "Detected recursion for fieldname \"%s\"", key);
1242+
break;
1243+
}
1244+
1245+
if (tmp_ht && ZEND_HASH_APPLY_PROTECTION(tmp_ht)) {
1246+
ZEND_HASH_INC_APPLY_COUNT(tmp_ht);
1247+
}
1248+
12561249
object_to_bson(entry, flags, key, key_len, bson TSRMLS_CC);
1250+
1251+
if (tmp_ht && ZEND_HASH_APPLY_PROTECTION(tmp_ht)) {
1252+
ZEND_HASH_DEC_APPLY_COUNT(tmp_ht);
1253+
}
12571254
break;
1255+
}
12581256

12591257
#if PHP_VERSION_ID >= 70000
12601258
case IS_INDIRECT:
@@ -1285,6 +1283,8 @@ void phongo_zval_to_bson(zval *data, php_phongo_bson_flags_t flags, bson_t *bson
12851283
* properties, we'll need to filter them out later. */
12861284
bool ht_data_from_properties = false;
12871285

1286+
ZVAL_UNDEF(&obj_data);
1287+
12881288
switch(Z_TYPE_P(data)) {
12891289
case IS_OBJECT:
12901290
if (instanceof_function(Z_OBJCE_P(data), php_phongo_serializable_ce TSRMLS_CC)) {
@@ -1296,7 +1296,7 @@ void phongo_zval_to_bson(zval *data, php_phongo_bson_flags_t flags, bson_t *bson
12961296

12971297
if (Z_ISUNDEF(obj_data)) {
12981298
/* zend_call_method() failed */
1299-
break;
1299+
return;
13001300
}
13011301

13021302
#if PHP_VERSION_ID >= 70000
@@ -1324,7 +1324,7 @@ void phongo_zval_to_bson(zval *data, php_phongo_bson_flags_t flags, bson_t *bson
13241324
)
13251325
);
13261326

1327-
break;
1327+
goto cleanup;
13281328
}
13291329

13301330
#if PHP_VERSION_ID >= 70000
@@ -1349,7 +1349,7 @@ void phongo_zval_to_bson(zval *data, php_phongo_bson_flags_t flags, bson_t *bson
13491349
if (instanceof_function(Z_OBJCE_P(data), php_phongo_type_ce TSRMLS_CC)) {
13501350
phongo_throw_exception(PHONGO_ERROR_UNEXPECTED_VALUE TSRMLS_CC, "%s instance %s cannot be serialized as a root element", ZSTR_VAL(php_phongo_type_ce->name), ZSTR_VAL(Z_OBJCE_P(data)->name));
13511351

1352-
break;
1352+
return;
13531353
}
13541354

13551355
ht_data = Z_OBJ_HT_P(data)->get_properties(data TSRMLS_CC);
@@ -1364,19 +1364,6 @@ void phongo_zval_to_bson(zval *data, php_phongo_bson_flags_t flags, bson_t *bson
13641364
return;
13651365
}
13661366

1367-
if (!ht_data || ZEND_HASH_GET_APPLY_COUNT(ht_data) > 1) {
1368-
#if PHP_VERSION_ID >= 70000
1369-
if (Z_TYPE_P(data) == IS_OBJECT && instanceof_function(Z_OBJCE_P(data), php_phongo_serializable_ce TSRMLS_CC)) {
1370-
#endif
1371-
if (!Z_ISUNDEF(obj_data)) {
1372-
zval_ptr_dtor(&obj_data);
1373-
}
1374-
#if PHP_VERSION_ID >= 70000
1375-
}
1376-
#endif
1377-
return;
1378-
}
1379-
13801367
#if PHP_VERSION_ID >= 70000
13811368
{
13821369
zend_string *string_key = NULL;
@@ -1483,15 +1470,11 @@ void phongo_zval_to_bson(zval *data, php_phongo_bson_flags_t flags, bson_t *bson
14831470
}
14841471
}
14851472
}
1486-
#if PHP_VERSION_ID >= 70000
1487-
if (Z_TYPE_P(data) == IS_OBJECT && instanceof_function(Z_OBJCE_P(data), php_phongo_serializable_ce TSRMLS_CC)) {
1488-
#endif
1489-
if (!Z_ISUNDEF(obj_data)) {
1490-
zval_ptr_dtor(&obj_data);
1491-
}
1492-
#if PHP_VERSION_ID >= 70000
1473+
1474+
cleanup:
1475+
if (!Z_ISUNDEF(obj_data)) {
1476+
zval_ptr_dtor(&obj_data);
14931477
}
1494-
#endif
14951478
}
14961479

14971480
/* }}} */

tests/bson/bson-fromPHP-004.phpt

Lines changed: 0 additions & 38 deletions
This file was deleted.
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
--TEST--
2+
BSON\fromPHP(): PHP documents with circular references
3+
--FILE--
4+
<?php
5+
6+
require_once __DIR__ . '/../utils/tools.php';
7+
8+
echo "\nTesting packed array with circular reference\n";
9+
10+
echo throws(function() {
11+
$document = ['x' => 1, 'y' => []];
12+
$document['y'][] = &$document['y'];
13+
fromPHP($document);
14+
}, 'MongoDB\Driver\Exception\UnexpectedValueException'), "\n";
15+
16+
echo "\nTesting associative array with circular reference\n";
17+
18+
echo throws(function() {
19+
$document = ['x' => 1, 'y' => []];
20+
$document['y']['z'] = &$document['y'];
21+
fromPHP($document);
22+
}, 'MongoDB\Driver\Exception\UnexpectedValueException'), "\n";
23+
24+
echo "\nTesting object with circular reference\n";
25+
26+
echo throws(function() {
27+
$document = (object) ['x' => 1, 'y' => (object) []];
28+
$document->y->z = &$document->y;
29+
fromPHP($document);
30+
}, 'MongoDB\Driver\Exception\UnexpectedValueException'), "\n";
31+
32+
?>
33+
===DONE===
34+
<?php exit(0); ?>
35+
--EXPECTF--
36+
Testing packed array with circular reference
37+
OK: Got MongoDB\Driver\Exception\UnexpectedValueException
38+
Detected recursion for fieldname "0"
39+
40+
Testing associative array with circular reference
41+
OK: Got MongoDB\Driver\Exception\UnexpectedValueException
42+
Detected recursion for fieldname "z"
43+
44+
Testing object with circular reference
45+
OK: Got MongoDB\Driver\Exception\UnexpectedValueException
46+
Detected recursion for fieldname "z"
47+
===DONE===
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
--TEST--
2+
MongoDB\BSON\fromPHP(): Serializable with circular references
3+
--FILE--
4+
<?php
5+
6+
require_once __DIR__ . '/../utils/tools.php';
7+
8+
class MyRecursiveSerializable implements MongoDB\BSON\Serializable
9+
{
10+
public $x = 1;
11+
12+
public function bsonSerialize()
13+
{
14+
return $this;
15+
}
16+
}
17+
18+
class MyIndirectlyRecursiveSerializable extends MyRecursiveSerializable
19+
{
20+
public function bsonSerialize()
21+
{
22+
return ['x' => $this];
23+
}
24+
}
25+
26+
echo "\nTesting Serializable with direct circular reference\n";
27+
28+
echo throws(function() {
29+
fromPHP(new MyRecursiveSerializable);
30+
}, 'MongoDB\Driver\Exception\UnexpectedValueException'), "\n";
31+
32+
echo "\nTesting Serializable with indirect circular reference\n";
33+
34+
echo throws(function() {
35+
fromPHP(new MyIndirectlyRecursiveSerializable);
36+
}, 'MongoDB\Driver\Exception\UnexpectedValueException'), "\n";
37+
38+
?>
39+
===DONE===
40+
<?php exit(0); ?>
41+
--EXPECT--
42+
Testing Serializable with direct circular reference
43+
OK: Got MongoDB\Driver\Exception\UnexpectedValueException
44+
Expected MyRecursiveSerializable::bsonSerialize() to return an array or stdClass, MyRecursiveSerializable given
45+
46+
Testing Serializable with indirect circular reference
47+
OK: Got MongoDB\Driver\Exception\UnexpectedValueException
48+
Detected recursion for fieldname "x"
49+
===DONE===

0 commit comments

Comments
 (0)