Fixed bug #61527 (Recursive/ArrayIterator gives misleading notice when array empty or moved to the end)

[reeze]

Hi,

when calling
- ArrayIterator->next(),
- ArrayIterator->key(),
- ArrayIterator->current(),
- RecurseArrayIterator->getChildren()
- RecurseArrayIterator->hasChildren()
it will raise misleading(array was modified outside) message. when array was empty or moved to then end of array.

This pull request is trying to fix it.

Thanks.

[nikic]

This probably should be ht, as that's what you're passing into the macro.

[reeze]

update: error messsage updated & if else branch cleanup

[reeze]

Thanks for reply, I will updated it soon;)

[dsp]

btw are you sure the bug id is correct? The bug you refer to is closed and has little to do with your change.

[smalyshev]

Which bug you are trying to fix? Bug #61347 is "inconsistent isset behavior of Arrayobject" and already fixed.

[smalyshev]

There should be no messages when array is iterated to an end - it is perfectly normal thing to do. Empty array is a perfectly normal thing too, there should be no messages on that either. Also, I don't see why array should be rewinded. If it's at the end, it should just stay there.

[reeze]

sorry ,my fault it's Bug#61527

[dsp]

I agree with @smalyshev here. It's perfectly valid to iterate over an empty array. As #61527 points out, there shouldn't be any Notice at all in those cases.

[reeze]

em, it makes sense, next() didn't notice too.

so no notice at all in this case, am I right?

[reeze]

but the method related to current entry still need notice if array was empty or reached the end.
so only next() shouldn't complain. I will fix it soon.

thanks

[reeze]

I have updated the patch:

• No notice anymore when $iter->next() and update the notice message.

@smalyshev I've already rebased the pull request, that has been updated. the previous code show up in discussion.
please refer to the diff.

"There should be no messages when array is iterated to an end - it is perfectly normal thing to do. Empty array is a perfectly normal thing too, there should be no messages on that either. Also, I don't see why array should be rewinded. If it's at the end, it should just stay there."

Thanks.

[smalyshev]

current() should not give any notices either. It should just return false. That notice does not contribute anything.

[laruence]

actually, I think only notice for :"modified outside" is enough. no need yelling for "reached the end"

[reeze]

Hi,
I've updated the patch. no extra notice on next(), current() or key(), only notice when array was really modified.
and I refactor some code to reduce duplication. This bug also affect RecuseArrayIterator, I fixed it both of them.

Thanks.

[php-pulls]

Comment on behalf of stas at php.net:

merged