Skip to content

Commit a794fc9

Browse files
committed
Fix internal closure comparison and expand test coverage
The previous comparison (orig_func == given_func) could never match for internal closures since zend_get_closure_method_def() returns a pointer to each closure's embedded copy. Compare function_name and scope instead. Also handle the mixed user/internal type case explicitly. Add tests for: userland first-class callables, cloned internal closures, and cross-type (user vs internal) closure rejection.
1 parent 3d1bc50 commit a794fc9

File tree

2 files changed

+53
-9
lines changed

2 files changed

+53
-9
lines changed

ext/reflection/php_reflection.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3448,16 +3448,20 @@ static void reflection_method_invoke(INTERNAL_FUNCTION_PARAMETERS, int variadic)
34483448
* different signatures, so we must reject those. However, closures created
34493449
* from the same source (e.g. in a loop) share the same op_array and should
34503450
* be allowed. For user closures compare op_array.opcodes, for internal
3451-
* closures (e.g. var_dump(...)) compare the handler pointer. */
3451+
* closures (e.g. var_dump(...)) compare function_name and scope since
3452+
* zend_get_closure_method_def() returns a per-object embedded copy. */
34523453
if (obj_ce == zend_ce_closure && !Z_ISUNDEF(intern->obj)
34533454
&& Z_OBJ_P(object) != Z_OBJ(intern->obj)) {
34543455
const zend_function *orig_func = zend_get_closure_method_def(Z_OBJ(intern->obj));
34553456
const zend_function *given_func = zend_get_closure_method_def(Z_OBJ_P(object));
34563457
bool same_closure;
34573458
if (orig_func->type == ZEND_USER_FUNCTION && given_func->type == ZEND_USER_FUNCTION) {
34583459
same_closure = orig_func->op_array.opcodes == given_func->op_array.opcodes;
3460+
} else if (orig_func->type == ZEND_INTERNAL_FUNCTION && given_func->type == ZEND_INTERNAL_FUNCTION) {
3461+
same_closure = orig_func->common.function_name == given_func->common.function_name
3462+
&& orig_func->common.scope == given_func->common.scope;
34593463
} else {
3460-
same_closure = orig_func == given_func;
3464+
same_closure = false;
34613465
}
34623466
if (!same_closure) {
34633467
if (!variadic) {

ext/reflection/tests/gh21362.phpt

Lines changed: 47 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -44,20 +44,55 @@ foreach ($closures as $closure) {
4444
var_dump($m2->invoke($closure));
4545
}
4646

47+
// First-class callable of a userland function
48+
function my_func($x) { return "my_func: $x"; }
49+
function other_func($x) { return "other_func: $x"; }
50+
51+
$mf = my_func(...);
52+
$mf2 = my_func(...);
53+
$of = other_func(...);
54+
55+
$m3 = new ReflectionMethod($mf, '__invoke');
56+
var_dump($m3->invoke($mf, 'test'));
57+
var_dump($m3->invoke($mf2, 'test'));
58+
59+
try {
60+
$m3->invoke($of, 'test');
61+
echo "No exception thrown\n";
62+
} catch (ReflectionException $e) {
63+
echo $e->getMessage() . "\n";
64+
}
65+
4766
// Internal closures (first-class callable syntax) should also be validated
4867
$vd = var_dump(...);
4968
$pr = print_r(...);
5069

51-
$m3 = new ReflectionMethod($vd, '__invoke');
52-
$m3->invoke($vd, 'internal closure OK');
70+
$m4 = new ReflectionMethod($vd, '__invoke');
71+
$m4->invoke($vd, 'internal closure OK');
5372

54-
// Same internal closure, different instance - should work
55-
$vd2 = var_dump(...);
56-
$m3->invoke($vd2, 'same internal closure OK');
73+
// Cloned internal closure is a different object but same function - should work
74+
$vd2 = clone $vd;
75+
$m4->invoke($vd2, 'cloned internal closure OK');
5776

5877
// Different internal closure should throw
5978
try {
60-
$m3->invoke($pr, 'should not print');
79+
$m4->invoke($pr, 'should not print');
80+
echo "No exception thrown\n";
81+
} catch (ReflectionException $e) {
82+
echo $e->getMessage() . "\n";
83+
}
84+
85+
// Cross-type: userland Closure to internal Closure's invoke should throw
86+
try {
87+
$m4->invoke($c1, 'should not print');
88+
echo "No exception thrown\n";
89+
} catch (ReflectionException $e) {
90+
echo $e->getMessage() . "\n";
91+
}
92+
93+
// Cross-type: internal Closure to userland Closure's invoke should throw
94+
try {
95+
$m->invoke($vd, 'should not print');
6196
echo "No exception thrown\n";
6297
} catch (ReflectionException $e) {
6398
echo $e->getMessage() . "\n";
@@ -70,6 +105,11 @@ Given Closure is not the same as the reflected Closure
70105
int(0)
71106
int(1)
72107
int(2)
108+
string(13) "my_func: test"
109+
string(13) "my_func: test"
110+
Given Closure is not the same as the reflected Closure
73111
string(19) "internal closure OK"
74-
string(24) "same internal closure OK"
112+
string(26) "cloned internal closure OK"
113+
Given Closure is not the same as the reflected Closure
114+
Given Closure is not the same as the reflected Closure
75115
Given Closure is not the same as the reflected Closure

0 commit comments

Comments
 (0)