php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Request #69659 ArrayAccess, isset() and the offsetExists method
Submitted: 2015-05-19 07:48 UTC Modified: 2016-05-03 14:52 UTC
Votes:1
Avg. Score:3.0 ± 0.0
Reproduced:1 of 1 (100.0%)
Same Version:1 (100.0%)
Same OS:1 (100.0%)
From: contact dot 01834e2c at renegade334 dot me dot uk Assigned: nikic
Status: Closed Package: SPL related
PHP Version: master-Git-2015-05-19 (Git) OS: Ubuntu 14.04.2 (Linux 3.13.0-52)
Private report: No CVE-ID:
 [2015-05-19 07:48 UTC] contact dot 01834e2c at renegade334 dot me dot uk
Description:
------------
Calling isset() or empty() on a dimension of an object that implements ArrayAccess results in the offsetExists method being called.

When calling isset() or empty() on a subdimension of a dimension of that object, only the offsetGet method is called, not the offsetExists method. This is inconsistent, undocumented, and often results in undefined index notices being raised.

Instead, when isset() or empty() is called on a subdimension of an object implementing ArrayAccess, the offsetExists method should first be called to check if the dimension exists. If the dimension does not exist, then offsetGet should not be called.

This could possibly be implemented at the level of zend_std_read_dimension() by calling and checking the "offsetexists" method if type==BP_VAR_IS.

This was previously raised several years ago as #41727. At the time, it was deemed WONTFIX, although the discussion seemed to miss the point somewhat. I think the issue should be revisited, not least because the lack of consistency leads to some interesting quirks elsewhere.

For example, SPL's ArrayObject class, which 'implements' ArrayAccess internally, will also not call offsetExists when executing isset($obj['foo']['bar']). However, while the native "get dimension" handler detects the BP_VAR_IS type and will not throw an invalid index error in this case, calling parent::offsetGet() from an overridden offsetGet method *will* result in invalid index errors being thrown. This was reported under #62059.

Test script:
---------------
https://gist.github.com/Renegade334/2415ec56367ff937c407

Expected result:
----------------
renegade@ochre:~$ builds/php7/bin/php test.php
* Calling isset($obj['foo'])...
offsetExists('foo')
bool(false)
* Calling isset($obj['foo']['bar'])...
offsetExists('foo')
bool(false)
* Setting offset...
offsetSet('foo', NULL)
* Calling isset($obj['foo']['bar'])...
offsetExists('foo')
offsetGet('foo')
bool(false)

Actual result:
--------------
renegade@ochre:~$ builds/php7/bin/php test.php
* Calling isset($obj['foo'])...
offsetExists('foo')
bool(false)
* Calling isset($obj['foo']['bar'])...
offsetGet('foo')

Notice: Undefined index: foo in /home/renegade/test.php on line 14
bool(false)
* Setting offset...
offsetSet('foo', NULL)
* Calling isset($obj['foo']['bar'])...
offsetGet('foo')
bool(false)

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2016-03-20 12:56 UTC] nikic@php.net
Mostly fixed by https://github.com/php/php-src/commit/a175aa9dcaed5e295d015ff73c663e06c2335155. ArrayObject may need additional modifications.
 [2016-03-20 13:07 UTC] nikic@php.net
-Assigned To: +Assigned To: nikic
 [2016-03-20 17:21 UTC] nikic@php.net
-Status: Assigned +Status: Closed
 [2016-05-03 13:19 UTC] jakob611 at yahoo dot de
bricks owncloud9 and i'm sure it also bricks a bunch of other pieces of software

print_r($this->server, true) => contains a key named REQUEST_URI and it has a value

$requestUri = isset($this->server['REQUEST_URI']) ? $this->server['REQUEST_URI'] : ''; => returns nothing

isset is broken

Personally, I also think that changing behaviour (even if it is a bugfix) in such an essential function within a minor release is a very bad idea.
 [2016-05-03 14:52 UTC] nikic@php.net
It is unfortunate that this change had to happen in a patch release, rather than in PHP 7.0.0. It's my fault that I did not notice during the pre-release cycle that this previously relatively harmless bug completely breaks the null-coalesce operator in PHP 7. However, that's how things went, and at this point a fix was critical.

For reference, the way to fix any issues with this change is to make sure your __isset() or offsetExists() functions behave correctly. If this change causes issues with your code, it indicates that you already have a broken __isset/offsetExists implementation, but it's brokenness did not previously surface (one bug canceling another...)
 
PHP Copyright © 2001-2017 The PHP Group
All rights reserved.
Last updated: Sat Apr 29 07:01:45 2017 UTC