Skip to content

Commit e1b4cab

Browse files
nikicnikic
committed
Partial fix for bug #77903
In the hash position APIs, make sure we always advance to the next non-undef element and not just when the position is 0 (similar to what foreach does). This can happen when the position of an ArrayIterator is one past its current end and a new element is inserted not directly at that position because the array is packed. There is still a bug here (as shown in the tests), but this is a separate issue that also affects plain array iteration in foreach.
1 parent a2f3ec1 commit e1b4cab

File tree

2 files changed

+65
-47
lines changed

2 files changed

+65
-47
lines changed

‎Zend/zend_hash.c‎

Lines changed: 13 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -377,10 +377,8 @@ ZEND_API uint32_t zend_array_count(HashTable *ht)
377377
}
378378
/* }}} */
379379

380-
static zend_always_inline HashPosition _zend_hash_get_first_pos(const HashTable *ht)
380+
static zend_always_inline HashPosition _zend_hash_get_valid_pos(const HashTable *ht, HashPosition pos)
381381
{
382-
HashPosition pos = 0;
383-
384382
while (pos < ht->nNumUsed && Z_ISUNDEF(ht->arData[pos].val)) {
385383
pos++;
386384
}
@@ -389,12 +387,7 @@ static zend_always_inline HashPosition _zend_hash_get_first_pos(const HashTable
389387

390388
static zend_always_inline HashPosition _zend_hash_get_current_pos(const HashTable *ht)
391389
{
392-
HashPosition pos = ht->nInternalPointer;
393-
394-
if (pos == 0) {
395-
pos = _zend_hash_get_first_pos(ht);
396-
}
397-
return pos;
390+
return _zend_hash_get_valid_pos(ht, ht->nInternalPointer);
398391
}
399392

400393
ZEND_API HashPosition ZEND_FASTCALL zend_hash_get_current_pos(const HashTable *ht)
@@ -2189,7 +2182,7 @@ ZEND_API void ZEND_FASTCALL zend_hash_internal_pointer_reset_ex(HashTable *ht, H
21892182
{
21902183
IS_CONSISTENT(ht);
21912184
HT_ASSERT(ht, &ht->nInternalPointer != pos || GC_REFCOUNT(ht) == 1);
2192-
*pos = _zend_hash_get_first_pos(ht);
2185+
*pos = _zend_hash_get_valid_pos(ht, 0);
21932186
}
21942187

21952188

@@ -2217,19 +2210,13 @@ ZEND_API void ZEND_FASTCALL zend_hash_internal_pointer_end_ex(HashTable *ht, Has
22172210

22182211
ZEND_API int ZEND_FASTCALL zend_hash_move_forward_ex(HashTable *ht, HashPosition *pos)
22192212
{
2220-
uint32_t idx = *pos;
2213+
uint32_t idx;
22212214

22222215
IS_CONSISTENT(ht);
22232216
HT_ASSERT(ht, &ht->nInternalPointer != pos || GC_REFCOUNT(ht) == 1);
22242217

2218+
idx = _zend_hash_get_valid_pos(ht, *pos);
22252219
if (idx < ht->nNumUsed) {
2226-
if (idx == 0) {
2227-
idx = _zend_hash_get_first_pos(ht);
2228-
if (idx >= ht->nNumUsed) {
2229-
*pos = idx;
2230-
return SUCCESS;
2231-
}
2232-
}
22332220
while (1) {
22342221
idx++;
22352222
if (idx >= ht->nNumUsed) {
@@ -2272,17 +2259,12 @@ ZEND_API int ZEND_FASTCALL zend_hash_move_backwards_ex(HashTable *ht, HashPositi
22722259
/* This function should be made binary safe */
22732260
ZEND_API int ZEND_FASTCALL zend_hash_get_current_key_ex(const HashTable *ht, zend_string **str_index, zend_ulong *num_index, HashPosition *pos)
22742261
{
2275-
uint32_t idx = *pos;
2262+
uint32_t idx;
22762263
Bucket *p;
22772264

22782265
IS_CONSISTENT(ht);
2266+
idx = _zend_hash_get_valid_pos(ht, *pos);
22792267
if (idx < ht->nNumUsed) {
2280-
if (idx == 0) {
2281-
idx = _zend_hash_get_first_pos(ht);
2282-
if (idx >= ht->nNumUsed) {
2283-
return HASH_KEY_NON_EXISTENT;
2284-
}
2285-
}
22862268
p = ht->arData + idx;
22872269
if (p->key) {
22882270
*str_index = p->key;
@@ -2297,20 +2279,14 @@ ZEND_API int ZEND_FASTCALL zend_hash_get_current_key_ex(const HashTable *ht, zen
22972279

22982280
ZEND_API void ZEND_FASTCALL zend_hash_get_current_key_zval_ex(const HashTable *ht, zval *key, HashPosition *pos)
22992281
{
2300-
uint32_t idx = *pos;
2282+
uint32_t idx;
23012283
Bucket *p;
23022284

23032285
IS_CONSISTENT(ht);
2286+
idx = _zend_hash_get_valid_pos(ht, *pos);
23042287
if (idx >= ht->nNumUsed) {
23052288
ZVAL_NULL(key);
23062289
} else {
2307-
if (idx == 0) {
2308-
idx = _zend_hash_get_first_pos(ht);
2309-
if (idx >= ht->nNumUsed) {
2310-
ZVAL_NULL(key);
2311-
return;
2312-
}
2313-
}
23142290
p = ht->arData + idx;
23152291
if (p->key) {
23162292
ZVAL_STR_COPY(key, p->key);
@@ -2322,17 +2298,12 @@ ZEND_API void ZEND_FASTCALL zend_hash_get_current_key_zval_ex(const HashTable *h
23222298

23232299
ZEND_API int ZEND_FASTCALL zend_hash_get_current_key_type_ex(HashTable *ht, HashPosition *pos)
23242300
{
2325-
uint32_t idx = *pos;
2301+
uint32_t idx;
23262302
Bucket *p;
23272303

23282304
IS_CONSISTENT(ht);
2305+
idx = _zend_hash_get_valid_pos(ht, *pos);
23292306
if (idx < ht->nNumUsed) {
2330-
if (idx == 0) {
2331-
idx = _zend_hash_get_first_pos(ht);
2332-
if (idx >= ht->nNumUsed) {
2333-
return HASH_KEY_NON_EXISTENT;
2334-
}
2335-
}
23362307
p = ht->arData + idx;
23372308
if (p->key) {
23382309
return HASH_KEY_IS_STRING;
@@ -2346,17 +2317,12 @@ ZEND_API int ZEND_FASTCALL zend_hash_get_current_key_type_ex(HashTable *ht, Hash
23462317

23472318
ZEND_API zval* ZEND_FASTCALL zend_hash_get_current_data_ex(HashTable *ht, HashPosition *pos)
23482319
{
2349-
uint32_t idx = *pos;
2320+
uint32_t idx;
23502321
Bucket *p;
23512322

23522323
IS_CONSISTENT(ht);
2324+
idx = _zend_hash_get_valid_pos(ht, *pos);
23532325
if (idx < ht->nNumUsed) {
2354-
if (idx == 0) {
2355-
idx = _zend_hash_get_first_pos(ht);
2356-
if (idx >= ht->nNumUsed) {
2357-
return NULL;
2358-
}
2359-
}
23602326
p = ht->arData + idx;
23612327
return &p->val;
23622328
} else {

‎ext/spl/tests/bug77903.phpt‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
--TEST--
2+
Bug #77903: ArrayIterator stops iterating after offsetSet call
3+
--FILE--
4+
<?php
5+
$a = new ArrayIterator();
6+
$a->rewind();
7+
var_dump($a->valid()); // false
8+
var_dump($a->current()); // null
9+
$a->offsetSet(1,1);
10+
var_dump($a->valid()); // true
11+
var_dump($a->current()); // 1
12+
$a->next();
13+
var_dump($a->valid()); // false
14+
var_dump($a->current()); // null
15+
$a->offsetSet(4,4);
16+
var_dump($a->valid()); // true
17+
var_dump($a->current()); // 4
18+
$a->next();
19+
var_dump($a->valid()); // false
20+
var_dump($a->current()); // null
21+
$a->next();
22+
var_dump($a->valid()); // false
23+
var_dump($a->current()); // null
24+
$a->offsetSet(2,2);
25+
var_dump($a->valid()); // should: true; got: false
26+
var_dump($a->current()); // should: 2; got: null
27+
$a->next();
28+
var_dump($a->valid()); // false
29+
var_dump($a->current()); // null
30+
$a->next();
31+
var_dump($a->valid()); // false
32+
var_dump($a->current()); // null
33+
?>
34+
--EXPECT--
35+
bool(false)
36+
NULL
37+
bool(true)
38+
int(1)
39+
bool(false)
40+
NULL
41+
bool(true)
42+
int(4)
43+
bool(false)
44+
NULL
45+
bool(false)
46+
NULL
47+
bool(false)
48+
NULL
49+
bool(false)
50+
NULL
51+
bool(false)
52+
NULL

0 commit comments

Comments
 (0)