From e1b4cabbd6d5ce72bb73ec4988b91c1e62c71335 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Tue, 16 Apr 2019 10:20:19 +0200 Subject: [PATCH 1/3] 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. --- Zend/zend_hash.c | 60 ++++++++----------------------------- ext/spl/tests/bug77903.phpt | 52 ++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 47 deletions(-) create mode 100644 ext/spl/tests/bug77903.phpt diff --git a/Zend/zend_hash.c b/Zend/zend_hash.c index 57dfd7a9641..5568b23af98 100644 --- a/Zend/zend_hash.c +++ b/Zend/zend_hash.c @@ -377,10 +377,8 @@ ZEND_API uint32_t zend_array_count(HashTable *ht) } /* }}} */ -static zend_always_inline HashPosition _zend_hash_get_first_pos(const HashTable *ht) +static zend_always_inline HashPosition _zend_hash_get_valid_pos(const HashTable *ht, HashPosition pos) { - HashPosition pos = 0; - while (pos < ht->nNumUsed && Z_ISUNDEF(ht->arData[pos].val)) { pos++; } @@ -389,12 +387,7 @@ static zend_always_inline HashPosition _zend_hash_get_first_pos(const HashTable static zend_always_inline HashPosition _zend_hash_get_current_pos(const HashTable *ht) { - HashPosition pos = ht->nInternalPointer; - - if (pos == 0) { - pos = _zend_hash_get_first_pos(ht); - } - return pos; + return _zend_hash_get_valid_pos(ht, ht->nInternalPointer); } 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 { IS_CONSISTENT(ht); HT_ASSERT(ht, &ht->nInternalPointer != pos || GC_REFCOUNT(ht) == 1); - *pos = _zend_hash_get_first_pos(ht); + *pos = _zend_hash_get_valid_pos(ht, 0); } @@ -2217,19 +2210,13 @@ ZEND_API void ZEND_FASTCALL zend_hash_internal_pointer_end_ex(HashTable *ht, Has ZEND_API int ZEND_FASTCALL zend_hash_move_forward_ex(HashTable *ht, HashPosition *pos) { - uint32_t idx = *pos; + uint32_t idx; IS_CONSISTENT(ht); HT_ASSERT(ht, &ht->nInternalPointer != pos || GC_REFCOUNT(ht) == 1); + idx = _zend_hash_get_valid_pos(ht, *pos); if (idx < ht->nNumUsed) { - if (idx == 0) { - idx = _zend_hash_get_first_pos(ht); - if (idx >= ht->nNumUsed) { - *pos = idx; - return SUCCESS; - } - } while (1) { idx++; if (idx >= ht->nNumUsed) { @@ -2272,17 +2259,12 @@ ZEND_API int ZEND_FASTCALL zend_hash_move_backwards_ex(HashTable *ht, HashPositi /* This function should be made binary safe */ ZEND_API int ZEND_FASTCALL zend_hash_get_current_key_ex(const HashTable *ht, zend_string **str_index, zend_ulong *num_index, HashPosition *pos) { - uint32_t idx = *pos; + uint32_t idx; Bucket *p; IS_CONSISTENT(ht); + idx = _zend_hash_get_valid_pos(ht, *pos); if (idx < ht->nNumUsed) { - if (idx == 0) { - idx = _zend_hash_get_first_pos(ht); - if (idx >= ht->nNumUsed) { - return HASH_KEY_NON_EXISTENT; - } - } p = ht->arData + idx; if (p->key) { *str_index = p->key; @@ -2297,20 +2279,14 @@ ZEND_API int ZEND_FASTCALL zend_hash_get_current_key_ex(const HashTable *ht, zen ZEND_API void ZEND_FASTCALL zend_hash_get_current_key_zval_ex(const HashTable *ht, zval *key, HashPosition *pos) { - uint32_t idx = *pos; + uint32_t idx; Bucket *p; IS_CONSISTENT(ht); + idx = _zend_hash_get_valid_pos(ht, *pos); if (idx >= ht->nNumUsed) { ZVAL_NULL(key); } else { - if (idx == 0) { - idx = _zend_hash_get_first_pos(ht); - if (idx >= ht->nNumUsed) { - ZVAL_NULL(key); - return; - } - } p = ht->arData + idx; if (p->key) { 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 ZEND_API int ZEND_FASTCALL zend_hash_get_current_key_type_ex(HashTable *ht, HashPosition *pos) { - uint32_t idx = *pos; + uint32_t idx; Bucket *p; IS_CONSISTENT(ht); + idx = _zend_hash_get_valid_pos(ht, *pos); if (idx < ht->nNumUsed) { - if (idx == 0) { - idx = _zend_hash_get_first_pos(ht); - if (idx >= ht->nNumUsed) { - return HASH_KEY_NON_EXISTENT; - } - } p = ht->arData + idx; if (p->key) { return HASH_KEY_IS_STRING; @@ -2346,17 +2317,12 @@ ZEND_API int ZEND_FASTCALL zend_hash_get_current_key_type_ex(HashTable *ht, Hash ZEND_API zval* ZEND_FASTCALL zend_hash_get_current_data_ex(HashTable *ht, HashPosition *pos) { - uint32_t idx = *pos; + uint32_t idx; Bucket *p; IS_CONSISTENT(ht); + idx = _zend_hash_get_valid_pos(ht, *pos); if (idx < ht->nNumUsed) { - if (idx == 0) { - idx = _zend_hash_get_first_pos(ht); - if (idx >= ht->nNumUsed) { - return NULL; - } - } p = ht->arData + idx; return &p->val; } else { diff --git a/ext/spl/tests/bug77903.phpt b/ext/spl/tests/bug77903.phpt new file mode 100644 index 00000000000..eb1c579b8fc --- /dev/null +++ b/ext/spl/tests/bug77903.phpt @@ -0,0 +1,52 @@ +--TEST-- +Bug #77903: ArrayIterator stops iterating after offsetSet call +--FILE-- +rewind(); +var_dump($a->valid()); // false +var_dump($a->current()); // null +$a->offsetSet(1,1); +var_dump($a->valid()); // true +var_dump($a->current()); // 1 +$a->next(); +var_dump($a->valid()); // false +var_dump($a->current()); // null +$a->offsetSet(4,4); +var_dump($a->valid()); // true +var_dump($a->current()); // 4 +$a->next(); +var_dump($a->valid()); // false +var_dump($a->current()); // null +$a->next(); +var_dump($a->valid()); // false +var_dump($a->current()); // null +$a->offsetSet(2,2); +var_dump($a->valid()); // should: true; got: false +var_dump($a->current()); // should: 2; got: null +$a->next(); +var_dump($a->valid()); // false +var_dump($a->current()); // null +$a->next(); +var_dump($a->valid()); // false +var_dump($a->current()); // null +?> +--EXPECT-- +bool(false) +NULL +bool(true) +int(1) +bool(false) +NULL +bool(true) +int(4) +bool(false) +NULL +bool(false) +NULL +bool(false) +NULL +bool(false) +NULL +bool(false) +NULL From 9a9eed472b05292f8e8aa82129ea5d1da4b0e0c2 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Tue, 16 Apr 2019 10:38:20 +0200 Subject: [PATCH 2/3] Fix second part of bug #77903 When a HT iterator is one past the end and we rehash, we need to make sure that it is move to the new one past the end position, to make sure that newly inserted elements are picked up. --- NEWS | 4 ++++ .../tests/foreach_by_ref_repacking_insert.phpt | 18 ++++++++++++++++++ Zend/zend_hash.c | 7 +++++++ ext/spl/tests/bug77903.phpt | 8 ++++---- 4 files changed, 33 insertions(+), 4 deletions(-) create mode 100644 Zend/tests/foreach_by_ref_repacking_insert.phpt diff --git a/NEWS b/NEWS index 58df7fb7841..eef56ef2aa2 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,10 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? ????, PHP 7.3.5 +- Core: + . Fixed bug #77903 (ArrayIterator stops iterating after offsetSet call). + (Nikita) + - CLI: . Fixed bug #77794 (Incorrect Date header format in built-in server). (kelunik) diff --git a/Zend/tests/foreach_by_ref_repacking_insert.phpt b/Zend/tests/foreach_by_ref_repacking_insert.phpt new file mode 100644 index 00000000000..1a2f9c7068b --- /dev/null +++ b/Zend/tests/foreach_by_ref_repacking_insert.phpt @@ -0,0 +1,18 @@ +--TEST-- +Perform a packed to hash insert when the iterator is at the end of the array +--FILE-- + + &$v) { + var_dump($v); + if ($k == 1) $a[4] = 4; + if ($k == 4) $a[2] = 2; +} + +?> +--EXPECT-- +int(1) +int(4) +int(2) diff --git a/Zend/zend_hash.c b/Zend/zend_hash.c index 5568b23af98..e26ebd75947 100644 --- a/Zend/zend_hash.c +++ b/Zend/zend_hash.c @@ -1070,6 +1070,7 @@ ZEND_API int ZEND_FASTCALL zend_hash_rehash(HashTable *ht) p++; } while (++i < ht->nNumUsed); } else { + uint32_t old_num_used = ht->nNumUsed; do { if (UNEXPECTED(Z_TYPE(p->val) == IS_UNDEF)) { uint32_t j = i; @@ -1126,6 +1127,12 @@ ZEND_API int ZEND_FASTCALL zend_hash_rehash(HashTable *ht) HT_HASH(ht, nIndex) = HT_IDX_TO_HASH(i); p++; } while (++i < ht->nNumUsed); + + /* Migrate pointer to one past the end of the array to the new one past the end, so that + * newly inserted elements are picked up correctly. */ + if (UNEXPECTED(HT_HAS_ITERATORS(ht))) { + _zend_hash_iterators_update(ht, old_num_used, ht->nNumUsed); + } } return SUCCESS; } diff --git a/ext/spl/tests/bug77903.phpt b/ext/spl/tests/bug77903.phpt index eb1c579b8fc..842de9cca2e 100644 --- a/ext/spl/tests/bug77903.phpt +++ b/ext/spl/tests/bug77903.phpt @@ -22,8 +22,8 @@ $a->next(); var_dump($a->valid()); // false var_dump($a->current()); // null $a->offsetSet(2,2); -var_dump($a->valid()); // should: true; got: false -var_dump($a->current()); // should: 2; got: null +var_dump($a->valid()); // true +var_dump($a->current()); // 2 $a->next(); var_dump($a->valid()); // false var_dump($a->current()); // null @@ -44,8 +44,8 @@ bool(false) NULL bool(false) NULL -bool(false) -NULL +bool(true) +int(2) bool(false) NULL bool(false) From 9a85a944d8ed79a9f006873be5c5f762e037ccbf Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Mon, 15 Apr 2019 17:14:11 +0200 Subject: [PATCH 3/3] s/mysql_connect()/mysqli_connect() in php.ini --- php.ini-development | 4 ++-- php.ini-production | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/php.ini-development b/php.ini-development index 5883743e53f..1ba130de349 100644 --- a/php.ini-development +++ b/php.ini-development @@ -1186,11 +1186,11 @@ mysqli.default_port = 3306 ; http://php.net/mysqli.default-socket mysqli.default_socket = -; Default host for mysql_connect() (doesn't apply in safe mode). +; Default host for mysqli_connect() (doesn't apply in safe mode). ; http://php.net/mysqli.default-host mysqli.default_host = -; Default user for mysql_connect() (doesn't apply in safe mode). +; Default user for mysqli_connect() (doesn't apply in safe mode). ; http://php.net/mysqli.default-user mysqli.default_user = diff --git a/php.ini-production b/php.ini-production index 5ae76c810a0..55afa561b43 100644 --- a/php.ini-production +++ b/php.ini-production @@ -1193,11 +1193,11 @@ mysqli.default_port = 3306 ; http://php.net/mysqli.default-socket mysqli.default_socket = -; Default host for mysql_connect() (doesn't apply in safe mode). +; Default host for mysql_iconnect() (doesn't apply in safe mode). ; http://php.net/mysqli.default-host mysqli.default_host = -; Default user for mysql_connect() (doesn't apply in safe mode). +; Default user for mysql_iconnect() (doesn't apply in safe mode). ; http://php.net/mysqli.default-user mysqli.default_user =