Fix GH-21362: ReflectionMethod::invoke/invokeArgs() rejects different Closure instances

[iliaal]

Summary

Fixes #21362

ReflectionMethod::invokeArgs() (and invoke()) for Closure::__invoke() incorrectly accepted any Closure object, not just the one the ReflectionMethod was created from.

Root cause

All Closure objects share a single zend_ce_closure class entry. The instanceof_function() check in reflection_method_invoke() compares class entries, so it always sees zend_ce_closure == zend_ce_closure and passes -- it cannot distinguish between different Closure instances.

Fix

Two changes to ext/reflection/php_reflection.c:

1. instantiate_reflection_method(): Store the original Closure object in intern->obj via ZVAL_OBJ_COPY() when reflecting Closure::__invoke(). Previously this was a no-op (/* do nothing, mptr already set */).

2. reflection_method_invoke(): After the existing instanceof_function check passes for Closures, add an identity check (Z_OBJ_P(object) != Z_OBJ(intern->obj)) to reject different Closure instances.

Test

ext/reflection/tests/gh21362.phpt covers:

• invokeArgs() with the correct Closure (should work)
• invokeArgs() with a different Closure (should throw ReflectionException)
• invoke() with a different Closure (should throw ReflectionException)

[TimWolla]

I did not test, but based on reading the diff I think this is not quite right, yet. Instead of comparing the objects, we should compare the inner op_array, because for:

$closures = [];

for ($i = 0; $i < 3; $i++) {
    $closures[] = function () use ($i) { return $i; };
}

$m = new ReflectionMethod(array_first($closures), '__invoke');
foreach ($closures as $closure) {
    var_dump($m->invoke($closure));
}

I expect this to successfully dump 0, 1, 2. All the closure instances are "of the same class". If that already works: Great, please just add that test case.

[iliaal]

Good catch -- the object identity check was too strict. Updated the comparison to use op_array.opcodes instead, so closures from the same source (e.g. created in a loop with different use bindings) are accepted, while closures from different source locations are still rejected.

Added the loop test case you suggested. All 515 reflection tests pass.

P.S. Thanks for the feedback, my php internals are a bit rusty 😆

[DanielEScherzer]

but they are an instance of the class, just not the correct instance - this error message should be improved