php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #72957 Null coalescing operator doesn't behave as expected with SimpleXMLElement
Submitted: 2016-08-28 18:42 UTC Modified: 2016-08-29 14:24 UTC
From: dailywatchdeal at gmail dot com Assigned:
Status: Closed Package: SimpleXML related
PHP Version: 7.0.10 OS: Ubuntu
Private report: No CVE-ID:
 [2016-08-28 18:42 UTC] dailywatchdeal at gmail dot com
Description:
------------
According to the docs, the ?? operator checks if the 1st operand exists and is not null. In other words, it's equivalent to isset.
When used to check if a SimpleXMLElement node exists, it doesn't work.


Test script:
---------------
<?php

$xml = new SimpleXMLElement('<root><elem>Text</elem></root>');

echo 'elem2 is: ' . ($xml->elem2 ?? 'backup string');
echo 'elem2 is: ' . (isset($xml->elem2) ? $xml->elem2 : 'backup string');


Expected result:
----------------
elem2 is: backup string
elem2 is: backup string

Actual result:
--------------
elem2 is: 
elem2 is: backup string

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2016-08-28 20:14 UTC] requinix@php.net
-Type: Bug +Type: Feature/Change Request
 [2016-08-28 20:14 UTC] requinix@php.net
That's because $xml->elem2 exists and isn't null. isset works differently.
 [2016-08-29 08:51 UTC] dailywatchdeal at gmail dot com
According to the docs, ?? is the same as isset, just a different syntax. Maybe the documentation should be updated?
 [2016-08-29 10:24 UTC] cmb@php.net
> That's because $xml->elem2 exists and isn't null.

If it is set and it is not NULL, shouldn't isset() return TRUE?
The current behavior (<https://3v4l.org/1NYAA>) looks like a bug
to me.
 [2016-08-29 11:18 UTC] requinix@php.net
-Status: Open +Status: Verified -Type: Feature/Change Request +Type: Bug
 [2016-08-29 11:18 UTC] requinix@php.net
I'm being hypocritical: I've said before that $x ?? $y should be equivalent to isset($x) ? $x : $y, so this really is more of a bug than a request.

As I try to look deeper, this does indeed seem more like a bug - albeit one that may uniquely affect SimpleXMLElement. I could keep going into this but I don't know enough about the engine to tell why the same opcodes don't invoke has_property on SimpleXMLElement but do call __isset on userland classes. I do see that it uses a FETCH_OBJ_IS instead of the ISSET_ISEMPTY_PROP_OBJ jmping logic that isset() does... So does FETCH_OBJ_IS differ for internal and userland code?
https://3v4l.org/DF8AE/vld#output

I'll bump to Verified but I'm not confident enough in my analysis to go to Analyzed.

@cmb: No doubt that's part of the magic. I'd guess that the idea was to allow code like
  foreach ($xml->zero_or_more_elements as $e) {
without having to wrap it all in an isset check. Breaking it would be a BC thing, but I wonder how often that magic is actually relied upon...
The inconsistency is a bit of a bother. Has anyone else ever noticed it? With a quick search I haven't found any bug reports mentioning it. Maybe it's worth breaking?
 [2016-08-29 14:24 UTC] cmb@php.net
> So does FETCH_OBJ_IS differ for internal and userland code?

It doesn't appear to be so. Consider <https://3v4l.org/PJGqt>. In
either case (SimpleXMLElement and stClass) ZEND_FETCH_OBJ_IS takes
the same code path, and calls zobj->handlers->read_property()[1].
That works as expected for all userland and most internal objects,
but not for SimpleXMLElements.

So it boils down to SimpleXMLElements allowing to read properties,
which they otherwise claim to not exist. At least that is one
possible interpretation. The other would be that the null coalesce
operator would have to be compiled into the same opcodes as the
repective `isset($x) ? $x : $y`. Still, I'd find the
SimpleXMLElement inconsistency to be confusing, at best. I can't
assess the impact of changing the behavior, though.

[1] <https://github.com/php/php-src/blob/PHP-7.1.0beta3/Zend/zend_vm_def.h#L2066>
 [2016-08-30 11:07 UTC] nikic@php.net
Automatic comment on behalf of nikic
Revision: http://git.php.net/?p=php-src.git;a=commit;h=bfd4277008d3bda95ff5b418c60d41d50488d33b
Log: Fix bug #72957
 [2016-08-30 11:07 UTC] nikic@php.net
-Status: Verified +Status: Closed
 [2016-08-30 12:15 UTC] ab@php.net
Automatic comment on behalf of nikic
Revision: http://git.php.net/?p=php-src.git;a=commit;h=345b96c4a3f20f4237028005b8189c2ab480e467
Log: Fix bug #72957
 [2016-09-13 04:09 UTC] stas@php.net
Automatic comment on behalf of nikic
Revision: http://git.php.net/?p=php-src.git;a=commit;h=345b96c4a3f20f4237028005b8189c2ab480e467
Log: Fix bug #72957
 [2016-09-13 04:11 UTC] stas@php.net
Automatic comment on behalf of nikic
Revision: http://git.php.net/?p=php-src.git;a=commit;h=345b96c4a3f20f4237028005b8189c2ab480e467
Log: Fix bug #72957
 [2016-10-17 10:08 UTC] bwoebi@php.net
Automatic comment on behalf of nikic
Revision: http://git.php.net/?p=php-src.git;a=commit;h=345b96c4a3f20f4237028005b8189c2ab480e467
Log: Fix bug #72957
 [2016-10-17 10:08 UTC] bwoebi@php.net
Automatic comment on behalf of nikic
Revision: http://git.php.net/?p=php-src.git;a=commit;h=bfd4277008d3bda95ff5b418c60d41d50488d33b
Log: Fix bug #72957
 
PHP Copyright © 2001-2017 The PHP Group
All rights reserved.
Last updated: Mon Feb 20 22:01:35 2017 UTC