Fixed bug #61527 (Recursive/ArrayIterator gives misleading notice when array empty...

diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c
index 84977c4b9bea6a8a39decbd0f470636a04c08e22..e2ea17a3fe3f89f39f5939f1464422c2f80cc1fd 100755 (executable)
--- a/ext/spl/spl_array.c
+++ b/ext/spl/spl_array.c
@@ -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. */
@@ -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);
        }
 }
 /* }}} */
@@ -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;
                }
        
@@ -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;
        }
 
@@ -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;
        }
 
@@ -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);
 }
 /* }}} */ 
 
@@ -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);
@@ -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;
        }
 
@@ -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;
        }
 

diff --git a/ext/spl/tests/bug61527.phpt b/ext/spl/tests/bug61527.phpt
new file mode 100644 (file)
index 0000000..ab13c69
--- /dev/null
+++ b/ext/spl/tests/bug61527.phpt
@@ -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==