diff --git a/NEWS b/NEWS index 4e95bed34c84..2300def47290 100644 --- a/NEWS +++ b/NEWS @@ -32,6 +32,9 @@ PHP NEWS . Fixed bug GH-22818 (stream_filter_register() orphaned user_filter_map on shutdown re-registration). (David Carlier) +- SPL: + . Fixed bug GH-11591 (AppendIterator fails with an empty generator). + (Matthias Görgens) 30 Jul 2026, PHP 8.6.0alpha3 - Core: @@ -519,7 +522,6 @@ PHP NEWS . pwhash argument-validation errors now throw ValueError instead of SodiumException. (iliaal) -- SPL: . DirectoryIterator key can now work better with filesystem supporting larger directory indexing. (David Carlier) . Fixed bug GH-21831 (SplObjectStorage::removeAllExcept() use-after-free with diff --git a/ext/spl/spl_iterators.c b/ext/spl/spl_iterators.c index 06d344990dfa..f4d31eed971d 100644 --- a/ext/spl/spl_iterators.c +++ b/ext/spl/spl_iterators.c @@ -18,7 +18,9 @@ #include "php.h" #include "zend_exceptions.h" +#include "zend_generators.h" #include "zend_interfaces.h" +#include "zend_weakrefs.h" #include "ext/pcre/php_pcre.h" #include "spl_iterators.h" @@ -120,6 +122,7 @@ typedef struct _spl_dual_it_object { struct { zval zarrayit; zend_object_iterator *iterator; + HashTable *empty_generators; } append; struct { zend_long flags; @@ -2024,6 +2027,11 @@ static void spl_dual_it_free_storage(zend_object *_object) if (object->dit_type == DIT_AppendIterator) { zend_iterator_dtor(object->u.append.iterator); + if (object->u.append.empty_generators) { + zend_weakrefs_hash_destroy(object->u.append.empty_generators); + FREE_HASHTABLE(object->u.append.empty_generators); + object->u.append.empty_generators = NULL; + } if (Z_TYPE(object->u.append.zarrayit) != IS_UNDEF) { zval_ptr_dtor(&object->u.append.zarrayit); } @@ -2783,6 +2791,13 @@ static zend_result spl_append_it_next_iterator(spl_dual_it_object *intern) /* {{ it = intern->u.append.iterator->funcs->get_current_data(intern->u.append.iterator); ZVAL_COPY(&intern->inner.zobject, it); intern->inner.ce = Z_OBJCE_P(it); + if (intern->u.append.empty_generators) { + zval *dummy = zend_hash_index_find(intern->u.append.empty_generators, + zend_object_to_weakref_key(Z_OBJ_P(it))); + if (dummy) { + return SUCCESS; + } + } intern->inner.iterator = intern->inner.ce->get_iterator(intern->inner.ce, it, 0); spl_dual_it_rewind(intern); return SUCCESS; @@ -2791,9 +2806,43 @@ static zend_result spl_append_it_next_iterator(spl_dual_it_object *intern) /* {{ } } /* }}} */ +static void spl_append_it_record_empty_generator(spl_dual_it_object *intern) /* {{{ */ +{ + if (EG(exception)) { + return; + } + + if (!intern->u.append.empty_generators) { + ALLOC_HASHTABLE(intern->u.append.empty_generators); + zend_hash_init(intern->u.append.empty_generators, 0, NULL, ZVAL_PTR_DTOR, 0); + } + + zval *dummy = zend_hash_index_find(intern->u.append.empty_generators, + zend_object_to_weakref_key(Z_OBJ(intern->inner.zobject))); + if (!dummy) { + zval empty_zval; + ZVAL_NULL(&empty_zval); + dummy = zend_weakrefs_hash_add(intern->u.append.empty_generators, + Z_OBJ(intern->inner.zobject), &empty_zval); + ZEND_ASSERT(dummy != NULL); + } +} /* }}} */ + static void spl_append_it_fetch(spl_dual_it_object *intern) /* {{{*/ { - while (spl_dual_it_valid(intern) != SUCCESS) { + while (true) { + if (spl_dual_it_valid(intern) == SUCCESS) { + break; + } + if (intern->inner.ce == zend_ce_generator) { + zend_generator *generator = (zend_generator *) Z_OBJ(intern->inner.zobject); + /* A generator that finished while initialising never yielded, so it is + * empty and can be skipped. One that yielded was consumed and still + * reports "Cannot rewind a generator that was already run". */ + if (generator->flags & ZEND_GENERATOR_AT_FIRST_YIELD) { + spl_append_it_record_empty_generator(intern); + } + } intern->u.append.iterator->funcs->move_forward(intern->u.append.iterator); if (spl_append_it_next_iterator(intern) != SUCCESS) { return; @@ -2824,6 +2873,7 @@ PHP_METHOD(AppendIterator, __construct) } intern->dit_type = DIT_AppendIterator; + intern->u.append.empty_generators = NULL; object_init_with_constructor(&intern->u.append.zarrayit, spl_ce_ArrayIterator, 0, NULL, NULL); intern->u.append.iterator = spl_ce_ArrayIterator->get_iterator(spl_ce_ArrayIterator, &intern->u.append.zarrayit, 0); diff --git a/ext/spl/tests/gh11591.phpt b/ext/spl/tests/gh11591.phpt new file mode 100644 index 000000000000..7a81f5bb594c --- /dev/null +++ b/ext/spl/tests/gh11591.phpt @@ -0,0 +1,51 @@ +--TEST-- +GH-11591 (AppendIterator with generators that yield nothing) +--FILE-- +append(emptyGenerator()); +values($iterator); +values($iterator); + +echo "leading\n"; +$iterator = new AppendIterator(); +$iterator->append(emptyGenerator()); +$iterator->append(new ArrayIterator(['A'])); +values($iterator); +values($iterator); + +echo "distinct\n"; +$iterator = new AppendIterator(); +$iterator->append(emptyGenerator()); +$iterator->append(emptyGenerator()); +values($iterator); +?> +--EXPECT-- +sole +array(0) { +} +array(0) { +} +leading +array(1) { + [0]=> + string(1) "A" +} +array(1) { + [0]=> + string(1) "A" +} +distinct +array(0) { +} diff --git a/ext/spl/tests/gh11591_2.phpt b/ext/spl/tests/gh11591_2.phpt new file mode 100644 index 000000000000..5d1630d62cd9 --- /dev/null +++ b/ext/spl/tests/gh11591_2.phpt @@ -0,0 +1,174 @@ +--TEST-- +GH-11591 (AppendIterator preserves other iterator semantics) +--FILE-- +getMessage(), "\n"; + } +} + +echo "duplicate\n"; +$generator = emptyGenerator(); +$iterator = new AppendIterator(); +$iterator->append($generator); +$iterator->append($generator); +values($iterator); + +echo "replacement\n"; +$iterator = new AppendIterator(); +$iterator->append(emptyGenerator()); +$iterator->getArrayIterator()[0] = new ArrayIterator(['R']); +values($iterator); + +echo "ordinary\n"; +$ordinary = new class implements Iterator { + public int $rewinds = 0; + public function rewind(): void { $this->rewinds++; } + public function valid(): bool { return false; } + public function current(): mixed { return null; } + public function key(): mixed { return null; } + public function next(): void {} +}; +$iterator = new AppendIterator(); +$iterator->append($ordinary); +echo $ordinary->rewinds, "\n"; +values($iterator); +echo $ordinary->rewinds, "\n"; +values($iterator); +echo $ordinary->rewinds, "\n"; + +echo "nonempty\n"; +$generator = (function () { yield 'G'; })(); +$iterator = new AppendIterator(); +$iterator->append($generator); +values($iterator); +values($iterator); + +echo "moved\n"; +$generator = emptyGenerator(); +$iterator = new AppendIterator(); +$iterator->append($generator); +$array = $iterator->getArrayIterator(); +unset($array[0]); +$array[] = $generator; +values($iterator); + +echo "append during traversal\n"; +$iterator = new AppendIterator(); +$iterator->append(new ArrayIterator(['A'])); +$added = false; +foreach ($iterator as $value) { + echo $value; + if (!$added) { + $iterator->append(emptyGenerator()); + $iterator->append(new ArrayIterator(['B'])); + $added = true; + } +} +echo "\n"; + +echo "externally closed\n"; +$generator = emptyGenerator(); +$generator->valid(); +$iterator = new AppendIterator(); +try { + $iterator->append($generator); +} catch (Throwable $e) { + echo $e->getMessage(), "\n"; +} + +echo "weak lifetime\n"; +$generator = emptyGenerator(); +$weak = WeakReference::create($generator); +$iterator = new AppendIterator(); +$iterator->append($generator); +unset($iterator->getArrayIterator()[0], $generator); +gc_collect_cycles(); +var_dump($weak->get() === null); + +echo "reentrant unset\n"; +$iterator = new AppendIterator(); +$generator = (function () use (&$iterator): Generator { + unset($iterator->getArrayIterator()[0]); + if (false) { + yield; + } +})(); +$iterator->append($generator); +values($iterator); + +echo "reentrant move\n"; +$iterator = new AppendIterator(); +$generator = (function () use (&$iterator, &$generator): Generator { + $array = $iterator->getArrayIterator(); + unset($array[0]); + $array[1] = $generator; + if (false) { + yield; + } +})(); +$iterator->append($generator); +values($iterator); + +echo "throwing\n"; +$iterator = new AppendIterator(); +try { + $iterator->append((function (): Generator { + if (false) { + yield; + } + throw new RuntimeException('boom'); + })()); +} catch (Throwable $e) { + echo $e->getMessage(), "\n"; +} +?> +--EXPECT-- +duplicate +array(0) { +} +replacement +array(1) { + [0]=> + string(1) "R" +} +ordinary +1 +array(0) { +} +2 +array(0) { +} +3 +nonempty +array(1) { + [0]=> + string(1) "G" +} +Cannot traverse an already closed generator +moved +array(0) { +} +append during traversal +AB +externally closed +Cannot traverse an already closed generator +weak lifetime +bool(true) +reentrant unset +array(0) { +} +reentrant move +array(0) { +} +throwing +boom diff --git a/ext/spl/tests/gh11591_3.phpt b/ext/spl/tests/gh11591_3.phpt new file mode 100644 index 000000000000..732710b75d7f --- /dev/null +++ b/ext/spl/tests/gh11591_3.phpt @@ -0,0 +1,89 @@ +--TEST-- +GH-11591 (AppendIterator skips probed-empty generators after array mutations) +--FILE-- +getMessage(), "\n"; + } +} + +echo "reinsert same key\n"; +$generator = emptyGenerator(); +$iterator = new AppendIterator(); +$iterator->append($generator); +$outer = $iterator->getArrayIterator(); +$outer->offsetUnset(0); +$outer->offsetSet(0, $generator); +values($iterator); + +echo "reinsert other key\n"; +$generator = emptyGenerator(); +$iterator = new AppendIterator(); +$iterator->append($generator); +$outer = $iterator->getArrayIterator(); +$outer->offsetUnset(0); +$outer->offsetSet(1, $generator); +values($iterator); + +echo "replace with non-empty generator\n"; +$generator = emptyGenerator(); +$iterator = new AppendIterator(); +$iterator->append($generator); +$outer = $iterator->getArrayIterator(); +$outer->offsetUnset(0); +$outer->offsetSet(0, (function () { yield 'NEW'; })()); +values($iterator); + +echo "empty appended after valid entry\n"; +$iterator = new AppendIterator(); +$iterator->append(new ArrayIterator(['A'])); +$iterator->append(emptyGenerator()); +values($iterator); +values($iterator); + +echo "empty generator behind wrapper\n"; +$iterator = new AppendIterator(); +$iterator->append(new IteratorIterator(emptyGenerator())); +$iterator->append(new ArrayIterator(['A'])); +values($iterator); +values($iterator); +?> +--EXPECT-- +reinsert same key +array(0) { +} +reinsert other key +array(0) { +} +replace with non-empty generator +array(1) { + [0]=> + string(3) "NEW" +} +empty appended after valid entry +array(1) { + [0]=> + string(1) "A" +} +array(1) { + [0]=> + string(1) "A" +} +empty generator behind wrapper +array(1) { + [0]=> + string(1) "A" +} +array(1) { + [0]=> + string(1) "A" +}