Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions Zend/tests/bug70437.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
--TEST--
Bug #70437 (Changes to static properties in trait methods are not reflected in the class)
--FILE--
<?php

trait foo {
static $bar = 1;

public static function baz() {
var_dump(++self::$bar);
var_dump(++static::$bar);
}
}

class bar {
use foo { baz as private foo; }

public static function baz() {
self::foo();
try {
foo::baz();
} catch (Error $e) {
print $e->getMessage()."\n";
}
++self::$bar;
var_dump(self::$bar);
try {
var_dump(foo::$bar);
} catch (Error $e) {
print $e->getMessage()."\n";
}
}
}

bar::baz();

?>
--EXPECT--
int(2)
int(3)
Cannot call abstract method foo::baz()
int(4)
Cannot access abstract property foo::$bar
7 changes: 7 additions & 0 deletions Zend/zend_compile.c
Original file line number Diff line number Diff line change
Expand Up @@ -4637,6 +4637,9 @@ void zend_begin_method_decl(zend_op_array *op_array, zend_string *name, zend_boo
} else if (!has_body) {
zend_error_noreturn(E_COMPILE_ERROR, "Non-abstract method %s::%s() must contain body",
ZSTR_VAL(ce->name), ZSTR_VAL(name));
} else if (in_trait) {
/* force abstract in traits to prevent the_trait::static_method() from being allowed to be called */
op_array->fn_flags |= ZEND_ACC_ABSTRACT | ZEND_ACC_CHANGED;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a hack, the method is no actually abstract but forced to have abstract flag. It's probably not a good idea.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have a better idea how to make methods non-callable and which integrates nicely into the existing source?

}

op_array->scope = ce;
Expand Down Expand Up @@ -4914,6 +4917,10 @@ void zend_compile_prop_decl(zend_ast *ast) /* {{{ */
zend_error_noreturn(E_COMPILE_ERROR, "Properties cannot be declared abstract");
}

if ((ce->ce_flags & ZEND_ACC_TRAIT) && !(flags & ZEND_ACC_PRIVATE)) {
flags |= ZEND_ACC_ABSTRACT;
}

/* Doc comment has been appended as last element in property list */
if (list->child[children - 1]->kind == ZEND_AST_ZVAL) {
doc_comment = zend_string_copy(zend_ast_get_str(list->child[children - 1]));
Expand Down
29 changes: 18 additions & 11 deletions Zend/zend_inheritance.c
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,9 @@ static void do_inherit_parent_constructor(zend_class_entry *ce) /* {{{ */

char *zend_visibility_string(uint32_t fn_flags) /* {{{ */
{
if (fn_flags & ZEND_ACC_ABSTRACT) {
return "abstract";
}
if (fn_flags & ZEND_ACC_PRIVATE) {
return "private";
}
Expand Down Expand Up @@ -269,7 +272,7 @@ static zend_bool zend_do_perform_implementation_check(const zend_function *fe, c
}

/* If both methods are private do not enforce a signature */
if ((fe->common.fn_flags & ZEND_ACC_PRIVATE) && (proto->common.fn_flags & ZEND_ACC_PRIVATE)) {
if ((fe->common.fn_flags & ZEND_ACC_PRIVATE) && (proto->common.fn_flags & ZEND_ACC_PRIVATE)) {
return 1;
}

Expand Down Expand Up @@ -536,7 +539,9 @@ static void do_inheritance_check_on_method(zend_function *child, zend_function *

/* Disallow making an inherited method abstract. */
if (UNEXPECTED((child_flags & ZEND_ACC_ABSTRACT) > (parent_flags & ZEND_ACC_ABSTRACT))) {
zend_error_noreturn(E_COMPILE_ERROR, "Cannot make non abstract method %s::%s() abstract in class %s", ZEND_FN_SCOPE_NAME(parent), ZSTR_VAL(child->common.function_name), ZEND_FN_SCOPE_NAME(child));
if (UNEXPECTED((child_flags & ZEND_ACC_CHANGED) == 0)) {
zend_error_noreturn(E_COMPILE_ERROR, "Cannot make non abstract method %s::%s() abstract in class %s", ZEND_FN_SCOPE_NAME(parent), ZSTR_VAL(child->common.function_name), ZEND_FN_SCOPE_NAME(child));
}
}

if (parent_flags & ZEND_ACC_CHANGED) {
Expand All @@ -545,7 +550,7 @@ static void do_inheritance_check_on_method(zend_function *child, zend_function *
/* Prevent derived classes from restricting access that was available in parent classes
*/
if (UNEXPECTED((child_flags & ZEND_ACC_PPP_MASK) > (parent_flags & ZEND_ACC_PPP_MASK))) {
zend_error_noreturn(E_COMPILE_ERROR, "Access level to %s::%s() must be %s (as in class %s)%s", ZEND_FN_SCOPE_NAME(child), ZSTR_VAL(child->common.function_name), zend_visibility_string(parent_flags), ZEND_FN_SCOPE_NAME(parent), (parent_flags&ZEND_ACC_PUBLIC) ? "" : " or weaker");
zend_error_noreturn(E_COMPILE_ERROR, "Access level to %s::%s() must be %s (as in class %s)%s", ZEND_FN_SCOPE_NAME(child), ZSTR_VAL(child->common.function_name), zend_visibility_string(parent_flags & ZEND_ACC_PPP_MASK), ZEND_FN_SCOPE_NAME(parent), (parent_flags & ZEND_ACC_PUBLIC) ? "" : " or weaker");
} else if (((child_flags & ZEND_ACC_PPP_MASK) < (parent_flags & ZEND_ACC_PPP_MASK))
&& ((parent_flags & ZEND_ACC_PPP_MASK) & ZEND_ACC_PRIVATE)) {
child->common.fn_flags |= ZEND_ACC_CHANGED;
Expand Down Expand Up @@ -618,7 +623,7 @@ static void do_inherit_property(zend_property_info *parent_info, zend_string *ke
}

if (UNEXPECTED((child_info->flags & ZEND_ACC_PPP_MASK) > (parent_info->flags & ZEND_ACC_PPP_MASK))) {
zend_error_noreturn(E_COMPILE_ERROR, "Access level to %s::$%s must be %s (as in class %s)%s", ZSTR_VAL(ce->name), ZSTR_VAL(key), zend_visibility_string(parent_info->flags), ZSTR_VAL(ce->parent->name), (parent_info->flags&ZEND_ACC_PUBLIC) ? "" : " or weaker");
zend_error_noreturn(E_COMPILE_ERROR, "Access level to %s::$%s must be %s (as in class %s)%s", ZSTR_VAL(ce->name), ZSTR_VAL(key), zend_visibility_string(parent_info->flags & ZEND_ACC_PPP_MASK), ZSTR_VAL(ce->parent->name), (parent_info->flags&ZEND_ACC_PUBLIC) ? "" : " or weaker");
} else if ((child_info->flags & ZEND_ACC_STATIC) == 0) {
int parent_num = OBJ_PROP_TO_NUM(parent_info->offset);
int child_num = OBJ_PROP_TO_NUM(child_info->offset);
Expand Down Expand Up @@ -1088,7 +1093,7 @@ static void zend_add_trait_method(zend_class_entry *ce, const char *name, zend_s
ZSTR_VAL(zend_get_function_declaration(fn)),
ZSTR_VAL(zend_get_function_declaration(existing_fn)));
}
} else if (fn->common.fn_flags & ZEND_ACC_ABSTRACT) {
} else if ((fn->common.fn_flags & (ZEND_ACC_ABSTRACT | ZEND_ACC_CHANGED)) == ZEND_ACC_ABSTRACT) {
/* Make sure the abstract declaration is compatible with previous declaration */
if (UNEXPECTED(!zend_traits_method_compatibility_check(existing_fn, fn))) {
zend_error_noreturn(E_COMPILE_ERROR, "Declaration of %s must be compatible with %s",
Expand All @@ -1112,7 +1117,7 @@ static void zend_add_trait_method(zend_class_entry *ce, const char *name, zend_s
ZSTR_VAL(zend_get_function_declaration(fn)),
ZSTR_VAL(zend_get_function_declaration(existing_fn)));
}
} else if (fn->common.fn_flags & ZEND_ACC_ABSTRACT) {
} else if ((fn->common.fn_flags & (ZEND_ACC_ABSTRACT | ZEND_ACC_CHANGED)) == ZEND_ACC_ABSTRACT) {
/* Make sure the abstract declaration is compatible with previous declaration */
if (UNEXPECTED(!zend_traits_method_compatibility_check(existing_fn, fn))) {
zend_error_noreturn(E_COMPILE_ERROR, "Declaration of %s must be compatible with %s",
Expand Down Expand Up @@ -1143,14 +1148,18 @@ static void zend_add_trait_method(zend_class_entry *ce, const char *name, zend_s
new_fn = zend_arena_alloc(&CG(arena), sizeof(zend_op_array));
memcpy(new_fn, fn, sizeof(zend_op_array));
fn = zend_hash_update_ptr(&ce->function_table, key, new_fn);

if (fn->common.fn_flags & ZEND_ACC_CHANGED) {
fn->common.fn_flags &= ZEND_ACC_CHANGED ^ ~ZEND_ACC_ABSTRACT;
}

zend_add_magic_methods(ce, key, fn);
}
/* }}} */

static void zend_fixup_trait_method(zend_function *fn, zend_class_entry *ce) /* {{{ */
{
if ((fn->common.scope->ce_flags & ZEND_ACC_TRAIT) == ZEND_ACC_TRAIT) {

fn->common.scope = ce;

if (fn->common.fn_flags & ZEND_ACC_ABSTRACT) {
Expand Down Expand Up @@ -1491,7 +1500,7 @@ static void zend_do_traits_property_binding(zend_class_entry *ce) /* {{{ */
zend_string_release(coliding_prop->name);
if (coliding_prop->doc_comment) {
zend_string_release(coliding_prop->doc_comment);
}
}
zend_hash_del(&ce->properties_info, prop_name);
flags |= ZEND_ACC_CHANGED;
} else {
Expand Down Expand Up @@ -1537,9 +1546,7 @@ static void zend_do_traits_property_binding(zend_class_entry *ce) /* {{{ */
if (Z_REFCOUNTED_P(prop_value)) Z_ADDREF_P(prop_value);

doc_comment = property_info->doc_comment ? zend_string_copy(property_info->doc_comment) : NULL;
zend_declare_property_ex(ce, prop_name,
prop_value, flags,
doc_comment);
zend_declare_property_ex(ce, prop_name, prop_value, flags & ~ZEND_ACC_ABSTRACT, doc_comment);
zend_string_release(prop_name);
} ZEND_HASH_FOREACH_END();
}
Expand Down
4 changes: 3 additions & 1 deletion Zend/zend_object_handlers.c
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,9 @@ static void zend_std_call_issetter(zval *object, zval *member, zval *retval) /*

static zend_always_inline int zend_verify_property_access(zend_property_info *property_info, zend_class_entry *ce) /* {{{ */
{
if (property_info->flags & ZEND_ACC_PUBLIC) {
if (UNEXPECTED(property_info->flags & ZEND_ACC_ABSTRACT)) {
return 0;
} else if (property_info->flags & ZEND_ACC_PUBLIC) {
return 1;
} else if (property_info->flags & ZEND_ACC_PRIVATE) {
return (ce == EG(scope) || property_info->ce == EG(scope));
Expand Down