Skip to content

PHPC-1748 Fix wrong return value of Cursor::key when iterator is not valid #1192

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 2 commits into from
Jan 25, 2021
Merged

PHPC-1748 Fix wrong return value of Cursor::key when iterator is not valid #1192

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b3ff8b6
PHPC-1748 Fix wrong return value of Cursor::key when iterator is not …
alcaeus Jan 21, 2021
d3687d8
Inline single-use iterator handlers
alcaeus Jan 22, 2021
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
161 changes: 66 additions & 95 deletions src/MongoDB/Cursor.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,96 +52,6 @@ static void php_phongo_cursor_free_current(php_phongo_cursor_t* cursor) /* {{{ *
}
} /* }}} */

/* {{{ MongoDB\Driver\Cursor iterator handlers */
static int php_phongo_cursor_valid(php_phongo_cursor_t* cursor) /* {{{ */
{
if (!Z_ISUNDEF(cursor->visitor_data.zchild)) {
return SUCCESS;
}

return FAILURE;
} /* }}} */

static void php_phongo_cursor_get_current_key(php_phongo_cursor_t* cursor, zval* key) /* {{{ */
{
ZVAL_LONG(key, cursor->current);
} /* }}} */

static zval* php_phongo_cursor_get_current_data(php_phongo_cursor_t* cursor) /* {{{ */
{
return &cursor->visitor_data.zchild;
} /* }}} */

static void php_phongo_cursor_move_forward(php_phongo_cursor_t* cursor) /* {{{ */
{
const bson_t* doc;

php_phongo_cursor_free_current(cursor);

/* If the cursor has already advanced, increment its position. Otherwise,
* the first call to mongoc_cursor_next() will be made below and we should
* leave its position at zero. */
if (cursor->advanced) {
cursor->current++;
} else {
cursor->advanced = true;
}

if (mongoc_cursor_next(cursor->cursor, &doc)) {
if (!php_phongo_bson_to_zval_ex(bson_get_data(doc), doc->len, &cursor->visitor_data)) {
/* Free invalid result, but don't return as we want to free the
* session if the cursor is exhausted. */
php_phongo_cursor_free_current(cursor);
}
} else {
bson_error_t error = { 0 };
const bson_t* doc = NULL;

if (mongoc_cursor_error_document(cursor->cursor, &error, &doc)) {
/* Intentionally not destroying the cursor as it will happen
* naturally now that there are no more results */
phongo_throw_exception_from_bson_error_t_and_reply(&error, doc);
}
}

php_phongo_cursor_free_session_if_exhausted(cursor);
} /* }}} */

static void php_phongo_cursor_rewind(php_phongo_cursor_t* cursor) /* {{{ */
{
const bson_t* doc;

/* If the cursor was never advanced (e.g. command cursor), do so now */
if (!cursor->advanced) {
cursor->advanced = true;

if (!phongo_cursor_advance_and_check_for_error(cursor->cursor)) {
/* Exception should already have been thrown */
return;
}
}

if (cursor->current > 0) {
phongo_throw_exception(PHONGO_ERROR_LOGIC, "Cursors cannot rewind after starting iteration");
return;
}

php_phongo_cursor_free_current(cursor);

doc = mongoc_cursor_current(cursor->cursor);

if (doc) {
if (!php_phongo_bson_to_zval_ex(bson_get_data(doc), doc->len, &cursor->visitor_data)) {
/* Free invalid result, but don't return as we want to free the
* session if the cursor is exhausted. */
php_phongo_cursor_free_current(cursor);
}
}

php_phongo_cursor_free_session_if_exhausted(cursor);
} /* }}} */
/* }}} */

/* {{{ proto void MongoDB\Driver\Cursor::setTypeMap(array $typemap)
Sets a type map to use for BSON unserialization */
static PHP_METHOD(Cursor, setTypeMap)
Expand Down Expand Up @@ -309,7 +219,7 @@ PHP_METHOD(Cursor, current)
}
zend_restore_error_handling(&error_handling);

data = php_phongo_cursor_get_current_data(intern);
data = &intern->visitor_data.zchild;

if (Z_ISUNDEF_P(data)) {
RETURN_NULL();
Expand All @@ -330,13 +240,18 @@ PHP_METHOD(Cursor, key)
}
zend_restore_error_handling(&error_handling);

php_phongo_cursor_get_current_key(intern, return_value);
if (Z_ISUNDEF(intern->visitor_data.zchild)) {
RETURN_NULL();
}

RETURN_LONG(intern->current);
}

PHP_METHOD(Cursor, next)
{
zend_error_handling error_handling;
php_phongo_cursor_t* intern = Z_CURSOR_OBJ_P(getThis());
const bson_t* doc;

zend_replace_error_handling(EH_THROW, phongo_exception_from_phongo_domain(PHONGO_ERROR_INVALID_ARGUMENT), &error_handling);
if (zend_parse_parameters_none() == FAILURE) {
Expand All @@ -345,7 +260,35 @@ PHP_METHOD(Cursor, next)
}
zend_restore_error_handling(&error_handling);

php_phongo_cursor_move_forward(intern);
php_phongo_cursor_free_current(intern);

/* If the intern has already advanced, increment its position. Otherwise,
* the first call to mongoc_cursor_next() will be made below and we should
* leave its position at zero. */
if (intern->advanced) {
intern->current++;
} else {
intern->advanced = true;
}

if (mongoc_cursor_next(intern->cursor, &doc)) {
if (!php_phongo_bson_to_zval_ex(bson_get_data(doc), doc->len, &intern->visitor_data)) {
/* Free invalid result, but don't return as we want to free the
* session if the intern is exhausted. */
php_phongo_cursor_free_current(intern);
}
} else {
bson_error_t error = { 0 };
const bson_t* doc = NULL;

if (mongoc_cursor_error_document(intern->cursor, &error, &doc)) {
/* Intentionally not destroying the intern as it will happen
* naturally now that there are no more results */
phongo_throw_exception_from_bson_error_t_and_reply(&error, doc);
}
}

php_phongo_cursor_free_session_if_exhausted(intern);
}

PHP_METHOD(Cursor, valid)
Expand All @@ -360,13 +303,14 @@ PHP_METHOD(Cursor, valid)
}
zend_restore_error_handling(&error_handling);

RETURN_BOOL(php_phongo_cursor_valid(intern) == SUCCESS);
RETURN_BOOL(!Z_ISUNDEF(intern->visitor_data.zchild));
}

PHP_METHOD(Cursor, rewind)
{
zend_error_handling error_handling;
php_phongo_cursor_t* intern = Z_CURSOR_OBJ_P(getThis());
const bson_t* doc;

zend_replace_error_handling(EH_THROW, phongo_exception_from_phongo_domain(PHONGO_ERROR_INVALID_ARGUMENT), &error_handling);
if (zend_parse_parameters_none() == FAILURE) {
Expand All @@ -375,7 +319,34 @@ PHP_METHOD(Cursor, rewind)
}
zend_restore_error_handling(&error_handling);

php_phongo_cursor_rewind(intern);
/* If the intern was never advanced (e.g. command intern), do so now */
if (!intern->advanced) {
intern->advanced = true;

if (!phongo_cursor_advance_and_check_for_error(intern->cursor)) {
/* Exception should already have been thrown */
return;
}
}

if (intern->current > 0) {
phongo_throw_exception(PHONGO_ERROR_LOGIC, "Cursors cannot rewind after starting iteration");
return;
}

php_phongo_cursor_free_current(intern);

doc = mongoc_cursor_current(intern->cursor);

if (doc) {
if (!php_phongo_bson_to_zval_ex(bson_get_data(doc), doc->len, &intern->visitor_data)) {
/* Free invalid result, but don't return as we want to free the
* session if the intern is exhausted. */
php_phongo_cursor_free_current(intern);
}
}

php_phongo_cursor_free_session_if_exhausted(intern);
}

/* {{{ MongoDB\Driver\Cursor function entries */
Expand Down
43 changes: 43 additions & 0 deletions tests/cursor/cursor-iterator-003.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
--TEST--
MongoDB\Driver\Cursor handles invalid positions gracefully
--SKIPIF--
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
<?php skip_if_not_live(); ?>
<?php skip_if_not_clean(); ?>
--FILE--
<?php
require_once __DIR__ . "/../utils/basic.inc";

$manager = new MongoDB\Driver\Manager(URI);

$bulkWrite = new MongoDB\Driver\BulkWrite;
$bulkWrite->insert(array('_id' => 0));

$writeResult = $manager->executeBulkWrite(NS, $bulkWrite);

$cursor = $manager->executeQuery(NS, new MongoDB\Driver\Query(array()));

$cursor->rewind();
var_dump($cursor->valid());
var_dump($cursor->key());
var_dump($cursor->current());

$cursor->next();
var_dump($cursor->valid());
var_dump($cursor->key());
var_dump($cursor->current());

?>
===DONE===
<?php exit(0); ?>
--EXPECTF--
bool(true)
int(0)
object(stdClass)#%d (1) {
["_id"]=>
int(0)
}
bool(false)
NULL
NULL
===DONE===