Implement #38992: invoke() and invokeArgs() static method calls should match

We don't want ReflectionMethod::invoke() to simply ignore its first argument,
if the method to invoke is a static method. Instead we match its ZPP with
that of ReflectionMethod::invokeArgs(). Furthermore, we apply the DRY
principle by factoring out the code to a common helper function to prevent
inadvertent future divergence of the implementations of both methods.

As can be seen from the necessity to adapt some test cases, this causes a
BC break for some pathological cases. Therefore we apply this patch to PHP
7.1 only, which is still in beta phase.
This commit is contained in:
Christoph M. Becker 2016-08-08 01:27:21 +02:00
parent e52c1f3ca9
commit f706897f33
8 changed files with 91 additions and 127 deletions

4
NEWS
View File

@ -21,6 +21,10 @@ PHP NEWS
. Fixed bug #72762 (Infinite loop while parsing a file with opcache enabled).
(Nikita)
- Reflection:
. Implemented request #38992 (invoke() and invokeArgs() static method calls
should match). (cmb)
- Standard:
. Fixed bug #55451 (substr_compare NULL length interpreted as 0). (Lauri
Kenttä)

View File

@ -78,6 +78,11 @@ PHP 7.1 UPGRADE NOTES
- OpenSSL:
. Dropped sslv2 stream.
- Reflection:
. The behavior of ReflectionMethod::invoke() and ::invokeArgs() has been
aligned, what causes slightly different behavior than before for some
pathological cases.
========================================
2. New Features
========================================

View File

@ -3185,109 +3185,14 @@ ZEND_METHOD(reflection_method, getClosure)
}
/* }}} */
/* {{{ proto public mixed ReflectionMethod::invoke(mixed object, mixed* args)
Invokes the method. */
ZEND_METHOD(reflection_method, invoke)
/* {{{ reflection_method_invoke */
static void reflection_method_invoke(INTERNAL_FUNCTION_PARAMETERS, int variadic)
{
zval retval;
zval *params = NULL;
zend_object *object;
zval *params = NULL, *val, *object;
reflection_object *intern;
zend_function *mptr;
int result, num_args = 0;
zend_fcall_info fci;
zend_fcall_info_cache fcc;
zend_class_entry *obj_ce;
METHOD_NOTSTATIC(reflection_method_ptr);
GET_REFLECTION_OBJECT_PTR(mptr);
if ((!(mptr->common.fn_flags & ZEND_ACC_PUBLIC)
|| (mptr->common.fn_flags & ZEND_ACC_ABSTRACT))
&& intern->ignore_visibility == 0)
{
if (mptr->common.fn_flags & ZEND_ACC_ABSTRACT) {
zend_throw_exception_ex(reflection_exception_ptr, 0,
"Trying to invoke abstract method %s::%s()",
ZSTR_VAL(mptr->common.scope->name), ZSTR_VAL(mptr->common.function_name));
} else {
zend_throw_exception_ex(reflection_exception_ptr, 0,
"Trying to invoke %s method %s::%s() from scope %s",
mptr->common.fn_flags & ZEND_ACC_PROTECTED ? "protected" : "private",
ZSTR_VAL(mptr->common.scope->name), ZSTR_VAL(mptr->common.function_name),
ZSTR_VAL(Z_OBJCE_P(getThis())->name));
}
return;
}
if (zend_parse_parameters(ZEND_NUM_ARGS(), "+", &params, &num_args) == FAILURE) {
return;
}
/* In case this is a static method, we should'nt pass an object_ptr
* (which is used as calling context aka $this). We can thus ignore the
* first parameter.
*
* Else, we verify that the given object is an instance of the class.
*/
if (mptr->common.fn_flags & ZEND_ACC_STATIC) {
object = NULL;
obj_ce = mptr->common.scope;
} else {
if (Z_TYPE(params[0]) != IS_OBJECT) {
_DO_THROW("Non-object passed to Invoke()");
/* Returns from this function */
}
obj_ce = Z_OBJCE(params[0]);
if (!instanceof_function(obj_ce, mptr->common.scope)) {
_DO_THROW("Given object is not an instance of the class this method was declared in");
/* Returns from this function */
}
object = Z_OBJ(params[0]);
}
fci.size = sizeof(fci);
ZVAL_UNDEF(&fci.function_name);
fci.object = object;
fci.retval = &retval;
fci.param_count = num_args - 1;
fci.params = params + 1;
fci.no_separation = 1;
fcc.initialized = 1;
fcc.function_handler = mptr;
fcc.calling_scope = obj_ce;
fcc.called_scope = intern->ce;
fcc.object = object;
result = zend_call_function(&fci, &fcc);
if (result == FAILURE) {
zend_throw_exception_ex(reflection_exception_ptr, 0,
"Invocation of method %s::%s() failed", ZSTR_VAL(mptr->common.scope->name), ZSTR_VAL(mptr->common.function_name));
return;
}
if (Z_TYPE(retval) != IS_UNDEF) {
ZVAL_COPY_VALUE(return_value, &retval);
}
}
/* }}} */
/* {{{ proto public mixed ReflectionMethod::invokeArgs(mixed object, array args)
Invokes the function and pass its arguments as array. */
ZEND_METHOD(reflection_method, invokeArgs)
{
zval retval;
zval *params, *val, *object;
reflection_object *intern;
zend_function *mptr;
int i, argc;
int result;
int i, argc = 0, result;
zend_fcall_info fci;
zend_fcall_info_cache fcc;
zend_class_entry *obj_ce;
@ -3297,10 +3202,6 @@ ZEND_METHOD(reflection_method, invokeArgs)
GET_REFLECTION_OBJECT_PTR(mptr);
if (zend_parse_parameters(ZEND_NUM_ARGS(), "o!a", &object, &param_array) == FAILURE) {
return;
}
if ((!(mptr->common.fn_flags & ZEND_ACC_PUBLIC)
|| (mptr->common.fn_flags & ZEND_ACC_ABSTRACT))
&& intern->ignore_visibility == 0)
@ -3319,14 +3220,24 @@ ZEND_METHOD(reflection_method, invokeArgs)
return;
}
argc = zend_hash_num_elements(Z_ARRVAL_P(param_array));
if (variadic) {
if (zend_parse_parameters(ZEND_NUM_ARGS(), "o!*", &object, &params, &argc) == FAILURE) {
return;
}
} else {
if (zend_parse_parameters(ZEND_NUM_ARGS(), "o!a", &object, &param_array) == FAILURE) {
return;
}
params = safe_emalloc(sizeof(zval), argc, 0);
argc = 0;
ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(param_array), val) {
ZVAL_COPY(&params[argc], val);
argc++;
} ZEND_HASH_FOREACH_END();
argc = zend_hash_num_elements(Z_ARRVAL_P(param_array));
params = safe_emalloc(sizeof(zval), argc, 0);
argc = 0;
ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(param_array), val) {
ZVAL_COPY(&params[argc], val);
argc++;
} ZEND_HASH_FOREACH_END();
}
/* In case this is a static method, we should'nt pass an object_ptr
* (which is used as calling context aka $this). We can thus ignore the
@ -3339,7 +3250,6 @@ ZEND_METHOD(reflection_method, invokeArgs)
obj_ce = mptr->common.scope;
} else {
if (!object) {
efree(params);
zend_throw_exception_ex(reflection_exception_ptr, 0,
"Trying to invoke non static method %s::%s() without an object",
ZSTR_VAL(mptr->common.scope->name), ZSTR_VAL(mptr->common.function_name));
@ -3349,7 +3259,9 @@ ZEND_METHOD(reflection_method, invokeArgs)
obj_ce = Z_OBJCE_P(object);
if (!instanceof_function(obj_ce, mptr->common.scope)) {
efree(params);
if (!variadic) {
efree(params);
}
_DO_THROW("Given object is not an instance of the class this method was declared in");
/* Returns from this function */
}
@ -3367,21 +3279,25 @@ ZEND_METHOD(reflection_method, invokeArgs)
fcc.function_handler = mptr;
fcc.calling_scope = obj_ce;
fcc.called_scope = intern->ce;
fcc.object = (object) ? Z_OBJ_P(object) : NULL;
fcc.object = object ? Z_OBJ_P(object) : NULL;
/*
* Copy the zend_function when calling via handler (e.g. Closure::__invoke())
*/
if ((mptr->internal_function.fn_flags & ZEND_ACC_CALL_VIA_TRAMPOLINE)) {
fcc.function_handler = _copy_function(mptr);
if (!variadic) {
/*
* Copy the zend_function when calling via handler (e.g. Closure::__invoke())
*/
if ((mptr->internal_function.fn_flags & ZEND_ACC_CALL_VIA_TRAMPOLINE)) {
fcc.function_handler = _copy_function(mptr);
}
}
result = zend_call_function(&fci, &fcc);
for (i = 0; i < argc; i++) {
zval_ptr_dtor(&params[i]);
if (!variadic) {
for (i = 0; i < argc; i++) {
zval_ptr_dtor(&params[i]);
}
efree(params);
}
efree(params);
if (result == FAILURE) {
zend_throw_exception_ex(reflection_exception_ptr, 0,
@ -3395,6 +3311,22 @@ ZEND_METHOD(reflection_method, invokeArgs)
}
/* }}} */
/* {{{ proto public mixed ReflectionMethod::invoke(mixed object, mixed* args)
Invokes the method. */
ZEND_METHOD(reflection_method, invoke)
{
reflection_method_invoke(INTERNAL_FUNCTION_PARAM_PASSTHRU, 1);
}
/* }}} */
/* {{{ proto public mixed ReflectionMethod::invokeArgs(mixed object, array args)
Invokes the function and pass its arguments as array. */
ZEND_METHOD(reflection_method, invokeArgs)
{
reflection_method_invoke(INTERNAL_FUNCTION_PARAM_PASSTHRU, 0);
}
/* }}} */
/* {{{ proto public bool ReflectionMethod::isFinal()
Returns whether this method is final */
ZEND_METHOD(reflection_method, isFinal)

View File

@ -38,6 +38,6 @@ try {
echo "===DONE===\n";?>
--EXPECTF--
Deprecated: Methods with the same name as their class will not be constructors in a future version of PHP; a has a deprecated constructor in %s on line %d
Non-object passed to Invoke()
Trying to invoke non static method a::a() without an object
Given object is not an instance of the class this method was declared in
===DONE===

View File

@ -115,5 +115,4 @@ string(86) "Trying to invoke private method TestClass::privateMethod() from scop
Abstract method:
string(53) "Trying to invoke abstract method AbstractClass::foo()"
Warning: ReflectionMethod::invokeArgs() expects exactly 2 parameters, 1 given in %s on line %d
string(53) "Trying to invoke abstract method AbstractClass::foo()"

View File

@ -97,8 +97,8 @@ Static method:
Warning: ReflectionMethod::invoke() expects at least 1 parameter, 0 given in %s on line %d
NULL
Called staticMethod()
Exception: Using $this when not in object context
Warning: ReflectionMethod::invoke() expects parameter 1 to be object, boolean given in %s on line %d
NULL
Called staticMethod()
Exception: Using $this when not in object context

View File

@ -59,7 +59,9 @@ try {
?>
--EXPECTF--
invoke() on a non-object:
string(29) "Non-object passed to Invoke()"
Warning: ReflectionMethod::invoke() expects parameter 1 to be object, boolean given in %s%eReflectionMethod_invoke_error1.php on line %d
NULL
invoke() on a non-instance:
string(72) "Given object is not an instance of the class this method was declared in"

View File

@ -0,0 +1,22 @@
--TEST--
Request #38992 (invoke() and invokeArgs() static method calls should match)
--FILE--
<?php
class MyClass
{
public static function doSomething()
{
echo "Did it!\n";
}
}
$r = new ReflectionMethod('MyClass', 'doSomething');
$r->invoke('WTF?');
$r->invokeArgs('WTF?', array());
?>
===DONE===
--EXPECTF--
Warning: ReflectionMethod::invoke() expects parameter 1 to be object, string given in %s%erequest38992.php on line %d
Warning: ReflectionMethod::invokeArgs() expects parameter 1 to be object, string given in %s%erequest38992.php on line %d
===DONE===