Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Prevent ArrayObject recursion in 5.6 #6096

Closed
wants to merge 1 commit into from

Conversation

datibbaw
Copy link
Contributor

@datibbaw datibbaw commented Apr 7, 2014

In PHP 5.6(next), calling isset() on an instance of ArrayObject will invoke both offsetExists() and offsetGet(). Adding an isset() inside offsetGet() will therefore result in an infinite recursion; this is related to php-src/614.

It causes tests/ZendTest/StdLib/ParametersTest.php to fail.

This patch addresses the above issue in a backward compatible manner.

@Ocramius
Copy link
Member

Ocramius commented Apr 7, 2014

@datibbaw such a fix needs a failing test case as well... Was this check removed in 5.6?

@datibbaw
Copy link
Contributor Author

datibbaw commented Apr 7, 2014

@Ocramius A failing test case would cause a crash, I'm not sure how to provide that without breaking the entire test suite :)

@Ocramius
Copy link
Member

Ocramius commented Apr 7, 2014

@datibbaw that's pretty much what it should do :-)

@datibbaw
Copy link
Contributor Author

datibbaw commented Apr 7, 2014

@Ocramius Perhaps I misunderstood what you're asking of me. The test case that should fail is already there; tests/ZendTest/Mvc/ApplicationTest.php

@Ocramius
Copy link
Member

Ocramius commented Apr 7, 2014

@datibbaw the test should be in ZendTest\Stdlib\ArrayObjectTest to avoid any possible breakage at component level

linking php/php-src#614

@datibbaw
Copy link
Contributor Author

datibbaw commented Apr 7, 2014

@Ocramius Ah, it fails tests/ZendTest/StdLib/ParametersTest.php as well :)

@Ocramius
Copy link
Member

Ocramius commented Apr 7, 2014

@datibbaw yeah, but those are all not testing at the source. While they indeed reveal the defect, a unit test on the affected unit should be written - I can eventually poke you in room11 when I have time.

@datibbaw
Copy link
Contributor Author

datibbaw commented Apr 8, 2014

@Ocramius I'm probably a bit dense, but the affected unit is Parameters and this patch aims to fix that :)

@Ocramius
Copy link
Member

Ocramius commented Apr 8, 2014

@datibbaw I'll be sure to have an additional cup of coffee tomorrow (was dragged away by the pr title) :-)

@Ocramius Ocramius added this to the 2.3.1 milestone Apr 8, 2014
@Ocramius Ocramius self-assigned this Apr 8, 2014
Ocramius added a commit that referenced this pull request Apr 8, 2014
@Ocramius Ocramius closed this in 2ec78a2 Apr 8, 2014
gianarb pushed a commit to zendframework/zend-stdlib that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-stdlib that referenced this pull request May 15, 2015
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants