Skip to content

Commit

Permalink
Fixed bug #61527 (Recursive/ArrayIterator gives misleading notice whe…
Browse files Browse the repository at this point in the history 
…n array empty or moved to the end)
  • Loading branch information
@smalyshev
smalyshev committed Jul 15, 2012
1 parent cfdccdb commit a5d45ba
Show file tree
Hide file tree
Showing 2 changed files with 126 additions and 55 deletions.
89 changes: 34 additions & 55 deletions ext/spl/spl_array.c
View file
Expand Up @@ -650,6 +650,28 @@ static int spl_array_has_dimension(zval *object, zval *offset, int check_empty T
return spl_array_has_dimension_ex(1, object, offset, check_empty TSRMLS_CC);
} /* }}} */

/* {{{ spl_array_object_verify_pos_ex */
static inline int spl_array_object_verify_pos_ex(spl_array_object *object, HashTable *ht, const char *msg_prefix TSRMLS_DC)
{
if (!ht) {
php_error_docref(NULL TSRMLS_CC, E_NOTICE, "%sArray was modified outside object and is no longer an array", msg_prefix);
return FAILURE;
}

if (object->pos && (object->ar_flags & SPL_ARRAY_IS_REF) && spl_hash_verify_pos_ex(object, ht TSRMLS_CC) == FAILURE) {
php_error_docref(NULL TSRMLS_CC, E_NOTICE, "%sArray was modified outside object and internal position is no longer valid", msg_prefix);
return FAILURE;
}

return SUCCESS;
} /* }}} */

/* {{{ spl_array_object_verify_pos */
static inline int spl_array_object_verify_pos(spl_array_object *object, HashTable *ht TSRMLS_DC)
{
return spl_array_object_verify_pos_ex(object, ht, "" TSRMLS_CC);
} /* }}} */

/* {{{ proto bool ArrayObject::offsetExists(mixed $index)
proto bool ArrayIterator::offsetExists(mixed $index)
Returns whether the requested $index exists. */
Expand Down Expand Up @@ -963,17 +985,11 @@ static int spl_array_it_valid(zend_object_iterator *iter TSRMLS_DC) /* {{{ */
if (object->ar_flags & SPL_ARRAY_OVERLOADED_VALID) {
return zend_user_it_valid(iter TSRMLS_CC);
} else {
if (!aht) {
php_error_docref(NULL TSRMLS_CC, E_NOTICE, "ArrayIterator::valid(): Array was modified outside object and is no longer an array");
return FAILURE;
}

if (object->pos && (object->ar_flags & SPL_ARRAY_IS_REF) && spl_hash_verify_pos_ex(object, aht TSRMLS_CC) == FAILURE) {
php_error_docref(NULL TSRMLS_CC, E_NOTICE, "ArrayIterator::valid(): Array was modified outside object and internal position is no longer valid");
if (spl_array_object_verify_pos_ex(object, aht, "ArrayIterator::valid(): " TSRMLS_CC) == FAILURE) {
return FAILURE;
} else {
return zend_hash_has_more_elements_ex(aht, &object->pos);
}

return zend_hash_has_more_elements_ex(aht, &object->pos);
}
}
/* }}} */
Expand Down Expand Up @@ -1003,13 +1019,7 @@ static int spl_array_it_get_current_key(zend_object_iterator *iter, char **str_k
if (object->ar_flags & SPL_ARRAY_OVERLOADED_KEY) {
return zend_user_it_get_current_key(iter, str_key, str_key_len, int_key TSRMLS_CC);
} else {
if (!aht) {
php_error_docref(NULL TSRMLS_CC, E_NOTICE, "ArrayIterator::current(): Array was modified outside object and is no longer an array");
return HASH_KEY_NON_EXISTANT;
}

if ((object->ar_flags & SPL_ARRAY_IS_REF) && spl_hash_verify_pos_ex(object, aht TSRMLS_CC) == FAILURE) {
php_error_docref(NULL TSRMLS_CC, E_NOTICE, "ArrayIterator::current(): Array was modified outside object and internal position is no longer valid");
if (spl_array_object_verify_pos_ex(object, aht, "ArrayIterator::current(): " TSRMLS_CC) == FAILURE) {
return HASH_KEY_NON_EXISTANT;
}

Expand Down Expand Up @@ -1494,13 +1504,7 @@ SPL_METHOD(Array, current)
return;
}

if (!aht) {
php_error_docref(NULL TSRMLS_CC, E_NOTICE, "Array was modified outside object and is no longer an array");
return;
}

if ((intern->ar_flags & SPL_ARRAY_IS_REF) && spl_hash_verify_pos_ex(intern, aht TSRMLS_CC) == FAILURE) {
php_error_docref(NULL TSRMLS_CC, E_NOTICE, "Array was modified outside object and internal position is no longer valid");
if (spl_array_object_verify_pos(intern, aht TSRMLS_CC) == FAILURE) {
return;
}

Expand Down Expand Up @@ -1530,13 +1534,7 @@ void spl_array_iterator_key(zval *object, zval *return_value TSRMLS_DC) /* {{{ *
ulong num_key;
HashTable *aht = spl_array_get_hash_table(intern, 0 TSRMLS_CC);

if (!aht) {
php_error_docref(NULL TSRMLS_CC, E_NOTICE, "Array was modified outside object and is no longer an array");
return;
}

if ((intern->ar_flags & SPL_ARRAY_IS_REF) && spl_hash_verify_pos_ex(intern, aht TSRMLS_CC) == FAILURE) {
php_error_docref(NULL TSRMLS_CC, E_NOTICE, "Array was modified outside object and internal position is no longer valid");
if (spl_array_object_verify_pos(intern, aht TSRMLS_CC) == FAILURE) {
return;
}

Expand Down Expand Up @@ -1564,13 +1562,12 @@ SPL_METHOD(Array, next)
if (zend_parse_parameters_none() == FAILURE) {
return;
}

if (!aht) {
php_error_docref(NULL TSRMLS_CC, E_NOTICE, "Array was modified outside object and is no longer an array");

if (spl_array_object_verify_pos(intern, aht TSRMLS_CC) == FAILURE) {
return;
}

spl_array_next_ex(intern, aht TSRMLS_CC);
spl_array_next_no_verify(intern, aht TSRMLS_CC);
}
/* }}} */

Expand All @@ -1585,14 +1582,8 @@ SPL_METHOD(Array, valid)
if (zend_parse_parameters_none() == FAILURE) {
return;
}

if (!aht) {
php_error_docref(NULL TSRMLS_CC, E_NOTICE, "Array was modified outside object and is no longer an array");
return;
}

if (intern->pos && (intern->ar_flags & SPL_ARRAY_IS_REF) && spl_hash_verify_pos_ex(intern, aht TSRMLS_CC) == FAILURE) {
php_error_docref(NULL TSRMLS_CC, E_NOTICE, "Array was modified outside object and internal position is no longer valid");
if (spl_array_object_verify_pos(intern, aht TSRMLS_CC) == FAILURE) {
RETURN_FALSE;
} else {
RETURN_BOOL(zend_hash_has_more_elements_ex(aht, &intern->pos) == SUCCESS);
Expand All @@ -1611,14 +1602,8 @@ SPL_METHOD(Array, hasChildren)
if (zend_parse_parameters_none() == FAILURE) {
return;
}

if (!aht) {
php_error_docref(NULL TSRMLS_CC, E_NOTICE, "Array was modified outside object and is no longer an array");
RETURN_FALSE;
}

if ((intern->ar_flags & SPL_ARRAY_IS_REF) && spl_hash_verify_pos_ex(intern, aht TSRMLS_CC) == FAILURE) {
php_error_docref(NULL TSRMLS_CC, E_NOTICE, "Array was modified outside object and internal position is no longer valid");
if (spl_array_object_verify_pos(intern, aht TSRMLS_CC) == FAILURE) {
RETURN_FALSE;
}

Expand All @@ -1642,13 +1627,7 @@ SPL_METHOD(Array, getChildren)
return;
}

if (!aht) {
php_error_docref(NULL TSRMLS_CC, E_NOTICE, "Array was modified outside object and is no longer an array");
return;
}

if ((intern->ar_flags & SPL_ARRAY_IS_REF) && spl_hash_verify_pos_ex(intern, aht TSRMLS_CC) == FAILURE) {
php_error_docref(NULL TSRMLS_CC, E_NOTICE, "Array was modified outside object and internal position is no longer valid");
if (spl_array_object_verify_pos(intern, aht TSRMLS_CC) == FAILURE) {
return;
}

Expand Down
92 changes: 92 additions & 0 deletions ext/spl/tests/bug61527.phpt
View file
@@ -0,0 +1,92 @@
--TEST--
Bug #61527 (Recursive/ArrayIterator gives misleading notice when array empty or moved to the end)
--FILE--
<?php
$ao = new ArrayObject(array());
$ai = $ao->getIterator();

/* testing empty array, should no notice at all */
$ai->next();
var_dump($ai->key());
var_dump($ai->current());

/* testing array changing */
$ao2 = new ArrayObject(array(1 => 1, 2, 3, 4, 5));
$ai2 = $ao2->getIterator();

$ao2->offsetUnset($ai2->key());
$ai2->next();

/* now point to 2 */
$ao2->offsetUnset($ai2->key());
var_dump($ai2->key());

/* now point to 3 */
$ao2->offsetUnset($ai2->key());
var_dump($ai2->current());

$ai2->next();
var_dump($ai2->key());
var_dump($ai2->current());

/* should be at the end and no notice */
$ai2->next();
var_dump($ai2->key());
var_dump($ai2->current());

$ai2->rewind();
$ai2->next();
$ai2->next();
/* should reached the end */
var_dump($ai2->next());
var_dump($ai2->key());

/* testing RecursiveArrayIterator */
$ao3 = new ArrayObject(array(), NULL, 'RecursiveArrayIterator');
$ai3 = $ao3->getIterator();

var_dump($ai3->getChildren());

$ao4 = new ArrayObject(array(1, 2), NULL, 'RecursiveArrayIterator');
$ai4 = $ao4->getIterator();

$ai4->next();
$ai4->next();
$ai4->next();
var_dump($ai4->hasChildren());

$ai4->rewind();
$ao4->offsetUnset($ai4->key());
var_dump($ai4->hasChildren());

$ao4->offsetUnset($ai4->key());
var_dump($ai4->getChildren());
?>
==DONE==
<?php exit(0); ?>
--EXPECTF--
NULL
NULL

Notice: ArrayIterator::next(): Array was modified outside object and internal position is no longer valid in %sbug61527.php on line %d

Notice: ArrayIterator::key(): Array was modified outside object and internal position is no longer valid in %sbug61527.php on line %d
NULL

Notice: ArrayIterator::current(): Array was modified outside object and internal position is no longer valid in %sbug61527.php on line %d
NULL
int(5)
int(5)
NULL
NULL
NULL
NULL
NULL
bool(false)

Notice: RecursiveArrayIterator::hasChildren(): Array was modified outside object and internal position is no longer valid in %sbug61527.php on line %d
bool(false)

Notice: RecursiveArrayIterator::getChildren(): Array was modified outside object and internal position is no longer valid in %sbug61527.php on line %d
NULL
==DONE==

0 comments on commit a5d45ba

Please sign in to comment.