From: Nikita Popov Date: Mon, 20 Jul 2020 09:05:33 +0000 (+0200) Subject: Unify magic method visibility check X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=149029b9d6411c8d8391d125891ac7f770ba277a;p=php Unify magic method visibility check This was missing entirely for the internal function case. --- diff --git a/Zend/tests/magic_methods_007.phpt b/Zend/tests/magic_methods_007.phpt index 86642b4ecb..5e0379060b 100644 --- a/Zend/tests/magic_methods_007.phpt +++ b/Zend/tests/magic_methods_007.phpt @@ -9,6 +9,4 @@ abstract class b { ?> --EXPECTF-- -Warning: The magic method b::__set() must have public visibility in %s on line %d - Fatal error: Method b::__set() must take exactly 2 arguments in %s on line %d diff --git a/Zend/tests/magic_methods_010.phpt b/Zend/tests/magic_methods_010.phpt index fda1cc1e10..3562189c34 100644 --- a/Zend/tests/magic_methods_010.phpt +++ b/Zend/tests/magic_methods_010.phpt @@ -10,6 +10,4 @@ class a { ?> --EXPECTF-- -Warning: The magic method a::__toString() must have public visibility in %s on line %d - Fatal error: Method a::__toString() cannot take arguments in %s on line %d diff --git a/Zend/zend_API.c b/Zend/zend_API.c index 734c792fd5..14f7cd695a 100644 --- a/Zend/zend_API.c +++ b/Zend/zend_API.c @@ -2049,6 +2049,16 @@ static void zend_check_magic_method_static( } } +static void zend_check_magic_method_public( + const char *name, const zend_class_entry *ce, const zend_function *fptr, int error_type) +{ + // TODO: Remove this warning after adding proper visibility handling. + if (!(fptr->common.fn_flags & ZEND_ACC_PUBLIC)) { + zend_error(E_WARNING, "The magic method %s::%s() must have public visibility", + ZSTR_VAL(ce->name), name); + } +} + static void zend_check_magic_method_no_return_type( const char *name, const zend_class_entry *ce, const zend_function *fptr, int error_type) { @@ -2079,38 +2089,50 @@ ZEND_API void zend_check_magic_method_implementation(const zend_class_entry *ce, } else if (zend_string_equals_literal(lcname, ZEND_GET_FUNC_NAME)) { zend_check_magic_method_args(1, "__get", ce, fptr, error_type); zend_check_magic_method_non_static("__get", ce, fptr, error_type); + zend_check_magic_method_public("__get", ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, ZEND_SET_FUNC_NAME)) { zend_check_magic_method_args(2, "__set", ce, fptr, error_type); zend_check_magic_method_non_static("__set", ce, fptr, error_type); + zend_check_magic_method_public("__set", ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, ZEND_UNSET_FUNC_NAME)) { zend_check_magic_method_args(1, "__unset", ce, fptr, error_type); zend_check_magic_method_non_static("__unset", ce, fptr, error_type); + zend_check_magic_method_public("__unset", ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, ZEND_ISSET_FUNC_NAME)) { zend_check_magic_method_args(1, "__isset", ce, fptr, error_type); zend_check_magic_method_non_static("__isset", ce, fptr, error_type); + zend_check_magic_method_public("__isset", ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, ZEND_CALL_FUNC_NAME)) { zend_check_magic_method_args(2, "__call", ce, fptr, error_type); zend_check_magic_method_non_static("__call", ce, fptr, error_type); + zend_check_magic_method_public("__call", ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, ZEND_CALLSTATIC_FUNC_NAME)) { zend_check_magic_method_args(2, "__callStatic", ce, fptr, error_type); zend_check_magic_method_static("__callStatic", ce, fptr, error_type); + zend_check_magic_method_public("__callStatic", ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, ZEND_TOSTRING_FUNC_NAME)) { zend_check_magic_method_args(0, "__toString", ce, fptr, error_type); zend_check_magic_method_non_static("__toString", ce, fptr, error_type); + zend_check_magic_method_public("__toString", ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, ZEND_DEBUGINFO_FUNC_NAME)) { zend_check_magic_method_args(0, "__debugInfo", ce, fptr, error_type); zend_check_magic_method_non_static("__debugInfo", ce, fptr, error_type); + zend_check_magic_method_public("__debugInfo", ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, "__serialize")) { zend_check_magic_method_args(0, "__serialize", ce, fptr, error_type); zend_check_magic_method_non_static("__serialize", ce, fptr, error_type); + zend_check_magic_method_public("__serialize", ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, "__unserialize")) { zend_check_magic_method_args(1, "__unserialize", ce, fptr, error_type); zend_check_magic_method_non_static("__unserialize", ce, fptr, error_type); + zend_check_magic_method_public("__unserialize", ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, "__set_state")) { zend_check_magic_method_args(1, "__set_state", ce, fptr, error_type); zend_check_magic_method_static("__set_state", ce, fptr, error_type); + zend_check_magic_method_public("__set_state", ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, "__invoke")) { zend_check_magic_method_non_static("__invoke", ce, fptr, error_type); + zend_check_magic_method_public("__invoke", ce, fptr, error_type); } } /* }}} */ @@ -2294,11 +2316,9 @@ ZEND_API int zend_register_functions(zend_class_entry *scope, const zend_functio reg_function = NULL; } else if (zend_string_equals_literal(lowercase_name, ZEND_CONSTRUCTOR_FUNC_NAME)) { ctor = reg_function; + ctor->common.fn_flags |= ZEND_ACC_CTOR; } else if (zend_string_equals_literal(lowercase_name, ZEND_DESTRUCTOR_FUNC_NAME)) { dtor = reg_function; - if (internal_function->num_args) { - zend_error(error_type, "Destructor %s::%s() cannot take arguments", ZSTR_VAL(scope->name), ptr->fname); - } } else if (zend_string_equals_literal(lowercase_name, ZEND_CLONE_FUNC_NAME)) { clone = reg_function; } else if (zend_string_equals_literal(lowercase_name, ZEND_CALL_FUNC_NAME)) { @@ -2368,9 +2388,6 @@ ZEND_API int zend_register_functions(zend_class_entry *scope, const zend_functio scope->__debugInfo = __debugInfo; scope->__serialize = __serialize; scope->__unserialize = __unserialize; - if (ctor) { - ctor->common.fn_flags |= ZEND_ACC_CTOR; - } efree((char*)lc_class_name); } return SUCCESS; diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index c2d0a117f2..cabc1087a7 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -6443,16 +6443,6 @@ static void zend_compile_implicit_closure_uses(closure_info *info) ZEND_HASH_FOREACH_END(); } -static void zend_check_magic_method_attr(uint32_t attr, zend_class_entry *ce, const char* method) /* {{{ */ -{ - if (!(attr & ZEND_ACC_PUBLIC)) { - zend_error(E_WARNING, - "The magic method %s::%s() must have public visibility", - ZSTR_VAL(ce->name), method); - } -} -/* }}} */ - static void add_stringable_interface(zend_class_entry *ce) { for (uint32_t i = 0; i < ce->num_interfaces; i++) { if (zend_string_equals_literal(ce->interface_names[i].lc_name, "stringable")) { @@ -6523,49 +6513,36 @@ zend_string *zend_begin_method_decl(zend_op_array *op_array, zend_string *name, /* pass */ } else if (zend_string_equals_literal(lcname, ZEND_CONSTRUCTOR_FUNC_NAME)) { ce->constructor = (zend_function *) op_array; + ce->constructor->common.fn_flags |= ZEND_ACC_CTOR; } else if (zend_string_equals_literal(lcname, ZEND_DESTRUCTOR_FUNC_NAME)) { ce->destructor = (zend_function *) op_array; } else if (zend_string_equals_literal(lcname, ZEND_CLONE_FUNC_NAME)) { ce->clone = (zend_function *) op_array; } else if (zend_string_equals_literal(lcname, ZEND_CALL_FUNC_NAME)) { - zend_check_magic_method_attr(fn_flags, ce, "__call"); ce->__call = (zend_function *) op_array; } else if (zend_string_equals_literal(lcname, ZEND_CALLSTATIC_FUNC_NAME)) { - zend_check_magic_method_attr(fn_flags, ce, "__callStatic"); ce->__callstatic = (zend_function *) op_array; } else if (zend_string_equals_literal(lcname, ZEND_GET_FUNC_NAME)) { - zend_check_magic_method_attr(fn_flags, ce, "__get"); ce->__get = (zend_function *) op_array; ce->ce_flags |= ZEND_ACC_USE_GUARDS; } else if (zend_string_equals_literal(lcname, ZEND_SET_FUNC_NAME)) { - zend_check_magic_method_attr(fn_flags, ce, "__set"); ce->__set = (zend_function *) op_array; ce->ce_flags |= ZEND_ACC_USE_GUARDS; } else if (zend_string_equals_literal(lcname, ZEND_UNSET_FUNC_NAME)) { - zend_check_magic_method_attr(fn_flags, ce, "__unset"); ce->__unset = (zend_function *) op_array; ce->ce_flags |= ZEND_ACC_USE_GUARDS; } else if (zend_string_equals_literal(lcname, ZEND_ISSET_FUNC_NAME)) { - zend_check_magic_method_attr(fn_flags, ce, "__isset"); ce->__isset = (zend_function *) op_array; ce->ce_flags |= ZEND_ACC_USE_GUARDS; } else if (zend_string_equals_literal(lcname, ZEND_TOSTRING_FUNC_NAME)) { - zend_check_magic_method_attr(fn_flags, ce, "__toString"); ce->__tostring = (zend_function *) op_array; add_stringable_interface(ce); - } else if (zend_string_equals_literal(lcname, ZEND_INVOKE_FUNC_NAME)) { - zend_check_magic_method_attr(fn_flags, ce, "__invoke"); } else if (zend_string_equals_literal(lcname, ZEND_DEBUGINFO_FUNC_NAME)) { - zend_check_magic_method_attr(fn_flags, ce, "__debugInfo"); ce->__debugInfo = (zend_function *) op_array; } else if (zend_string_equals_literal(lcname, "__serialize")) { - zend_check_magic_method_attr(fn_flags, ce, "__serialize"); ce->__serialize = (zend_function *) op_array; } else if (zend_string_equals_literal(lcname, "__unserialize")) { - zend_check_magic_method_attr(fn_flags, ce, "__unserialize"); ce->__unserialize = (zend_function *) op_array; - } else if (zend_string_equals_literal(lcname, "__set_state")) { - zend_check_magic_method_attr(fn_flags, ce, "__set_state"); } return lcname; @@ -6740,6 +6717,7 @@ void zend_compile_func_decl(znode *result, zend_ast *ast, zend_bool toplevel) /* zend_compile_stmt(stmt_ast); if (is_method) { + CG(zend_lineno) = decl->start_lineno; zend_check_magic_method_implementation( CG(active_class_entry), (zend_function *) op_array, method_lcname, E_COMPILE_ERROR); zend_string_release_ex(method_lcname, 0); @@ -7159,10 +7137,6 @@ void zend_compile_class_decl(znode *result, zend_ast *ast, zend_bool toplevel) / /* Reset lineno for final opcodes and errors */ CG(zend_lineno) = ast->lineno; - if (ce->constructor) { - ce->constructor->common.fn_flags |= ZEND_ACC_CTOR; - } - if ((ce->ce_flags & (ZEND_ACC_IMPLICIT_ABSTRACT_CLASS|ZEND_ACC_INTERFACE|ZEND_ACC_TRAIT|ZEND_ACC_EXPLICIT_ABSTRACT_CLASS)) == ZEND_ACC_IMPLICIT_ABSTRACT_CLASS) { zend_verify_abstract_class(ce); }