Skip to content

Commit 149029b

Browse files
nikicnikic
committed
Unify magic method visibility check
This was missing entirely for the internal function case.
1 parent 9f0213f commit 149029b

File tree

4 files changed

+25
-38
lines changed

4 files changed

+25
-38
lines changed

‎Zend/tests/magic_methods_007.phpt

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,4 @@ abstract class b {
99

1010
?>
1111
--EXPECTF--
12-
Warning: The magic method b::__set() must have public visibility in %s on line %d
13-
1412
Fatal error: Method b::__set() must take exactly 2 arguments in %s on line %d

‎Zend/tests/magic_methods_010.phpt

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,4 @@ class a {
1010

1111
?>
1212
--EXPECTF--
13-
Warning: The magic method a::__toString() must have public visibility in %s on line %d
14-
1513
Fatal error: Method a::__toString() cannot take arguments in %s on line %d

‎Zend/zend_API.c

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2049,6 +2049,16 @@ static void zend_check_magic_method_static(
20492049
}
20502050
}
20512051

2052+
static void zend_check_magic_method_public(
2053+
const char *name, const zend_class_entry *ce, const zend_function *fptr, int error_type)
2054+
{
2055+
// TODO: Remove this warning after adding proper visibility handling.
2056+
if (!(fptr->common.fn_flags & ZEND_ACC_PUBLIC)) {
2057+
zend_error(E_WARNING, "The magic method %s::%s() must have public visibility",
2058+
ZSTR_VAL(ce->name), name);
2059+
}
2060+
}
2061+
20522062
static void zend_check_magic_method_no_return_type(
20532063
const char *name, const zend_class_entry *ce, const zend_function *fptr, int error_type)
20542064
{
@@ -2079,38 +2089,50 @@ ZEND_API void zend_check_magic_method_implementation(const zend_class_entry *ce,
20792089
} else if (zend_string_equals_literal(lcname, ZEND_GET_FUNC_NAME)) {
20802090
zend_check_magic_method_args(1, "__get", ce, fptr, error_type);
20812091
zend_check_magic_method_non_static("__get", ce, fptr, error_type);
2092+
zend_check_magic_method_public("__get", ce, fptr, error_type);
20822093
} else if (zend_string_equals_literal(lcname, ZEND_SET_FUNC_NAME)) {
20832094
zend_check_magic_method_args(2, "__set", ce, fptr, error_type);
20842095
zend_check_magic_method_non_static("__set", ce, fptr, error_type);
2096+
zend_check_magic_method_public("__set", ce, fptr, error_type);
20852097
} else if (zend_string_equals_literal(lcname, ZEND_UNSET_FUNC_NAME)) {
20862098
zend_check_magic_method_args(1, "__unset", ce, fptr, error_type);
20872099
zend_check_magic_method_non_static("__unset", ce, fptr, error_type);
2100+
zend_check_magic_method_public("__unset", ce, fptr, error_type);
20882101
} else if (zend_string_equals_literal(lcname, ZEND_ISSET_FUNC_NAME)) {
20892102
zend_check_magic_method_args(1, "__isset", ce, fptr, error_type);
20902103
zend_check_magic_method_non_static("__isset", ce, fptr, error_type);
2104+
zend_check_magic_method_public("__isset", ce, fptr, error_type);
20912105
} else if (zend_string_equals_literal(lcname, ZEND_CALL_FUNC_NAME)) {
20922106
zend_check_magic_method_args(2, "__call", ce, fptr, error_type);
20932107
zend_check_magic_method_non_static("__call", ce, fptr, error_type);
2108+
zend_check_magic_method_public("__call", ce, fptr, error_type);
20942109
} else if (zend_string_equals_literal(lcname, ZEND_CALLSTATIC_FUNC_NAME)) {
20952110
zend_check_magic_method_args(2, "__callStatic", ce, fptr, error_type);
20962111
zend_check_magic_method_static("__callStatic", ce, fptr, error_type);
2112+
zend_check_magic_method_public("__callStatic", ce, fptr, error_type);
20972113
} else if (zend_string_equals_literal(lcname, ZEND_TOSTRING_FUNC_NAME)) {
20982114
zend_check_magic_method_args(0, "__toString", ce, fptr, error_type);
20992115
zend_check_magic_method_non_static("__toString", ce, fptr, error_type);
2116+
zend_check_magic_method_public("__toString", ce, fptr, error_type);
21002117
} else if (zend_string_equals_literal(lcname, ZEND_DEBUGINFO_FUNC_NAME)) {
21012118
zend_check_magic_method_args(0, "__debugInfo", ce, fptr, error_type);
21022119
zend_check_magic_method_non_static("__debugInfo", ce, fptr, error_type);
2120+
zend_check_magic_method_public("__debugInfo", ce, fptr, error_type);
21032121
} else if (zend_string_equals_literal(lcname, "__serialize")) {
21042122
zend_check_magic_method_args(0, "__serialize", ce, fptr, error_type);
21052123
zend_check_magic_method_non_static("__serialize", ce, fptr, error_type);
2124+
zend_check_magic_method_public("__serialize", ce, fptr, error_type);
21062125
} else if (zend_string_equals_literal(lcname, "__unserialize")) {
21072126
zend_check_magic_method_args(1, "__unserialize", ce, fptr, error_type);
21082127
zend_check_magic_method_non_static("__unserialize", ce, fptr, error_type);
2128+
zend_check_magic_method_public("__unserialize", ce, fptr, error_type);
21092129
} else if (zend_string_equals_literal(lcname, "__set_state")) {
21102130
zend_check_magic_method_args(1, "__set_state", ce, fptr, error_type);
21112131
zend_check_magic_method_static("__set_state", ce, fptr, error_type);
2132+
zend_check_magic_method_public("__set_state", ce, fptr, error_type);
21122133
} else if (zend_string_equals_literal(lcname, "__invoke")) {
21132134
zend_check_magic_method_non_static("__invoke", ce, fptr, error_type);
2135+
zend_check_magic_method_public("__invoke", ce, fptr, error_type);
21142136
}
21152137
}
21162138
/* }}} */
@@ -2294,11 +2316,9 @@ ZEND_API int zend_register_functions(zend_class_entry *scope, const zend_functio
22942316
reg_function = NULL;
22952317
} else if (zend_string_equals_literal(lowercase_name, ZEND_CONSTRUCTOR_FUNC_NAME)) {
22962318
ctor = reg_function;
2319+
ctor->common.fn_flags |= ZEND_ACC_CTOR;
22972320
} else if (zend_string_equals_literal(lowercase_name, ZEND_DESTRUCTOR_FUNC_NAME)) {
22982321
dtor = reg_function;
2299-
if (internal_function->num_args) {
2300-
zend_error(error_type, "Destructor %s::%s() cannot take arguments", ZSTR_VAL(scope->name), ptr->fname);
2301-
}
23022322
} else if (zend_string_equals_literal(lowercase_name, ZEND_CLONE_FUNC_NAME)) {
23032323
clone = reg_function;
23042324
} 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
23682388
scope->__debugInfo = __debugInfo;
23692389
scope->__serialize = __serialize;
23702390
scope->__unserialize = __unserialize;
2371-
if (ctor) {
2372-
ctor->common.fn_flags |= ZEND_ACC_CTOR;
2373-
}
23742391
efree((char*)lc_class_name);
23752392
}
23762393
return SUCCESS;

‎Zend/zend_compile.c

Lines changed: 2 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -6443,16 +6443,6 @@ static void zend_compile_implicit_closure_uses(closure_info *info)
64436443
ZEND_HASH_FOREACH_END();
64446444
}
64456445

6446-
static void zend_check_magic_method_attr(uint32_t attr, zend_class_entry *ce, const char* method) /* {{{ */
6447-
{
6448-
if (!(attr & ZEND_ACC_PUBLIC)) {
6449-
zend_error(E_WARNING,
6450-
"The magic method %s::%s() must have public visibility",
6451-
ZSTR_VAL(ce->name), method);
6452-
}
6453-
}
6454-
/* }}} */
6455-
64566446
static void add_stringable_interface(zend_class_entry *ce) {
64576447
for (uint32_t i = 0; i < ce->num_interfaces; i++) {
64586448
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,
65236513
/* pass */
65246514
} else if (zend_string_equals_literal(lcname, ZEND_CONSTRUCTOR_FUNC_NAME)) {
65256515
ce->constructor = (zend_function *) op_array;
6516+
ce->constructor->common.fn_flags |= ZEND_ACC_CTOR;
65266517
} else if (zend_string_equals_literal(lcname, ZEND_DESTRUCTOR_FUNC_NAME)) {
65276518
ce->destructor = (zend_function *) op_array;
65286519
} else if (zend_string_equals_literal(lcname, ZEND_CLONE_FUNC_NAME)) {
65296520
ce->clone = (zend_function *) op_array;
65306521
} else if (zend_string_equals_literal(lcname, ZEND_CALL_FUNC_NAME)) {
6531-
zend_check_magic_method_attr(fn_flags, ce, "__call");
65326522
ce->__call = (zend_function *) op_array;
65336523
} else if (zend_string_equals_literal(lcname, ZEND_CALLSTATIC_FUNC_NAME)) {
6534-
zend_check_magic_method_attr(fn_flags, ce, "__callStatic");
65356524
ce->__callstatic = (zend_function *) op_array;
65366525
} else if (zend_string_equals_literal(lcname, ZEND_GET_FUNC_NAME)) {
6537-
zend_check_magic_method_attr(fn_flags, ce, "__get");
65386526
ce->__get = (zend_function *) op_array;
65396527
ce->ce_flags |= ZEND_ACC_USE_GUARDS;
65406528
} else if (zend_string_equals_literal(lcname, ZEND_SET_FUNC_NAME)) {
6541-
zend_check_magic_method_attr(fn_flags, ce, "__set");
65426529
ce->__set = (zend_function *) op_array;
65436530
ce->ce_flags |= ZEND_ACC_USE_GUARDS;
65446531
} else if (zend_string_equals_literal(lcname, ZEND_UNSET_FUNC_NAME)) {
6545-
zend_check_magic_method_attr(fn_flags, ce, "__unset");
65466532
ce->__unset = (zend_function *) op_array;
65476533
ce->ce_flags |= ZEND_ACC_USE_GUARDS;
65486534
} else if (zend_string_equals_literal(lcname, ZEND_ISSET_FUNC_NAME)) {
6549-
zend_check_magic_method_attr(fn_flags, ce, "__isset");
65506535
ce->__isset = (zend_function *) op_array;
65516536
ce->ce_flags |= ZEND_ACC_USE_GUARDS;
65526537
} else if (zend_string_equals_literal(lcname, ZEND_TOSTRING_FUNC_NAME)) {
6553-
zend_check_magic_method_attr(fn_flags, ce, "__toString");
65546538
ce->__tostring = (zend_function *) op_array;
65556539
add_stringable_interface(ce);
6556-
} else if (zend_string_equals_literal(lcname, ZEND_INVOKE_FUNC_NAME)) {
6557-
zend_check_magic_method_attr(fn_flags, ce, "__invoke");
65586540
} else if (zend_string_equals_literal(lcname, ZEND_DEBUGINFO_FUNC_NAME)) {
6559-
zend_check_magic_method_attr(fn_flags, ce, "__debugInfo");
65606541
ce->__debugInfo = (zend_function *) op_array;
65616542
} else if (zend_string_equals_literal(lcname, "__serialize")) {
6562-
zend_check_magic_method_attr(fn_flags, ce, "__serialize");
65636543
ce->__serialize = (zend_function *) op_array;
65646544
} else if (zend_string_equals_literal(lcname, "__unserialize")) {
6565-
zend_check_magic_method_attr(fn_flags, ce, "__unserialize");
65666545
ce->__unserialize = (zend_function *) op_array;
6567-
} else if (zend_string_equals_literal(lcname, "__set_state")) {
6568-
zend_check_magic_method_attr(fn_flags, ce, "__set_state");
65696546
}
65706547

65716548
return lcname;
@@ -6740,6 +6717,7 @@ void zend_compile_func_decl(znode *result, zend_ast *ast, zend_bool toplevel) /*
67406717
zend_compile_stmt(stmt_ast);
67416718

67426719
if (is_method) {
6720+
CG(zend_lineno) = decl->start_lineno;
67436721
zend_check_magic_method_implementation(
67446722
CG(active_class_entry), (zend_function *) op_array, method_lcname, E_COMPILE_ERROR);
67456723
zend_string_release_ex(method_lcname, 0);
@@ -7159,10 +7137,6 @@ void zend_compile_class_decl(znode *result, zend_ast *ast, zend_bool toplevel) /
71597137
/* Reset lineno for final opcodes and errors */
71607138
CG(zend_lineno) = ast->lineno;
71617139

7162-
if (ce->constructor) {
7163-
ce->constructor->common.fn_flags |= ZEND_ACC_CTOR;
7164-
}
7165-
71667140
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) {
71677141
zend_verify_abstract_class(ce);
71687142
}

0 commit comments

Comments
 (0)