From ad2ab3a140a39c5290d22d63af8f87ba8391bad4 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Tue, 21 Apr 2020 15:14:25 +0200 Subject: [PATCH 1/5] Fix #79487: ::getStaticProperties() ignores property modifications When retrieving the static class properties via reflection, we have to cater to possible modifications. --- ext/reflection/php_reflection.c | 15 ++++++++++----- ext/reflection/tests/bug79487.phpt | 17 +++++++++++++++++ 2 files changed, 27 insertions(+), 5 deletions(-) create mode 100644 ext/reflection/tests/bug79487.phpt diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index dbfb67386e9bf..6bae12adccfc4 100644 --- a/ext/reflection/php_reflection.c +++ b/ext/reflection/php_reflection.c @@ -3719,7 +3719,7 @@ ZEND_METHOD(reflection_class, __construct) /* }}} */ /* {{{ add_class_vars */ -static void add_class_vars(zend_class_entry *ce, int statics, zval *return_value) +static void add_class_vars(zend_class_entry *ce, int statics, int defaults, zval *return_value) { zend_property_info *prop_info; zval *prop, prop_copy; @@ -3734,7 +3734,12 @@ static void add_class_vars(zend_class_entry *ce, int statics, zval *return_value } prop = NULL; if (statics && (prop_info->flags & ZEND_ACC_STATIC) != 0) { - prop = &ce->default_static_members_table[prop_info->offset]; + zval *members; + if (!defaults && (members = CE_STATIC_MEMBERS(ce))) { + prop = &members[prop_info->offset]; + } else { + prop = &ce->default_static_members_table[prop_info->offset]; + } ZVAL_DEINDIRECT(prop); } else if (!statics && (prop_info->flags & ZEND_ACC_STATIC) == 0) { prop = &ce->default_properties_table[OBJ_PROP_TO_NUM(prop_info->offset)]; @@ -3778,7 +3783,7 @@ ZEND_METHOD(reflection_class, getStaticProperties) } array_init(return_value); - add_class_vars(ce, 1, return_value); + add_class_vars(ce, 1, 0, return_value); } /* }}} */ @@ -3876,8 +3881,8 @@ ZEND_METHOD(reflection_class, getDefaultProperties) if (UNEXPECTED(zend_update_class_constants(ce) != SUCCESS)) { return; } - add_class_vars(ce, 1, return_value); - add_class_vars(ce, 0, return_value); + add_class_vars(ce, 1, 1, return_value); + add_class_vars(ce, 0, 1, return_value); } /* }}} */ diff --git a/ext/reflection/tests/bug79487.phpt b/ext/reflection/tests/bug79487.phpt new file mode 100644 index 0000000000000..be8f733d79df1 --- /dev/null +++ b/ext/reflection/tests/bug79487.phpt @@ -0,0 +1,17 @@ +--TEST-- +Bug #79487 (::getStaticProperties() ignores property modifications) +--FILE-- +getStaticProperties()); +?> +--EXPECT-- +array(1) { + ["bar"]=> + string(3) "new" +} From 2156b985363df2d50bbbbe8de9e0801858f94300 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Tue, 21 Apr 2020 16:21:07 +0200 Subject: [PATCH 2/5] Inline special cased add_class_vars() As suggested by Nikita. --- ext/reflection/php_reflection.c | 53 ++++++++++++++++++++++++++------- 1 file changed, 43 insertions(+), 10 deletions(-) diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index 6bae12adccfc4..b54235a7816a3 100644 --- a/ext/reflection/php_reflection.c +++ b/ext/reflection/php_reflection.c @@ -3719,7 +3719,7 @@ ZEND_METHOD(reflection_class, __construct) /* }}} */ /* {{{ add_class_vars */ -static void add_class_vars(zend_class_entry *ce, int statics, int defaults, zval *return_value) +static void add_class_vars(zend_class_entry *ce, int statics, zval *return_value) { zend_property_info *prop_info; zval *prop, prop_copy; @@ -3734,12 +3734,7 @@ static void add_class_vars(zend_class_entry *ce, int statics, int defaults, zval } prop = NULL; if (statics && (prop_info->flags & ZEND_ACC_STATIC) != 0) { - zval *members; - if (!defaults && (members = CE_STATIC_MEMBERS(ce))) { - prop = &members[prop_info->offset]; - } else { - prop = &ce->default_static_members_table[prop_info->offset]; - } + prop = &ce->default_static_members_table[prop_info->offset]; ZVAL_DEINDIRECT(prop); } else if (!statics && (prop_info->flags & ZEND_ACC_STATIC) == 0) { prop = &ce->default_properties_table[OBJ_PROP_TO_NUM(prop_info->offset)]; @@ -3771,6 +3766,9 @@ ZEND_METHOD(reflection_class, getStaticProperties) { reflection_object *intern; zend_class_entry *ce; + zend_property_info *prop_info; + zval *prop, prop_copy; + zend_string *key; if (zend_parse_parameters_none() == FAILURE) { return; @@ -3783,7 +3781,42 @@ ZEND_METHOD(reflection_class, getStaticProperties) } array_init(return_value); - add_class_vars(ce, 1, 0, return_value); + + ZEND_HASH_FOREACH_STR_KEY_PTR(&ce->properties_info, key, prop_info) { + if (((prop_info->flags & ZEND_ACC_PROTECTED) && + !zend_check_protected(prop_info->ce, ce)) || + ((prop_info->flags & ZEND_ACC_PRIVATE) && + prop_info->ce != ce)) { + continue; + } + prop = NULL; + if ((prop_info->flags & ZEND_ACC_STATIC) != 0) { + zval *members; + if ((members = CE_STATIC_MEMBERS(ce))) { + prop = &members[prop_info->offset]; + } else { + prop = &ce->default_static_members_table[prop_info->offset]; + } + ZVAL_DEINDIRECT(prop); + } + if (!prop || (prop_info->type && Z_ISUNDEF_P(prop))) { + continue; + } + + /* copy: enforce read only access */ + ZVAL_DEREF(prop); + ZVAL_COPY_OR_DUP(&prop_copy, prop); + + /* this is necessary to make it able to work with default array + * properties, returned to user */ + if (Z_TYPE(prop_copy) == IS_CONSTANT_AST) { + if (UNEXPECTED(zval_update_constant_ex(&prop_copy, ce) != SUCCESS)) { + return; + } + } + + zend_hash_update(Z_ARRVAL_P(return_value), key, &prop_copy); + } ZEND_HASH_FOREACH_END(); } /* }}} */ @@ -3881,8 +3914,8 @@ ZEND_METHOD(reflection_class, getDefaultProperties) if (UNEXPECTED(zend_update_class_constants(ce) != SUCCESS)) { return; } - add_class_vars(ce, 1, 1, return_value); - add_class_vars(ce, 0, 1, return_value); + add_class_vars(ce, 1, return_value); + add_class_vars(ce, 0, return_value); } /* }}} */ From 3a35569c3553b24be0797ac48e215d6e7b119d17 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Tue, 19 May 2020 15:11:36 +0200 Subject: [PATCH 3/5] Drop superfluous protected checks --- ext/reflection/php_reflection.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index b54235a7816a3..44b8f6432ffda 100644 --- a/ext/reflection/php_reflection.c +++ b/ext/reflection/php_reflection.c @@ -3726,9 +3726,7 @@ static void add_class_vars(zend_class_entry *ce, int statics, zval *return_value zend_string *key; ZEND_HASH_FOREACH_STR_KEY_PTR(&ce->properties_info, key, prop_info) { - if (((prop_info->flags & ZEND_ACC_PROTECTED) && - !zend_check_protected(prop_info->ce, ce)) || - ((prop_info->flags & ZEND_ACC_PRIVATE) && + if (((prop_info->flags & ZEND_ACC_PRIVATE) && prop_info->ce != ce)) { continue; } @@ -3783,9 +3781,7 @@ ZEND_METHOD(reflection_class, getStaticProperties) array_init(return_value); ZEND_HASH_FOREACH_STR_KEY_PTR(&ce->properties_info, key, prop_info) { - if (((prop_info->flags & ZEND_ACC_PROTECTED) && - !zend_check_protected(prop_info->ce, ce)) || - ((prop_info->flags & ZEND_ACC_PRIVATE) && + if (((prop_info->flags & ZEND_ACC_PRIVATE) && prop_info->ce != ce)) { continue; } From 7a7654ca1fcf0b5caa3ef66db65b87c5132e6e48 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Tue, 19 May 2020 15:44:22 +0200 Subject: [PATCH 4/5] Simplify by continuing early --- ext/reflection/php_reflection.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index 44b8f6432ffda..498bba644c641 100644 --- a/ext/reflection/php_reflection.c +++ b/ext/reflection/php_reflection.c @@ -3781,21 +3781,24 @@ ZEND_METHOD(reflection_class, getStaticProperties) array_init(return_value); ZEND_HASH_FOREACH_STR_KEY_PTR(&ce->properties_info, key, prop_info) { + zval *members; + if (((prop_info->flags & ZEND_ACC_PRIVATE) && prop_info->ce != ce)) { continue; } - prop = NULL; - if ((prop_info->flags & ZEND_ACC_STATIC) != 0) { - zval *members; - if ((members = CE_STATIC_MEMBERS(ce))) { - prop = &members[prop_info->offset]; - } else { - prop = &ce->default_static_members_table[prop_info->offset]; - } - ZVAL_DEINDIRECT(prop); + if ((prop_info->flags & ZEND_ACC_STATIC) == 0) { + continue; } - if (!prop || (prop_info->type && Z_ISUNDEF_P(prop))) { + + if ((members = CE_STATIC_MEMBERS(ce))) { + prop = &members[prop_info->offset]; + } else { + prop = &ce->default_static_members_table[prop_info->offset]; + } + ZVAL_DEINDIRECT(prop); + + if (prop_info->type && Z_ISUNDEF_P(prop)) { continue; } From 237b1b95a1908b3cef12588ae5822e05b8777085 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Tue, 23 Jun 2020 17:15:25 +0200 Subject: [PATCH 5/5] Init class statics if necessary This is necessary for inherited properties, and allows us to drop the check later. We also remove the unnecessary `IS_CONSTANT_AST` handling. --- ext/reflection/php_reflection.c | 18 +++++------------- ext/reflection/tests/bug79487.phpt | 17 +++++++++++++++++ 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index 498bba644c641..41c8c04e9080b 100644 --- a/ext/reflection/php_reflection.c +++ b/ext/reflection/php_reflection.c @@ -3778,6 +3778,10 @@ ZEND_METHOD(reflection_class, getStaticProperties) return; } + if (!CE_STATIC_MEMBERS(ce)) { + zend_class_init_statics(ce); + } + array_init(return_value); ZEND_HASH_FOREACH_STR_KEY_PTR(&ce->properties_info, key, prop_info) { @@ -3791,11 +3795,7 @@ ZEND_METHOD(reflection_class, getStaticProperties) continue; } - if ((members = CE_STATIC_MEMBERS(ce))) { - prop = &members[prop_info->offset]; - } else { - prop = &ce->default_static_members_table[prop_info->offset]; - } + prop = &CE_STATIC_MEMBERS(ce)[prop_info->offset]; ZVAL_DEINDIRECT(prop); if (prop_info->type && Z_ISUNDEF_P(prop)) { @@ -3806,14 +3806,6 @@ ZEND_METHOD(reflection_class, getStaticProperties) ZVAL_DEREF(prop); ZVAL_COPY_OR_DUP(&prop_copy, prop); - /* this is necessary to make it able to work with default array - * properties, returned to user */ - if (Z_TYPE(prop_copy) == IS_CONSTANT_AST) { - if (UNEXPECTED(zval_update_constant_ex(&prop_copy, ce) != SUCCESS)) { - return; - } - } - zend_hash_update(Z_ARRVAL_P(return_value), key, &prop_copy); } ZEND_HASH_FOREACH_END(); } diff --git a/ext/reflection/tests/bug79487.phpt b/ext/reflection/tests/bug79487.phpt index be8f733d79df1..5185c98bba907 100644 --- a/ext/reflection/tests/bug79487.phpt +++ b/ext/reflection/tests/bug79487.phpt @@ -9,9 +9,26 @@ class Foo { Foo::$bar = 'new'; $rc = new ReflectionClass('Foo'); var_dump($rc->getStaticProperties()); + +class A { + public static $a = 'A old'; +} +class B extends A { + public static $b = 'B old'; +} + +$rc = new ReflectionClass(B::class); +A::$a = 'A new'; +var_dump($rc->getStaticProperties()); ?> --EXPECT-- array(1) { ["bar"]=> string(3) "new" } +array(2) { + ["b"]=> + string(5) "B old" + ["a"]=> + string(5) "A new" +}