Skip to content

Commit d6bef26

Browse files
jmikolaalcaeus
jmikola
and
alcaeus
authored
PHPC-2210 and PHPC-2211: Fix direct BSON copy and toJson memory leak (#1415)
* PHPC-2211: Free intermediary result in php_phongo_bson_to_json() This was responsible for several memory leaks in MongoDB\BSON\Document's JSON output methods. * PHPC-2210: Fix direct copying of Document and PackedArray BSON bson_copy_to() cannot be used with a bson_t originally allocated with bson_new(), since there is no way to revert the bson_t to an uninitialized state. The previous code with bson_destroy() resulted in invalid reads/writes (reported by Valgrind) and could cause "malloc(): unaligned tcache chunk detected" aborts later in the process during a future, unrelated call to bson_copy_to(). With bson_destroy() removed, the malloc() aborts are avoided but Valgrind reports a leak for the originally allocation by bson_new(). The solution applied here borrows logic from bson_copy_to_excluding_noinit() to do a more flexible copy. bson_copy_to_excluding_noinit() could not be used directly because it requires at least one exclusion field name. * Run clang-format Co-authored-by: Andreas Braun <[email protected]>
1 parent 7a0359d commit d6bef26

File tree

8 files changed

+232
-4
lines changed

8 files changed

+232
-4
lines changed

‎src/phongo_bson.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1438,6 +1438,7 @@ bool php_phongo_bson_to_json(zval* return_value, const bson_t* bson, php_phongo_
14381438
}
14391439

14401440
ZVAL_STRINGL(return_value, json, json_len);
1441+
bson_free(json);
14411442

14421443
return true;
14431444
}

‎src/phongo_bson_encode.c

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,24 @@ static void php_phongo_bson_append(bson_t* bson, php_phongo_field_path* field_pa
381381
}
382382
}
383383

384+
/* This is based on bson_copy_to_excluding_noinit() and is necessary because
385+
* bson_copy_to() cannot be used with a bson_t allocated with bson_new(). */
386+
static void phongo_bson_copy_to_noinit(const bson_t* src, bson_t* dst)
387+
{
388+
bson_iter_t iter;
389+
390+
if (bson_iter_init(&iter, src)) {
391+
while (bson_iter_next(&iter)) {
392+
if (!bson_append_iter(dst, NULL, 0, &iter)) {
393+
/* This should not be able to happen since we are copying from
394+
* within a valid bson_t. */
395+
phongo_throw_exception(PHONGO_ERROR_UNEXPECTED_VALUE, "Error copying \"%s\" field from source document", bson_iter_key(&iter));
396+
return;
397+
}
398+
}
399+
}
400+
}
401+
384402
static void php_phongo_zval_to_bson_internal(zval* data, php_phongo_field_path* field_path, php_phongo_bson_flags_t flags, bson_t* bson, bson_t** bson_out)
385403
{
386404
HashTable* ht_data = NULL;
@@ -403,17 +421,15 @@ static void php_phongo_zval_to_bson_internal(zval* data, php_phongo_field_path*
403421
if (instanceof_function(Z_OBJCE_P(data), php_phongo_document_ce)) {
404422
php_phongo_document_t* intern = Z_DOCUMENT_OBJ_P(data);
405423

406-
bson_destroy(bson);
407-
bson_copy_to(intern->bson, bson);
424+
phongo_bson_copy_to_noinit(intern->bson, bson);
408425

409426
goto done;
410427
}
411428

412429
if (instanceof_function(Z_OBJCE_P(data), php_phongo_packedarray_ce)) {
413430
php_phongo_packedarray_t* intern = Z_PACKEDARRAY_OBJ_P(data);
414431

415-
bson_destroy(bson);
416-
bson_copy_to(intern->bson, bson);
432+
phongo_bson_copy_to_noinit(intern->bson, bson);
417433

418434
goto done;
419435
}

‎tests/bson/bson-document-fromPHP-004.phpt

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
--TEST--
2+
MongoDB\BSON\Document::fromPHP() copies BSON data from Document and PackedArray
3+
--FILE--
4+
<?php
5+
6+
require_once __DIR__ . '/../utils/basic.inc';
7+
8+
$document = MongoDB\BSON\Document::fromJSON('{ "foo": "bar" }');
9+
$fromDocument = MongoDB\BSON\Document::fromPHP($document);
10+
echo $fromDocument->toRelaxedExtendedJSON(), "\n";
11+
12+
// This will be interpreted as an object after copying, i.e. { "0": 1, "1": 2, "2": 3 }
13+
$packedArray = MongoDB\BSON\PackedArray::fromPHP([ 1, 2, 3 ]);
14+
$fromPackedArray = MongoDB\BSON\Document::fromPHP($packedArray);
15+
echo $fromPackedArray->toRelaxedExtendedJSON(), "\n";
16+
17+
?>
18+
===DONE===
19+
<?php exit(0); ?>
20+
--EXPECT--
21+
{ "foo" : "bar" }
22+
{ "0" : 1, "1" : 2, "2" : 3 }
23+
===DONE===

‎tests/bulk/bulkwrite-delete-003.phpt

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
--TEST--
2+
MongoDB\Driver\BulkWrite::delete() $filter is MongoDB\BSON\Document
3+
--SKIPIF--
4+
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
5+
<?php skip_if_not_live(); ?>
6+
<?php skip_if_not_clean(); ?>
7+
--FILE--
8+
<?php
9+
10+
require_once __DIR__ . "/../utils/basic.inc";
11+
12+
$manager = create_test_manager();
13+
14+
$bulk = new MongoDB\Driver\BulkWrite();
15+
$bulk->insert(['_id' => 1]);
16+
$bulk->insert(['_id' => 2]);
17+
$manager->executeBulkWrite(NS, $bulk);
18+
19+
$filter = MongoDB\BSON\Document::fromJSON('{ "_id": { "$gt": 1 } }');
20+
21+
$bulk = new MongoDB\Driver\BulkWrite;
22+
$bulk->delete($filter);
23+
$result = $manager->executeBulkWrite(NS, $bulk);
24+
25+
var_dump($result->getDeletedCount());
26+
27+
?>
28+
===DONE===
29+
<?php exit(0); ?>
30+
--EXPECT--
31+
int(1)
32+
===DONE===

‎tests/bulk/bulkwrite-insert-002.phpt

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
--TEST--
2+
MongoDB\Driver\BulkWrite::insert() $document is MongoDB\BSON\Document
3+
--SKIPIF--
4+
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
5+
<?php skip_if_not_live(); ?>
6+
<?php skip_if_not_clean(); ?>
7+
--FILE--
8+
<?php
9+
10+
require_once __DIR__ . "/../utils/basic.inc";
11+
12+
$manager = create_test_manager();
13+
14+
$document = MongoDB\BSON\Document::fromJSON('{ "_id": 2 }');
15+
16+
$bulk = new MongoDB\Driver\BulkWrite();
17+
$insertedId = $bulk->insert($document);
18+
$result = $manager->executeBulkWrite(NS, $bulk);
19+
20+
var_dump($insertedId);
21+
var_dump($result->getInsertedCount());
22+
23+
?>
24+
===DONE===
25+
<?php exit(0); ?>
26+
--EXPECT--
27+
int(2)
28+
int(1)
29+
===DONE===

‎tests/bulk/bulkwrite-update-005.phpt

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
--TEST--
2+
MongoDB\Driver\BulkWrite::update() $filter and $newObj are MongoDB\BSON\Document
3+
--SKIPIF--
4+
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
5+
<?php skip_if_not_live(); ?>
6+
<?php skip_if_not_clean(); ?>
7+
--FILE--
8+
<?php
9+
10+
require_once __DIR__ . "/../utils/basic.inc";
11+
12+
$manager = create_test_manager();
13+
14+
$bulk = new MongoDB\Driver\BulkWrite();
15+
$bulk->insert(['_id' => 1]);
16+
$bulk->insert(['_id' => 2]);
17+
$manager->executeBulkWrite(NS, $bulk);
18+
19+
$filter = MongoDB\BSON\Document::fromJSON('{ "_id": { "$gt": 1 } }');
20+
$newObj = MongoDB\BSON\Document::fromJSON('{ "$set": { "x": 1 } }');
21+
22+
$bulk = new MongoDB\Driver\BulkWrite;
23+
$bulk->update($filter, $newObj);
24+
$result = $manager->executeBulkWrite(NS, $bulk);
25+
26+
var_dump($result->getMatchedCount());
27+
var_dump($result->getModifiedCount());
28+
29+
?>
30+
===DONE===
31+
<?php exit(0); ?>
32+
--EXPECT--
33+
int(1)
34+
int(1)
35+
===DONE===

‎tests/command/command-ctor-002.phpt

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
--TEST--
2+
MongoDB\Driver\Command::__construct() $document is MongoDB\Driver\Document
3+
--SKIPIF--
4+
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
5+
<?php skip_if_not_live(); ?>
6+
<?php skip_if_not_clean(); ?>
7+
--FILE--
8+
<?php
9+
10+
require_once __DIR__ . "/../utils/basic.inc";
11+
12+
$manager = create_test_manager();
13+
14+
$document = MongoDB\BSON\Document::fromJSON('{ "ping": 1 }');
15+
$command = new MongoDB\Driver\Command($document);
16+
17+
var_dump($command);
18+
19+
$cursor = $manager->executeCommand(DATABASE_NAME, $command);
20+
21+
var_dump($cursor->toArray()[0]);
22+
23+
?>
24+
===DONE===
25+
<?php exit(0); ?>
26+
--EXPECTF--
27+
object(MongoDB\Driver\Command)#%d (%d) {
28+
["command"]=>
29+
object(stdClass)#%d (%d) {
30+
["ping"]=>
31+
int(1)
32+
}
33+
}
34+
object(stdClass)#%d (%d) {
35+
["ok"]=>
36+
float(1)%A
37+
}
38+
===DONE===

‎tests/query/query-ctor-007.phpt

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
--TEST--
2+
MongoDB\Driver\Query::__construct() $filter is MongoDB\Driver\Document
3+
--SKIPIF--
4+
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
5+
<?php skip_if_not_live(); ?>
6+
<?php skip_if_not_clean(); ?>
7+
--FILE--
8+
<?php
9+
10+
require_once __DIR__ . "/../utils/basic.inc";
11+
12+
$manager = create_test_manager();
13+
14+
$bulk = new MongoDB\Driver\BulkWrite;
15+
$bulk->insert(['_id' => 1]);
16+
$bulk->insert(['_id' => 2]);
17+
$manager->executeBulkWrite(NS, $bulk);
18+
19+
$filter = MongoDB\BSON\Document::fromJSON('{ "_id": { "$gt": 1 } }');
20+
$query = new MongoDB\Driver\Query($filter);
21+
22+
var_dump($query);
23+
24+
$cursor = $manager->executeQuery(NS, $query);
25+
26+
var_dump($cursor->toArray());
27+
28+
?>
29+
===DONE===
30+
<?php exit(0); ?>
31+
--EXPECTF--
32+
object(MongoDB\Driver\Query)#%d (%d) {
33+
["filter"]=>
34+
object(stdClass)#%d (%d) {
35+
["_id"]=>
36+
object(stdClass)#%d (%d) {
37+
["$gt"]=>
38+
int(1)
39+
}
40+
}
41+
["options"]=>
42+
object(stdClass)#%d (%d) {
43+
}
44+
["readConcern"]=>
45+
NULL
46+
}
47+
array(1) {
48+
[0]=>
49+
object(stdClass)#%d (%d) {
50+
["_id"]=>
51+
int(2)
52+
}
53+
}
54+
===DONE===

0 commit comments

Comments
 (0)