Skip to content

Commit a5d45ba

Browse files
smalyshevsmalyshev
committed
Fixed bug #61527 (Recursive/ArrayIterator gives misleading notice when array empty or moved to the end)
1 parent cfdccdb commit a5d45ba

File tree

2 files changed

+126
-55
lines changed

2 files changed

+126
-55
lines changed

‎ext/spl/spl_array.c‎

Lines changed: 34 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -650,6 +650,28 @@ static int spl_array_has_dimension(zval *object, zval *offset, int check_empty T
650650
return spl_array_has_dimension_ex(1, object, offset, check_empty TSRMLS_CC);
651651
} /* }}} */
652652

653+
/* {{{ spl_array_object_verify_pos_ex */
654+
static inline int spl_array_object_verify_pos_ex(spl_array_object *object, HashTable *ht, const char *msg_prefix TSRMLS_DC)
655+
{
656+
if (!ht) {
657+
php_error_docref(NULL TSRMLS_CC, E_NOTICE, "%sArray was modified outside object and is no longer an array", msg_prefix);
658+
return FAILURE;
659+
}
660+
661+
if (object->pos && (object->ar_flags & SPL_ARRAY_IS_REF) && spl_hash_verify_pos_ex(object, ht TSRMLS_CC) == FAILURE) {
662+
php_error_docref(NULL TSRMLS_CC, E_NOTICE, "%sArray was modified outside object and internal position is no longer valid", msg_prefix);
663+
return FAILURE;
664+
}
665+
666+
return SUCCESS;
667+
} /* }}} */
668+
669+
/* {{{ spl_array_object_verify_pos */
670+
static inline int spl_array_object_verify_pos(spl_array_object *object, HashTable *ht TSRMLS_DC)
671+
{
672+
return spl_array_object_verify_pos_ex(object, ht, "" TSRMLS_CC);
673+
} /* }}} */
674+
653675
/* {{{ proto bool ArrayObject::offsetExists(mixed $index)
654676
proto bool ArrayIterator::offsetExists(mixed $index)
655677
Returns whether the requested $index exists. */
@@ -963,17 +985,11 @@ static int spl_array_it_valid(zend_object_iterator *iter TSRMLS_DC) /* {{{ */
963985
if (object->ar_flags & SPL_ARRAY_OVERLOADED_VALID) {
964986
return zend_user_it_valid(iter TSRMLS_CC);
965987
} else {
966-
if (!aht) {
967-
php_error_docref(NULL TSRMLS_CC, E_NOTICE, "ArrayIterator::valid(): Array was modified outside object and is no longer an array");
968-
return FAILURE;
969-
}
970-
971-
if (object->pos && (object->ar_flags & SPL_ARRAY_IS_REF) && spl_hash_verify_pos_ex(object, aht TSRMLS_CC) == FAILURE) {
972-
php_error_docref(NULL TSRMLS_CC, E_NOTICE, "ArrayIterator::valid(): Array was modified outside object and internal position is no longer valid");
988+
if (spl_array_object_verify_pos_ex(object, aht, "ArrayIterator::valid(): " TSRMLS_CC) == FAILURE) {
973989
return FAILURE;
974-
} else {
975-
return zend_hash_has_more_elements_ex(aht, &object->pos);
976990
}
991+
992+
return zend_hash_has_more_elements_ex(aht, &object->pos);
977993
}
978994
}
979995
/* }}} */
@@ -1003,13 +1019,7 @@ static int spl_array_it_get_current_key(zend_object_iterator *iter, char **str_k
10031019
if (object->ar_flags & SPL_ARRAY_OVERLOADED_KEY) {
10041020
return zend_user_it_get_current_key(iter, str_key, str_key_len, int_key TSRMLS_CC);
10051021
} else {
1006-
if (!aht) {
1007-
php_error_docref(NULL TSRMLS_CC, E_NOTICE, "ArrayIterator::current(): Array was modified outside object and is no longer an array");
1008-
return HASH_KEY_NON_EXISTANT;
1009-
}
1010-
1011-
if ((object->ar_flags & SPL_ARRAY_IS_REF) && spl_hash_verify_pos_ex(object, aht TSRMLS_CC) == FAILURE) {
1012-
php_error_docref(NULL TSRMLS_CC, E_NOTICE, "ArrayIterator::current(): Array was modified outside object and internal position is no longer valid");
1022+
if (spl_array_object_verify_pos_ex(object, aht, "ArrayIterator::current(): " TSRMLS_CC) == FAILURE) {
10131023
return HASH_KEY_NON_EXISTANT;
10141024
}
10151025

@@ -1494,13 +1504,7 @@ SPL_METHOD(Array, current)
14941504
return;
14951505
}
14961506

1497-
if (!aht) {
1498-
php_error_docref(NULL TSRMLS_CC, E_NOTICE, "Array was modified outside object and is no longer an array");
1499-
return;
1500-
}
1501-
1502-
if ((intern->ar_flags & SPL_ARRAY_IS_REF) && spl_hash_verify_pos_ex(intern, aht TSRMLS_CC) == FAILURE) {
1503-
php_error_docref(NULL TSRMLS_CC, E_NOTICE, "Array was modified outside object and internal position is no longer valid");
1507+
if (spl_array_object_verify_pos(intern, aht TSRMLS_CC) == FAILURE) {
15041508
return;
15051509
}
15061510

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

1533-
if (!aht) {
1534-
php_error_docref(NULL TSRMLS_CC, E_NOTICE, "Array was modified outside object and is no longer an array");
1535-
return;
1536-
}
1537-
1538-
if ((intern->ar_flags & SPL_ARRAY_IS_REF) && spl_hash_verify_pos_ex(intern, aht TSRMLS_CC) == FAILURE) {
1539-
php_error_docref(NULL TSRMLS_CC, E_NOTICE, "Array was modified outside object and internal position is no longer valid");
1537+
if (spl_array_object_verify_pos(intern, aht TSRMLS_CC) == FAILURE) {
15401538
return;
15411539
}
15421540

@@ -1564,13 +1562,12 @@ SPL_METHOD(Array, next)
15641562
if (zend_parse_parameters_none() == FAILURE) {
15651563
return;
15661564
}
1567-
1568-
if (!aht) {
1569-
php_error_docref(NULL TSRMLS_CC, E_NOTICE, "Array was modified outside object and is no longer an array");
1565+
1566+
if (spl_array_object_verify_pos(intern, aht TSRMLS_CC) == FAILURE) {
15701567
return;
15711568
}
15721569

1573-
spl_array_next_ex(intern, aht TSRMLS_CC);
1570+
spl_array_next_no_verify(intern, aht TSRMLS_CC);
15741571
}
15751572
/* }}} */
15761573

@@ -1585,14 +1582,8 @@ SPL_METHOD(Array, valid)
15851582
if (zend_parse_parameters_none() == FAILURE) {
15861583
return;
15871584
}
1588-
1589-
if (!aht) {
1590-
php_error_docref(NULL TSRMLS_CC, E_NOTICE, "Array was modified outside object and is no longer an array");
1591-
return;
1592-
}
15931585

1594-
if (intern->pos && (intern->ar_flags & SPL_ARRAY_IS_REF) && spl_hash_verify_pos_ex(intern, aht TSRMLS_CC) == FAILURE) {
1595-
php_error_docref(NULL TSRMLS_CC, E_NOTICE, "Array was modified outside object and internal position is no longer valid");
1586+
if (spl_array_object_verify_pos(intern, aht TSRMLS_CC) == FAILURE) {
15961587
RETURN_FALSE;
15971588
} else {
15981589
RETURN_BOOL(zend_hash_has_more_elements_ex(aht, &intern->pos) == SUCCESS);
@@ -1611,14 +1602,8 @@ SPL_METHOD(Array, hasChildren)
16111602
if (zend_parse_parameters_none() == FAILURE) {
16121603
return;
16131604
}
1614-
1615-
if (!aht) {
1616-
php_error_docref(NULL TSRMLS_CC, E_NOTICE, "Array was modified outside object and is no longer an array");
1617-
RETURN_FALSE;
1618-
}
16191605

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

@@ -1642,13 +1627,7 @@ SPL_METHOD(Array, getChildren)
16421627
return;
16431628
}
16441629

1645-
if (!aht) {
1646-
php_error_docref(NULL TSRMLS_CC, E_NOTICE, "Array was modified outside object and is no longer an array");
1647-
return;
1648-
}
1649-
1650-
if ((intern->ar_flags & SPL_ARRAY_IS_REF) && spl_hash_verify_pos_ex(intern, aht TSRMLS_CC) == FAILURE) {
1651-
php_error_docref(NULL TSRMLS_CC, E_NOTICE, "Array was modified outside object and internal position is no longer valid");
1630+
if (spl_array_object_verify_pos(intern, aht TSRMLS_CC) == FAILURE) {
16521631
return;
16531632
}
16541633

‎ext/spl/tests/bug61527.phpt‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
--TEST--
2+
Bug #61527 (Recursive/ArrayIterator gives misleading notice when array empty or moved to the end)
3+
--FILE--
4+
<?php
5+
$ao = new ArrayObject(array());
6+
$ai = $ao->getIterator();
7+
8+
/* testing empty array, should no notice at all */
9+
$ai->next();
10+
var_dump($ai->key());
11+
var_dump($ai->current());
12+
13+
/* testing array changing */
14+
$ao2 = new ArrayObject(array(1 => 1, 2, 3, 4, 5));
15+
$ai2 = $ao2->getIterator();
16+
17+
$ao2->offsetUnset($ai2->key());
18+
$ai2->next();
19+
20+
/* now point to 2 */
21+
$ao2->offsetUnset($ai2->key());
22+
var_dump($ai2->key());
23+
24+
/* now point to 3 */
25+
$ao2->offsetUnset($ai2->key());
26+
var_dump($ai2->current());
27+
28+
$ai2->next();
29+
var_dump($ai2->key());
30+
var_dump($ai2->current());
31+
32+
/* should be at the end and no notice */
33+
$ai2->next();
34+
var_dump($ai2->key());
35+
var_dump($ai2->current());
36+
37+
$ai2->rewind();
38+
$ai2->next();
39+
$ai2->next();
40+
/* should reached the end */
41+
var_dump($ai2->next());
42+
var_dump($ai2->key());
43+
44+
/* testing RecursiveArrayIterator */
45+
$ao3 = new ArrayObject(array(), NULL, 'RecursiveArrayIterator');
46+
$ai3 = $ao3->getIterator();
47+
48+
var_dump($ai3->getChildren());
49+
50+
$ao4 = new ArrayObject(array(1, 2), NULL, 'RecursiveArrayIterator');
51+
$ai4 = $ao4->getIterator();
52+
53+
$ai4->next();
54+
$ai4->next();
55+
$ai4->next();
56+
var_dump($ai4->hasChildren());
57+
58+
$ai4->rewind();
59+
$ao4->offsetUnset($ai4->key());
60+
var_dump($ai4->hasChildren());
61+
62+
$ao4->offsetUnset($ai4->key());
63+
var_dump($ai4->getChildren());
64+
?>
65+
==DONE==
66+
<?php exit(0); ?>
67+
--EXPECTF--
68+
NULL
69+
NULL
70+
71+
Notice: ArrayIterator::next(): Array was modified outside object and internal position is no longer valid in %sbug61527.php on line %d
72+
73+
Notice: ArrayIterator::key(): Array was modified outside object and internal position is no longer valid in %sbug61527.php on line %d
74+
NULL
75+
76+
Notice: ArrayIterator::current(): Array was modified outside object and internal position is no longer valid in %sbug61527.php on line %d
77+
NULL
78+
int(5)
79+
int(5)
80+
NULL
81+
NULL
82+
NULL
83+
NULL
84+
NULL
85+
bool(false)
86+
87+
Notice: RecursiveArrayIterator::hasChildren(): Array was modified outside object and internal position is no longer valid in %sbug61527.php on line %d
88+
bool(false)
89+
90+
Notice: RecursiveArrayIterator::getChildren(): Array was modified outside object and internal position is no longer valid in %sbug61527.php on line %d
91+
NULL
92+
==DONE==

0 commit comments

Comments
 (0)