Skip to content

Commit 45d948f

Browse files
TimWollaTimWolla
authored
Zend: Add zend_check_method_accessible() to DRY method visibility checks (#18995)
* Zend: Add `zend_check_method_accessible()` to DRY method visibility checks * Zend: Add assertions verifying flags didn't change before `zend_check_method_accessible()` * Try `zend_always_inline` for `zend_check_method_accessible`
1 parent 9225cb4 commit 45d948f

File tree

8 files changed

+90
-139
lines changed

8 files changed

+90
-139
lines changed

‎Zend/zend_API.c‎

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4021,13 +4021,11 @@ static zend_always_inline bool zend_is_callable_check_func(zval *callable, zend_
40214021
((fcc->object && fcc->calling_scope->__call) ||
40224022
(!fcc->object && fcc->calling_scope->__callstatic)))) {
40234023
scope = get_scope(frame);
4024-
if (fcc->function_handler->common.scope != scope) {
4025-
if ((fcc->function_handler->common.fn_flags & ZEND_ACC_PRIVATE)
4026-
|| !zend_check_protected(zend_get_function_root_class(fcc->function_handler), scope)) {
4027-
retval = 0;
4028-
fcc->function_handler = NULL;
4029-
goto get_function_via_handler;
4030-
}
4024+
ZEND_ASSERT(!(fcc->function_handler->common.fn_flags & ZEND_ACC_PUBLIC));
4025+
if (!zend_check_method_accessible(fcc->function_handler, scope)) {
4026+
retval = 0;
4027+
fcc->function_handler = NULL;
4028+
goto get_function_via_handler;
40314029
}
40324030
}
40334031
} else {
@@ -4086,17 +4084,15 @@ static zend_always_inline bool zend_is_callable_check_func(zval *callable, zend_
40864084
if (retval
40874085
&& !(fcc->function_handler->common.fn_flags & ZEND_ACC_PUBLIC)) {
40884086
scope = get_scope(frame);
4089-
if (fcc->function_handler->common.scope != scope) {
4090-
if ((fcc->function_handler->common.fn_flags & ZEND_ACC_PRIVATE)
4091-
|| (!zend_check_protected(zend_get_function_root_class(fcc->function_handler), scope))) {
4092-
if (error) {
4093-
if (*error) {
4094-
efree(*error);
4095-
}
4096-
zend_spprintf(error, 0, "cannot access %s method %s::%s()", zend_visibility_string(fcc->function_handler->common.fn_flags), ZSTR_VAL(fcc->calling_scope->name), ZSTR_VAL(fcc->function_handler->common.function_name));
4087+
ZEND_ASSERT(!(fcc->function_handler->common.fn_flags & ZEND_ACC_PUBLIC));
4088+
if (!zend_check_method_accessible(fcc->function_handler, scope)) {
4089+
if (error) {
4090+
if (*error) {
4091+
efree(*error);
40974092
}
4098-
retval = 0;
4093+
zend_spprintf(error, 0, "cannot access %s method %s::%s()", zend_visibility_string(fcc->function_handler->common.fn_flags), ZSTR_VAL(fcc->calling_scope->name), ZSTR_VAL(fcc->function_handler->common.function_name));
40994094
}
4095+
retval = 0;
41004096
}
41014097
}
41024098
}

‎Zend/zend_ast.c‎

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1098,21 +1098,14 @@ ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate_inner(
10981098
} else {
10991099
fptr = zend_hash_find_ptr_lc(&ce->function_table, method_name);
11001100
if (fptr) {
1101-
if (!(fptr->common.fn_flags & ZEND_ACC_PUBLIC)) {
1102-
if (UNEXPECTED(fptr->common.scope != scope)) {
1103-
if (
1104-
UNEXPECTED(fptr->op_array.fn_flags & ZEND_ACC_PRIVATE)
1105-
|| UNEXPECTED(!zend_check_protected(zend_get_function_root_class(fptr), scope))
1106-
) {
1107-
if (ce->__callstatic) {
1108-
zend_throw_error(NULL, "Creating a callable for the magic __callStatic() method is not supported in constant expressions");
1109-
} else {
1110-
zend_bad_method_call(fptr, method_name, scope);
1111-
}
1112-
1113-
return FAILURE;
1114-
}
1101+
if (!zend_check_method_accessible(fptr, scope)) {
1102+
if (ce->__callstatic) {
1103+
zend_throw_error(NULL, "Creating a callable for the magic __callStatic() method is not supported in constant expressions");
1104+
} else {
1105+
zend_bad_method_call(fptr, method_name, scope);
11151106
}
1107+
1108+
return FAILURE;
11161109
}
11171110
} else {
11181111
if (ce->__callstatic) {

‎Zend/zend_builtin_functions.c‎

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -89,14 +89,9 @@ ZEND_FUNCTION(clone)
8989
RETURN_THROWS();
9090
}
9191

92-
if (clone && !(clone->common.fn_flags & ZEND_ACC_PUBLIC)) {
93-
if (clone->common.scope != scope) {
94-
if (UNEXPECTED(clone->common.fn_flags & ZEND_ACC_PRIVATE)
95-
|| UNEXPECTED(!zend_check_protected(zend_get_function_root_class(clone), scope))) {
96-
zend_bad_method_call(clone, clone->common.function_name, scope);
97-
RETURN_THROWS();
98-
}
99-
}
92+
if (clone && !zend_check_method_accessible(clone, scope)) {
93+
zend_bad_method_call(clone, clone->common.function_name, scope);
94+
RETURN_THROWS();
10095
}
10196

10297
zend_object *cloned;
@@ -953,13 +948,7 @@ ZEND_FUNCTION(get_class_methods)
953948
scope = zend_get_executed_scope();
954949

955950
ZEND_HASH_MAP_FOREACH_PTR(&ce->function_table, mptr) {
956-
if ((mptr->common.fn_flags & ZEND_ACC_PUBLIC)
957-
|| (scope &&
958-
(((mptr->common.fn_flags & ZEND_ACC_PROTECTED) &&
959-
zend_check_protected(mptr->common.scope, scope))
960-
|| ((mptr->common.fn_flags & ZEND_ACC_PRIVATE) &&
961-
scope == mptr->common.scope)))
962-
) {
951+
if (zend_check_method_accessible(mptr, scope)) {
963952
ZVAL_STR_COPY(&method_name, mptr->common.function_name);
964953
zend_hash_next_index_insert_new(Z_ARRVAL_P(return_value), &method_name);
965954
}

‎Zend/zend_object_handlers.c‎

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1949,17 +1949,15 @@ ZEND_API zend_function *zend_std_get_static_method(zend_class_entry *ce, zend_st
19491949
zval *func = zend_hash_find(&ce->function_table, lc_function_name);
19501950
if (EXPECTED(func)) {
19511951
fbc = Z_FUNC_P(func);
1952-
if (!(fbc->op_array.fn_flags & ZEND_ACC_PUBLIC)) {
1952+
if (!(fbc->common.fn_flags & ZEND_ACC_PUBLIC)) {
19531953
zend_class_entry *scope = zend_get_executed_scope();
1954-
if (UNEXPECTED(fbc->common.scope != scope)) {
1955-
if (UNEXPECTED(fbc->op_array.fn_flags & ZEND_ACC_PRIVATE)
1956-
|| UNEXPECTED(!zend_check_protected(zend_get_function_root_class(fbc), scope))) {
1957-
zend_function *fallback_fbc = get_static_method_fallback(ce, function_name);
1958-
if (!fallback_fbc) {
1959-
zend_bad_method_call(fbc, function_name, scope);
1960-
}
1961-
fbc = fallback_fbc;
1954+
ZEND_ASSERT(!(fbc->common.fn_flags & ZEND_ACC_PUBLIC));
1955+
if (!zend_check_method_accessible(fbc, scope)) {
1956+
zend_function *fallback_fbc = get_static_method_fallback(ce, function_name);
1957+
if (!fallback_fbc) {
1958+
zend_bad_method_call(fbc, function_name, scope);
19621959
}
1960+
fbc = fallback_fbc;
19631961
}
19641962
}
19651963
} else {
@@ -2115,15 +2113,13 @@ ZEND_API zend_function *zend_std_get_constructor(zend_object *zobj) /* {{{ */
21152113
zend_function *constructor = zobj->ce->constructor;
21162114

21172115
if (constructor) {
2118-
if (UNEXPECTED(!(constructor->op_array.fn_flags & ZEND_ACC_PUBLIC))) {
2116+
if (UNEXPECTED(!(constructor->common.fn_flags & ZEND_ACC_PUBLIC))) {
21192117
zend_class_entry *scope = get_fake_or_executed_scope();
2120-
if (UNEXPECTED(constructor->common.scope != scope)) {
2121-
if (UNEXPECTED(constructor->op_array.fn_flags & ZEND_ACC_PRIVATE)
2122-
|| UNEXPECTED(!zend_check_protected(zend_get_function_root_class(constructor), scope))) {
2123-
zend_bad_constructor_call(constructor, scope);
2124-
zend_object_store_ctor_failed(zobj);
2125-
constructor = NULL;
2126-
}
2118+
ZEND_ASSERT(!(constructor->common.fn_flags & ZEND_ACC_PUBLIC));
2119+
if (!zend_check_method_accessible(constructor, scope)) {
2120+
zend_bad_constructor_call(constructor, scope);
2121+
zend_object_store_ctor_failed(zobj);
2122+
constructor = NULL;
21272123
}
21282124
}
21292125
}

‎Zend/zend_objects.c‎

Lines changed: 16 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -123,49 +123,25 @@ ZEND_API void zend_objects_destroy_object(zend_object *object)
123123
zend_object *old_exception;
124124
const zend_op *old_opline_before_exception;
125125

126-
if (destructor->op_array.fn_flags & (ZEND_ACC_PRIVATE|ZEND_ACC_PROTECTED)) {
127-
if (destructor->op_array.fn_flags & ZEND_ACC_PRIVATE) {
128-
/* Ensure that if we're calling a private function, we're allowed to do so.
129-
*/
130-
if (EG(current_execute_data)) {
131-
zend_class_entry *scope = zend_get_executed_scope();
132-
133-
if (object->ce != scope) {
134-
zend_throw_error(NULL,
135-
"Call to private %s::__destruct() from %s%s",
136-
ZSTR_VAL(object->ce->name),
137-
scope ? "scope " : "global scope",
138-
scope ? ZSTR_VAL(scope->name) : ""
139-
);
140-
return;
141-
}
142-
} else {
143-
zend_error(E_WARNING,
144-
"Call to private %s::__destruct() from global scope during shutdown ignored",
145-
ZSTR_VAL(object->ce->name));
126+
if (destructor->common.fn_flags & (ZEND_ACC_PRIVATE|ZEND_ACC_PROTECTED)) {
127+
if (EG(current_execute_data)) {
128+
zend_class_entry *scope = zend_get_executed_scope();
129+
/* Ensure that if we're calling a protected or private function, we're allowed to do so. */
130+
ZEND_ASSERT(!(destructor->common.fn_flags & ZEND_ACC_PUBLIC));
131+
if (!zend_check_method_accessible(destructor, scope)) {
132+
zend_throw_error(NULL,
133+
"Call to %s %s::__destruct() from %s%s",
134+
zend_visibility_string(destructor->common.fn_flags), ZSTR_VAL(object->ce->name),
135+
scope ? "scope " : "global scope",
136+
scope ? ZSTR_VAL(scope->name) : ""
137+
);
146138
return;
147139
}
148140
} else {
149-
/* Ensure that if we're calling a protected function, we're allowed to do so.
150-
*/
151-
if (EG(current_execute_data)) {
152-
zend_class_entry *scope = zend_get_executed_scope();
153-
154-
if (!zend_check_protected(zend_get_function_root_class(destructor), scope)) {
155-
zend_throw_error(NULL,
156-
"Call to protected %s::__destruct() from %s%s",
157-
ZSTR_VAL(object->ce->name),
158-
scope ? "scope " : "global scope",
159-
scope ? ZSTR_VAL(scope->name) : ""
160-
);
161-
return;
162-
}
163-
} else {
164-
zend_error(E_WARNING,
165-
"Call to protected %s::__destruct() from global scope during shutdown ignored",
166-
ZSTR_VAL(object->ce->name));
167-
return;
168-
}
141+
zend_error(E_WARNING,
142+
"Call to %s %s::__destruct() from global scope during shutdown ignored",
143+
zend_visibility_string(destructor->common.fn_flags), ZSTR_VAL(object->ce->name));
144+
return;
169145
}
170146
}
171147

‎Zend/zend_objects_API.h‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,5 +137,16 @@ static inline zend_property_info *zend_get_typed_property_info_for_slot(zend_obj
137137
return NULL;
138138
}
139139

140+
static zend_always_inline bool zend_check_method_accessible(const zend_function *fn, const zend_class_entry *scope)
141+
{
142+
if (!(fn->common.fn_flags & ZEND_ACC_PUBLIC)
143+
&& fn->common.scope != scope
144+
&& (UNEXPECTED(fn->common.fn_flags & ZEND_ACC_PRIVATE)
145+
|| UNEXPECTED(!zend_check_protected(zend_get_function_root_class(fn), scope)))) {
146+
return false;
147+
}
148+
149+
return true;
150+
}
140151

141152
#endif /* ZEND_OBJECTS_H */

‎Zend/zend_vm_def.h‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6043,14 +6043,12 @@ ZEND_VM_COLD_CONST_HANDLER(110, ZEND_CLONE, CONST|TMPVAR|UNUSED|THIS|CV, ANY)
60436043

60446044
if (clone && !(clone->common.fn_flags & ZEND_ACC_PUBLIC)) {
60456045
scope = EX(func)->op_array.scope;
6046-
if (clone->common.scope != scope) {
6047-
if (UNEXPECTED(clone->common.fn_flags & ZEND_ACC_PRIVATE)
6048-
|| UNEXPECTED(!zend_check_protected(zend_get_function_root_class(clone), scope))) {
6049-
zend_bad_method_call(clone, clone->common.function_name, scope);
6050-
FREE_OP1();
6051-
ZVAL_UNDEF(EX_VAR(opline->result.var));
6052-
HANDLE_EXCEPTION();
6053-
}
6046+
ZEND_ASSERT(!(clone->common.fn_flags & ZEND_ACC_PUBLIC));
6047+
if (!zend_check_method_accessible(clone, scope)) {
6048+
zend_bad_method_call(clone, clone->common.function_name, scope);
6049+
FREE_OP1();
6050+
ZVAL_UNDEF(EX_VAR(opline->result.var));
6051+
HANDLE_EXCEPTION();
60546052
}
60556053
}
60566054

‎Zend/zend_vm_execute.h‎

Lines changed: 21 additions & 29 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)