Merge branch 'PHP-5.4'

* PHP-5.4:
  Fixed bug #61527 (Recursive/ArrayIterator gives misleading notice when array empty or moved to the end)

2 files changed, 126 insertions, 55 deletions

diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c
index 84977c4b9b..e2ea17a3fe 100755
--- 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
index 0000000000..ab13c69547
--- /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==