php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #63976 Parent class incorrectly using child constant in class property
Submitted: 2013-01-12 03:51 UTC Modified: 2013-03-19 13:06 UTC
Votes:9
Avg. Score:5.0 ± 0.0
Reproduced:9 of 9 (100.0%)
Same Version:8 (88.9%)
Same OS:8 (88.9%)
From: don at smugmug dot com Assigned: dmitry
Status: Closed Package: Class/Object related
PHP Version: 5.4.10 OS: Ubuntu 12.04
Private report: No CVE-ID:
 [2013-01-12 03:51 UTC] don at smugmug dot com
Description:
------------
Class properties that rely on potentially inherited class constants have 
unpredictable behavior.

Since PHP doesn't support Child class properties referencing static values like 
static::CONST, the meaning of self::CONST is ambiguous. One of two things should 
happen:

1. It should use the value defined in the actual class in question (like self:: 
is used throughout the rest of PHP).

2. It should treat self:: in this case, since it's compile-time and not late 
static binding, like static:: and walk the inheritance tree, delivering the 
result for the Child class.

Option #1 seems the most sane, but PHP often behaves like it intends #2 to work. 
But not always...

In the provided examples, 'brokenA.php' behaves like #1, above, while 
'brokenB.php' and 'brokenC.php' behave like #2. The only thing that's changed is 
the order in which the classes are require()'d.

In a complex script, with autoloaders, class instantiation order isn't 
predictable, of course, resulting in unpredictable results.

Test script:
---------------
Example code: https://github.com/onethumb/php-parent-child-constant-bug

Expected result:
----------------
Consistent results for Baz->table.  Either 'foo' or 'baz' 100% of the time, rather 
than mixed up depending on require() order.

Have a preference for adding static::CONST to PHP and making self::CONST behave 
like self:: does in the rest of the language (resulting in Baz->table == 'baz' in 
the examples if we used static::CONST), but if that's not preferable for some 
reason, self::CONST should probably behave like self:: everywhere else (resulting 
in Baz->table == 'foo' in the examples).

Actual result:
--------------
brokenA.php:
Bar Object
(
    [table] => bar
)
Baz Object
(
    [table] => foo
)

brokenB.php:
Bar Object
(
    [table] => bar
)
Baz Object
(
    [table] => baz
)

brokenC.php:
Baz Object
(
    [table] => baz
)
Bar Object
(
    [table] => bar
)
Baz Object
(
    [table] => baz
)

Patches

zend_do_fetch_constant (last revision 2013-03-19 09:59 UTC) by mike@php.net)
update_class_constants (last revision 2013-02-14 17:30 UTC) by mike@php.net)

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2013-01-12 03:53 UTC] don at smugmug dot com
'Have a preference' should have said 'I have a preference'.  Certainly not 
intending for PHP to add some new INI option or something to change how static:: 
and self:: behave. :)

Also, have confirmed this on 5.4.4 and CentOS in addition to Ubuntu and 5.4.10, 
both with and without extensions like APC loaded.
 [2013-02-14 17:30 UTC] mike@php.net
The following patch has been added/updated:

Patch Name: update_class_constants
Revision:   1360863036
URL:        https://bugs.php.net/patch-display.php?bug=63976&patch=update_class_constants&revision=1360863036
 [2013-03-14 09:01 UTC] mike@php.net
-Assigned To: +Assigned To: dmitry
 [2013-03-14 09:01 UTC] mike@php.net
@dmitry, can you have a look at the , please? 

The condition could actually be simplified to

 if (ce->type == ZEND_USER_CLASS) ...

Thank you.
 [2013-03-14 17:42 UTC] don at smugmug dot com
I have validated that the patch solves our use case (and the test case in this 
bug).  Thanks Mike!
 [2013-03-18 13:09 UTC] dmitry@php.net
I understood the problem and your patch, I also agree what it fixes the problem for this particular case.

Unfortunately, I afraid that it may break some applications, because it may make constant resolution early in the moment, when such constants were not defined yet. For example your patch is going to break the following script.

<?php
class A {
	public $x = X;
}

class B extends A {
}

const X = 1;

$b = new B;
var_dump($b);
?>

I think the problem may be solved by substituting "self" and "parent" by actual class names at compile-time. It must be quite easy to do it for "self" (in zend_do_fetch_constant or in zend_do_declare_property), but not so easy for "parent" (we may use CG(parent_class_name) to solve it).
 [2013-03-19 09:59 UTC] mike@php.net
The following patch has been added/updated:

Patch Name: zend_do_fetch_constant
Revision:   1363687163
URL:        https://bugs.php.net/patch-display.php?bug=63976&patch=zend_do_fetch_constant&revision=1363687163
 [2013-03-19 10:00 UTC] mike@php.net
Unfortunately, this also changes error messages to something like:

Undefined class constant 'Foo::B' instead of 'self::B'...
 [2013-03-19 12:35 UTC] mike@php.net
Reflection is also affected.

...and I couldn't come up with code yet, where parent:: is problematic.
 [2013-03-19 13:05 UTC] dmitry@php.net
Automatic comment on behalf of dmitry@zend.com
Revision: http://git.php.net/?p=php-src.git;a=commit;h=7b0993bfb4b19bb160a72310b907c4fc12a5bef9
Log: Fixed bug #63976 (Parent class incorrectly using child constant in class property)
 [2013-03-19 13:05 UTC] dmitry@php.net
-Status: Assigned +Status: Closed
 [2013-03-19 13:06 UTC] dmitry@php.net
The fix for this bug has been committed.

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/.

 For Windows:

http://windows.php.net/snapshots/
 
Thank you for the report, and for helping us make PHP better.


 [2013-04-16 08:58 UTC] giacomo at boticca dot com
I agree this was a bug but as it breaks existing code it shouldn't have been 
released in the 5.4 branch. It should have thrown an E_WARNING at worst.

A backward incompatible change shouldn't be in a minor release, that's what major 
releases are for.
 
PHP Copyright © 2001-2014 The PHP Group
All rights reserved.
Last updated: Fri Apr 18 18:01:58 2014 UTC