From c6658146a2d8402b51ecc45c132b7c51c4f66ab9 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 3 Dec 2023 01:53:01 +0100 Subject: [PATCH 1/2] Fix GH-12854: 8.3 - as final trait-used method does not correctly report visibility in Reflection --- Zend/tests/traits/gh12854.phpt | 49 ++++++++++++++++++++++++++++++++++ Zend/zend_inheritance.c | 16 ++++++++--- 2 files changed, 61 insertions(+), 4 deletions(-) create mode 100644 Zend/tests/traits/gh12854.phpt diff --git a/Zend/tests/traits/gh12854.phpt b/Zend/tests/traits/gh12854.phpt new file mode 100644 index 0000000000000..3bd1db396b214 --- /dev/null +++ b/Zend/tests/traits/gh12854.phpt @@ -0,0 +1,49 @@ +--TEST-- +GH-12854 (8.3 - as final trait-used method does not correctly report visibility in Reflection) +--FILE-- +isFinal()); + var_dump($rm->isPublic()); + var_dump($rm->isProtected()); + var_dump($rm->isPrivate()); +} + +?> +--EXPECTF-- +Warning: Private methods cannot be final as they are never overridden by other classes in %s on line %d +--- Method: pub --- +bool(true) +bool(true) +bool(false) +bool(false) +--- Method: prot --- +bool(true) +bool(false) +bool(true) +bool(false) +--- Method: priv --- +bool(true) +bool(false) +bool(false) +bool(true) diff --git a/Zend/zend_inheritance.c b/Zend/zend_inheritance.c index d1c66b310a6b4..6362090ada908 100644 --- a/Zend/zend_inheritance.c +++ b/Zend/zend_inheritance.c @@ -1949,6 +1949,10 @@ static void zend_add_trait_method(zend_class_entry *ce, zend_string *name, zend_ zend_function *new_fn; bool check_inheritance = false; + if ((fn->common.fn_flags & (ZEND_ACC_PRIVATE | ZEND_ACC_FINAL)) == (ZEND_ACC_PRIVATE | ZEND_ACC_FINAL)) { + zend_error(E_COMPILE_WARNING, "Private methods cannot be final as they are never overridden by other classes"); + } + if ((existing_fn = zend_hash_find_ptr(&ce->function_table, key)) != NULL) { /* if it is the same function with the same visibility and has not been assigned a class scope yet, regardless * of where it is coming from there is no conflict and we do not need to add it again */ @@ -2048,10 +2052,10 @@ static void zend_traits_copy_functions(zend_string *fnname, zend_function *fn, z && zend_string_equals_ci(alias->trait_method.method_name, fnname) ) { fn_copy = *fn; - - /* if it is 0, no modifiers have been changed */ - if (alias->modifiers) { + if (alias->modifiers & ZEND_ACC_PPP_MASK) { fn_copy.common.fn_flags = alias->modifiers | (fn->common.fn_flags & ~ZEND_ACC_PPP_MASK); + } else { + fn_copy.common.fn_flags = alias->modifiers | fn->common.fn_flags; } lcname = zend_string_tolower(alias->alias); @@ -2079,7 +2083,11 @@ static void zend_traits_copy_functions(zend_string *fnname, zend_function *fn, z && fn->common.scope == aliases[i] && zend_string_equals_ci(alias->trait_method.method_name, fnname) ) { - fn_copy.common.fn_flags = alias->modifiers | (fn->common.fn_flags & ~ZEND_ACC_PPP_MASK); + if (alias->modifiers & ZEND_ACC_PPP_MASK) { + fn_copy.common.fn_flags = alias->modifiers | (fn->common.fn_flags & ~ZEND_ACC_PPP_MASK); + } else { + fn_copy.common.fn_flags = alias->modifiers | fn->common.fn_flags; + } } alias_ptr++; alias = *alias_ptr; From 18be0958e8e550e426d67be0fdf373e3b69dfe29 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 3 Dec 2023 15:33:37 +0100 Subject: [PATCH 2/2] More testing --- Zend/tests/traits/gh12854.phpt | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/Zend/tests/traits/gh12854.phpt b/Zend/tests/traits/gh12854.phpt index 3bd1db396b214..471b4c6b56558 100644 --- a/Zend/tests/traits/gh12854.phpt +++ b/Zend/tests/traits/gh12854.phpt @@ -8,6 +8,10 @@ trait SimpleTrait public function pub() {} protected function prot() {} private function priv() {} + + public final function final1() {} + public final function final2() {} + public final function final3() {} } @@ -17,10 +21,14 @@ class Test pub as final; prot as final; priv as final; + + final1 as private; + final2 as protected; + final3 as public; } } -foreach (['pub', 'prot', 'priv'] as $method) { +foreach (['pub', 'prot', 'priv', 'final1', 'final2', 'final3'] as $method) { echo "--- Method: $method ---\n"; $rm = new ReflectionMethod(Test::class, $method); var_dump($rm->isFinal()); @@ -31,6 +39,8 @@ foreach (['pub', 'prot', 'priv'] as $method) { ?> --EXPECTF-- +Warning: Private methods cannot be final as they are never overridden by other classes in %s on line %d + Warning: Private methods cannot be final as they are never overridden by other classes in %s on line %d --- Method: pub --- bool(true) @@ -47,3 +57,18 @@ bool(true) bool(false) bool(false) bool(true) +--- Method: final1 --- +bool(true) +bool(false) +bool(false) +bool(true) +--- Method: final2 --- +bool(true) +bool(false) +bool(true) +bool(false) +--- Method: final3 --- +bool(true) +bool(true) +bool(false) +bool(false)