isset() and empty() produce apparently spurious runtime error

Bug #53971	isset() and empty() produce apparently spurious runtime error
Submitted:	2011-02-09 13:12 UTC	Modified:	2011-02-14 09:48 UTC
From:	david at frankieandshadow dot com	Assigned:	dmitry (profile)
Status:	Closed	Package:	Arrays related
PHP Version:	5.3.5	OS:	Linux, Redhat Enterprise
Private report:	No	CVE-ID:	None

[2011-02-09 13:12 UTC] david at frankieandshadow dot com

Description:
------------
First, apologies, this is 5.3.3. I have no means of upgrading to check whether 
this is a fixed issue. I can't see anything similar in the bug database.

An expression of the form 
  isset($obj->m['a']['b'])
produces a runtime error when m is not actually an array but a zero length 
string:
  Uninitialized string offset: 0
The same is the case if I use empty instead of isset.
The same code worked in 5.2. Changing it to 
  isset($obj->m['a']) && isset($obj->m['a']['b'])
works.

isset is not supposed to produce any runtime error, surely, in this kind of use.

The object member values arise from a database lookup, and normally the field (m 
above) will be a serialized array in the database, but the first time the 
database column will be empty, leading to an empty string assignment to m. In 
addition once populated the object is stored in $_SESSION (actually in 
$_SESSION['p']['q']) and then $obj is obtained by assignment from the session, 
like this
  $session =& $_SESSION['p']; // where $_SESSION['p'] is set in a previous page
  // populate new $obj1 from database
  $session['q'] = $obj1;
  ...
  $obj =& $session['q'];
and the offending code is then executed elsewhere some time later.

However, abstracting the code from this much bigger program does not demonstrate 
the problem, which suggests to me something is corrupted somewhere. This is what 
I tried on its own, which is as close as I can reasonably get to the situation 
here, but it works. (The =& are leftovers from what was originally a PHP4 app; I 
know all objects are assigned by reference in PHP5).

class c { var $m; }

session_start();
if (! isset($_SESSION['p'])) {
  $_SESSION['p'] = array();
  echo "set session array";
  exit;
}
$session =& $_SESSION['p'];
$obj1 = new c();
$obj1->m = '';
$session['q'] = $obj1;
$obj =& $session['q'];
function check() {
  global $obj;
  echo (isset($obj->m['a']['b']) ? 'Yes' : 'No');
}
check();

Expected result:
----------------
isset to return FALSE

Actual result:
--------------
runtime error
Uninitialized string offset: 0

Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports

[2011-02-09 17:50 UTC] dtajchreber@php.net

$obj->m is an empty string. You try to access a non-integer offset. Non-integer 
offsets are converted to 
integers. So in other words:
$obj->m['a'] becomes $obj->m[0]
$obj->m is an empty string and $obj->m[0] doesn't exist
This behavior is documented here: 
http://us.php.net/manual/en/language.types.string.php

"Warning
Writing to an out of range offset pads the string with spaces. Non-integer types 
are converted to integer. 
Illegal offset type emits E_NOTICE. Negative offset emits E_NOTICE in write but 
reads empty string. Only the 
first character of an assigned string is used. Assigning empty string assigns 
NUL byte."

Simplifying the problem: http://codepad.org/G31wr4oJ

[2011-02-09 21:30 UTC] david at frankieandshadow dot com

I appreciate that this is the case. When you say "$obj->m[0] doesn't exist", yes 
I agree, BUT that is what the isset is testing for.

If it SHOULD produce a runtime error, then (a) this is a very subtle non-upwards 
compatible change from 5.2, and (b) the example I quoted does NOT produce a 
runtime error so is a bug.

(And producing a runtime error in these circumstances is terribly inconvenient, 
it means you can't test existence in one go but have to try each element 
individually).

If it SHOULD NOT produce a runtime error then there is a problem with the larger 
code I have which follows this pattern and is doing so.

There is a bug here one way or the other: either my larger program is wrong (but 
has worked for years with this code in it) or the example I put in the bug 
report is wrong in that it does not produce an error and never has. At present 
the behaviour is inconsistent.

[2011-02-11 18:41 UTC] dtajchreber@php.net

It looks like there was the change in behavior between 5.2.9 and 5.2.10. I don't 
know whether this was intentional or a side affect of another change. 

The difference can be seen here:
http://codepad.org/iYcCCAkA /* 5.2.5 */
http://codepad.viper-7.com/msAbwQ  /* 5.2.15RC3-dev */

I've reopened the ticket so this can be discussed. If it is intentional, it 
needs to be documented.

[2011-02-13 16:48 UTC] cataphract@php.net

-Status: Open +Status: Assigned -Assigned To: +Assigned To: dmitry

[2011-02-13 16:48 UTC] cataphract@php.net

isset() should definitely not be emitting notices.

Note that $string[1][0] is a relatively new construct (rev 301320), but it should follow the previous rules that empty and isset are silent.

[2011-02-14 09:48 UTC] dmitry@php.net

-Status: Assigned +Status: Closed

[2011-02-14 09:48 UTC] dmitry@php.net

This bug has been fixed in SVN.

Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.
 
Thank you for the report, and for helping us make PHP better.