Skip to content

Commit 24eb9bc

Browse files
alcaeusalcaeus
authored
PHPC-1748 Fix wrong return value of Cursor::key when iterator is not valid (#1192)
* PHPC-1748 Fix wrong return value of Cursor::key when iterator is not valid * Inline single-use iterator handlers
1 parent 70a097e commit 24eb9bc

File tree

2 files changed

+109
-95
lines changed

2 files changed

+109
-95
lines changed

‎src/MongoDB/Cursor.c‎

Lines changed: 66 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -52,96 +52,6 @@ static void php_phongo_cursor_free_current(php_phongo_cursor_t* cursor) /* {{{ *
5252
}
5353
} /* }}} */
5454

55-
/* {{{ MongoDB\Driver\Cursor iterator handlers */
56-
static int php_phongo_cursor_valid(php_phongo_cursor_t* cursor) /* {{{ */
57-
{
58-
if (!Z_ISUNDEF(cursor->visitor_data.zchild)) {
59-
return SUCCESS;
60-
}
61-
62-
return FAILURE;
63-
} /* }}} */
64-
65-
static void php_phongo_cursor_get_current_key(php_phongo_cursor_t* cursor, zval* key) /* {{{ */
66-
{
67-
ZVAL_LONG(key, cursor->current);
68-
} /* }}} */
69-
70-
static zval* php_phongo_cursor_get_current_data(php_phongo_cursor_t* cursor) /* {{{ */
71-
{
72-
return &cursor->visitor_data.zchild;
73-
} /* }}} */
74-
75-
static void php_phongo_cursor_move_forward(php_phongo_cursor_t* cursor) /* {{{ */
76-
{
77-
const bson_t* doc;
78-
79-
php_phongo_cursor_free_current(cursor);
80-
81-
/* If the cursor has already advanced, increment its position. Otherwise,
82-
* the first call to mongoc_cursor_next() will be made below and we should
83-
* leave its position at zero. */
84-
if (cursor->advanced) {
85-
cursor->current++;
86-
} else {
87-
cursor->advanced = true;
88-
}
89-
90-
if (mongoc_cursor_next(cursor->cursor, &doc)) {
91-
if (!php_phongo_bson_to_zval_ex(bson_get_data(doc), doc->len, &cursor->visitor_data)) {
92-
/* Free invalid result, but don't return as we want to free the
93-
* session if the cursor is exhausted. */
94-
php_phongo_cursor_free_current(cursor);
95-
}
96-
} else {
97-
bson_error_t error = { 0 };
98-
const bson_t* doc = NULL;
99-
100-
if (mongoc_cursor_error_document(cursor->cursor, &error, &doc)) {
101-
/* Intentionally not destroying the cursor as it will happen
102-
* naturally now that there are no more results */
103-
phongo_throw_exception_from_bson_error_t_and_reply(&error, doc);
104-
}
105-
}
106-
107-
php_phongo_cursor_free_session_if_exhausted(cursor);
108-
} /* }}} */
109-
110-
static void php_phongo_cursor_rewind(php_phongo_cursor_t* cursor) /* {{{ */
111-
{
112-
const bson_t* doc;
113-
114-
/* If the cursor was never advanced (e.g. command cursor), do so now */
115-
if (!cursor->advanced) {
116-
cursor->advanced = true;
117-
118-
if (!phongo_cursor_advance_and_check_for_error(cursor->cursor)) {
119-
/* Exception should already have been thrown */
120-
return;
121-
}
122-
}
123-
124-
if (cursor->current > 0) {
125-
phongo_throw_exception(PHONGO_ERROR_LOGIC, "Cursors cannot rewind after starting iteration");
126-
return;
127-
}
128-
129-
php_phongo_cursor_free_current(cursor);
130-
131-
doc = mongoc_cursor_current(cursor->cursor);
132-
133-
if (doc) {
134-
if (!php_phongo_bson_to_zval_ex(bson_get_data(doc), doc->len, &cursor->visitor_data)) {
135-
/* Free invalid result, but don't return as we want to free the
136-
* session if the cursor is exhausted. */
137-
php_phongo_cursor_free_current(cursor);
138-
}
139-
}
140-
141-
php_phongo_cursor_free_session_if_exhausted(cursor);
142-
} /* }}} */
143-
/* }}} */
144-
14555
/* {{{ proto void MongoDB\Driver\Cursor::setTypeMap(array $typemap)
14656
Sets a type map to use for BSON unserialization */
14757
static PHP_METHOD(Cursor, setTypeMap)
@@ -309,7 +219,7 @@ PHP_METHOD(Cursor, current)
309219
}
310220
zend_restore_error_handling(&error_handling);
311221

312-
data = php_phongo_cursor_get_current_data(intern);
222+
data = &intern->visitor_data.zchild;
313223

314224
if (Z_ISUNDEF_P(data)) {
315225
RETURN_NULL();
@@ -330,13 +240,18 @@ PHP_METHOD(Cursor, key)
330240
}
331241
zend_restore_error_handling(&error_handling);
332242

333-
php_phongo_cursor_get_current_key(intern, return_value);
243+
if (Z_ISUNDEF(intern->visitor_data.zchild)) {
244+
RETURN_NULL();
245+
}
246+
247+
RETURN_LONG(intern->current);
334248
}
335249

336250
PHP_METHOD(Cursor, next)
337251
{
338252
zend_error_handling error_handling;
339253
php_phongo_cursor_t* intern = Z_CURSOR_OBJ_P(getThis());
254+
const bson_t* doc;
340255

341256
zend_replace_error_handling(EH_THROW, phongo_exception_from_phongo_domain(PHONGO_ERROR_INVALID_ARGUMENT), &error_handling);
342257
if (zend_parse_parameters_none() == FAILURE) {
@@ -345,7 +260,35 @@ PHP_METHOD(Cursor, next)
345260
}
346261
zend_restore_error_handling(&error_handling);
347262

348-
php_phongo_cursor_move_forward(intern);
263+
php_phongo_cursor_free_current(intern);
264+
265+
/* If the intern has already advanced, increment its position. Otherwise,
266+
* the first call to mongoc_cursor_next() will be made below and we should
267+
* leave its position at zero. */
268+
if (intern->advanced) {
269+
intern->current++;
270+
} else {
271+
intern->advanced = true;
272+
}
273+
274+
if (mongoc_cursor_next(intern->cursor, &doc)) {
275+
if (!php_phongo_bson_to_zval_ex(bson_get_data(doc), doc->len, &intern->visitor_data)) {
276+
/* Free invalid result, but don't return as we want to free the
277+
* session if the intern is exhausted. */
278+
php_phongo_cursor_free_current(intern);
279+
}
280+
} else {
281+
bson_error_t error = { 0 };
282+
const bson_t* doc = NULL;
283+
284+
if (mongoc_cursor_error_document(intern->cursor, &error, &doc)) {
285+
/* Intentionally not destroying the intern as it will happen
286+
* naturally now that there are no more results */
287+
phongo_throw_exception_from_bson_error_t_and_reply(&error, doc);
288+
}
289+
}
290+
291+
php_phongo_cursor_free_session_if_exhausted(intern);
349292
}
350293

351294
PHP_METHOD(Cursor, valid)
@@ -360,13 +303,14 @@ PHP_METHOD(Cursor, valid)
360303
}
361304
zend_restore_error_handling(&error_handling);
362305

363-
RETURN_BOOL(php_phongo_cursor_valid(intern) == SUCCESS);
306+
RETURN_BOOL(!Z_ISUNDEF(intern->visitor_data.zchild));
364307
}
365308

366309
PHP_METHOD(Cursor, rewind)
367310
{
368311
zend_error_handling error_handling;
369312
php_phongo_cursor_t* intern = Z_CURSOR_OBJ_P(getThis());
313+
const bson_t* doc;
370314

371315
zend_replace_error_handling(EH_THROW, phongo_exception_from_phongo_domain(PHONGO_ERROR_INVALID_ARGUMENT), &error_handling);
372316
if (zend_parse_parameters_none() == FAILURE) {
@@ -375,7 +319,34 @@ PHP_METHOD(Cursor, rewind)
375319
}
376320
zend_restore_error_handling(&error_handling);
377321

378-
php_phongo_cursor_rewind(intern);
322+
/* If the intern was never advanced (e.g. command intern), do so now */
323+
if (!intern->advanced) {
324+
intern->advanced = true;
325+
326+
if (!phongo_cursor_advance_and_check_for_error(intern->cursor)) {
327+
/* Exception should already have been thrown */
328+
return;
329+
}
330+
}
331+
332+
if (intern->current > 0) {
333+
phongo_throw_exception(PHONGO_ERROR_LOGIC, "Cursors cannot rewind after starting iteration");
334+
return;
335+
}
336+
337+
php_phongo_cursor_free_current(intern);
338+
339+
doc = mongoc_cursor_current(intern->cursor);
340+
341+
if (doc) {
342+
if (!php_phongo_bson_to_zval_ex(bson_get_data(doc), doc->len, &intern->visitor_data)) {
343+
/* Free invalid result, but don't return as we want to free the
344+
* session if the intern is exhausted. */
345+
php_phongo_cursor_free_current(intern);
346+
}
347+
}
348+
349+
php_phongo_cursor_free_session_if_exhausted(intern);
379350
}
380351

381352
/* {{{ MongoDB\Driver\Cursor function entries */

‎tests/cursor/cursor-iterator-003.phpt‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
--TEST--
2+
MongoDB\Driver\Cursor handles invalid positions gracefully
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+
require_once __DIR__ . "/../utils/basic.inc";
10+
11+
$manager = new MongoDB\Driver\Manager(URI);
12+
13+
$bulkWrite = new MongoDB\Driver\BulkWrite;
14+
$bulkWrite->insert(array('_id' => 0));
15+
16+
$writeResult = $manager->executeBulkWrite(NS, $bulkWrite);
17+
18+
$cursor = $manager->executeQuery(NS, new MongoDB\Driver\Query(array()));
19+
20+
$cursor->rewind();
21+
var_dump($cursor->valid());
22+
var_dump($cursor->key());
23+
var_dump($cursor->current());
24+
25+
$cursor->next();
26+
var_dump($cursor->valid());
27+
var_dump($cursor->key());
28+
var_dump($cursor->current());
29+
30+
?>
31+
===DONE===
32+
<?php exit(0); ?>
33+
--EXPECTF--
34+
bool(true)
35+
int(0)
36+
object(stdClass)#%d (1) {
37+
["_id"]=>
38+
int(0)
39+
}
40+
bool(false)
41+
NULL
42+
NULL
43+
===DONE===

0 commit comments

Comments
 (0)