DatePeriod::getIterator()->valid() advances DatePeriod iterator

[dg]

Description

The following code:

<?php
$start = \DateTime::createFromFormat('Y-m-d', '2021-01-01');
$end = \DateTime::createFromFormat('Y-m-d', '2021-01-10');
$period = new \DatePeriod($start, \DateInterval::createFromDateString('1 day'), $end);
$iterator = new \CachingIterator($period->getIterator(), 0);

foreach ($iterator as $item) {
	echo $item->format('c'), "\n";
	$iterator->hasNext(); // advances iterator
	$iterator->hasNext(); // advances iterator
}

It returns:

2021-01-01T07:56:05+01:00
2021-01-04T07:56:05+01:00
2021-01-07T07:56:05+01:00
...

But it should return

2021-01-01T07:54:37+01:00
2021-01-02T07:54:37+01:00
2021-01-03T07:54:37+01:00
...

PHP Version

PHP 8.0-8.1

Operating System

No response

[drupol]

Same issue in PHP 8.1.1

[cmb69]

Yeah, that looks wrong: https://3v4l.org/sNs51. This appears to be related to https://bugs.php.net/50981, but contrary to that issue, this really looks like an implementation bug. Anyhow, use ::isValid() as workaround.

[aragon999]

For a short weekend distraction I looked a bit around in the source code which seems to be responsible for this bug.

It seems that this bug only occurs with InternalIterators. For example the following code (from a comment in the PHP docs) seems to not be affected by this issue:
https://3v4l.org/P6icu

Since the CachingIterator checks that the element of the internal iterator (which is one step ahead in the actual iteration) is valid, I then looked into the code which is responsible for the validation in the case of an InternalIterator:

php-src/Zend/zend_interfaces.c

Lines 587 to 600 in 024d5f4

	ZEND_METHOD(InternalIterator, valid) {
	ZEND_PARSE_PARAMETERS_NONE();
	zend_internal_iterator *intern = zend_internal_iterator_fetch(ZEND_THIS);
	if (!intern) {
	RETURN_THROWS();
	}
	if (zend_internal_iterator_ensure_rewound(intern) == FAILURE) {
	RETURN_THROWS();
	}
	RETURN_BOOL(intern->iter->funcs->valid(intern->iter) == SUCCESS);
	}

In particular the zend_internal_iterator_ensure_rewound function

php-src/Zend/zend_interfaces.c

Lines 514 to 526 in 024d5f4

	static zend_result zend_internal_iterator_ensure_rewound(zend_internal_iterator *intern) {
	if (!intern->rewind_called) {
	zend_object_iterator *iter = intern->iter;
	intern->rewind_called = 1;
	if (iter->funcs->rewind) {
	iter->funcs->rewind(iter);
	if (UNEXPECTED(EG(exception))) {
	return FAILURE;
	}
	}
	}
	return SUCCESS;
	}

seems to be responsible for the faulty behavior described here.

Unfortunately I have no idea how to fix this problem, since I lack understanding how it should be addressed.

[TysonAndre]

<?php

$start = \DateTime::createFromFormat('Y-m-d', '2021-01-01');
$end = \DateTime::createFromFormat('Y-m-d', '2021-01-10');
$period = new \DatePeriod($start, \DateInterval::createFromDateString('1 day'), $end);
$iterator = $period->getIterator();

foreach ($iterator as $item) {
	echo $item->format('c'), "\n";
	$iterator->valid(); // advances iterator
	$iterator->valid(); // advances iterator
}

This looks like an issue with DatePeriod::getIterator, not CachingIterator. https://3v4l.org/Er6ej

[TysonAndre]

Specifically, it's caused by /* apply modification if it's not the first iteration */ in ext/date/php_date.c

1. It shouldn't be modifying state (or not modifying more than once?) in valid()
2. It should be modifying state in rewind, construction, and move_forward, I believe? Not familiar with this extension's implementation details.

// ext/date/php_date.c
/* iterator handler table */
static const zend_object_iterator_funcs date_period_it_funcs = {
	date_period_it_dtor,
	date_period_it_has_more,
// ...


/* {{{ date_period_it_has_more */
static int date_period_it_has_more(zend_object_iterator *iter)
{
// ext/date/php_date.c
	date_period_it *iterator = (date_period_it *)iter;
	php_period_obj *object   = Z_PHPPERIOD_P(&iterator->intern.data);
	timelib_time   *it_time = object->current;

	/* apply modification if it's not the first iteration */
	if (!object->include_start_date || iterator->current_index > 0) {
		it_time->have_relative = 1;
		it_time->relative = *object->interval;
		it_time->sse_uptodate = 0;
		timelib_update_ts(it_time, NULL);
		timelib_update_from_sse(it_time);
	}

	if (object->end) {
		return object->current->sse < object->end->sse ? SUCCESS : FAILURE;
	} else {
		return (iterator->current_index < object->recurrences) ? SUCCESS : FAILURE;
	}
}
/* }}} */