php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #72223 regression with _isset, __get call precedence
Submitted: 2016-05-16 13:00 UTC Modified: 2016-05-16 15:14 UTC
From: koubel at seznam dot cz Assigned:
Status: Not a bug Package: *General Issues
PHP Version: 7.0.6 OS: Linux
Private report: No CVE-ID: None
 [2016-05-16 13:00 UTC] koubel at seznam dot cz
Description:
------------
When we wanted migrate from PHP 5 to PHP 7 (7.0.6), we hitted BC break with __isset and __get call precedence. It stops migration, because we have a tons of lines with isset($instance->magicProperty["index")) calls.

There is also no mention about this break in documentation.

P.S. According https://3v4l.org/ka9na, it seems that something changed in PHP 7.0.6, 7.0.5 works fine.

Test script:
---------------
class IsGet {
	private $arr = [1 => 1, 2 => 2, 3 => 3];

	function &__get($property) {
		echo "__get\n";
		return $this->arr;
	}

	function __isset($property) {
		echo "__isset\n";
		return false;
	}
}

$instance = new IsGet();
var_dump(isset($instance->arr[1]));


Expected result:
----------------
__get
bool(true)

Actual result:
--------------
__isset
bool(false)

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2016-05-16 14:46 UTC] koubel at seznam dot cz
So, maybe it's related with fix mentioned in https://bugs.php.net/bug.php?id=72117. It means that

function __isset($property) {
 return $property == "arr";
}

fixed this bug.

But this one isn't mentioned case. Caller know that magic property exists and it's array. Returning this array and call isset on it is expected. Implement __isset doesn't make any sense in this case.

If you want to keep new behaviour, it's necessary to mention it on documentation on magic properties and also in migration with from 5.6.x to 7.0.x, because it's BC break.
 [2016-05-16 15:09 UTC] koubel at seznam dot cz
Hmm, it seems that without any __isset it works fine.
In real case we inherit from "generic" __isset from other library. 
I still thinking it's BC break and needs to be documented in migration guide.
 [2016-05-16 15:14 UTC] requinix@php.net
-Status: Open +Status: Not a bug
 [2016-05-16 15:15 UTC] requinix@php.net
The fact that __isset was not being called before was a bug. This is a bug fix.
See bug #72117.
 [2016-05-16 15:39 UTC] koubel at seznam dot cz
Maybe, but it's definitely BC break, __isset wasn't called in that cases in PHP < 7.0.6.

We have many objects which inherits from one external "Object" generic library which has implemented __isset without any knowledge this new precedence.

If we want to switch to 7.0.6, we must
- patch the library
or 
- check all our descendants and write new correct __isset for each one

It's very complicated
 
PHP Copyright © 2001-2021 The PHP Group
All rights reserved.
Last updated: Sun Feb 28 04:01:23 2021 UTC