From 4b821f0fc6aade0eb9793a8b4fa3cd28b347ac2f Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Sat, 10 Oct 2015 13:39:26 +0200 Subject: [PATCH] Normalize rebinding failures Move all rebinding checks into one function to make sure they stay in sync. Normalize return value to be NULL for all rebinding failures, instead of returning an improperly bound closure in some cases. --- Zend/tests/closure_041.phpt | 8 +-- Zend/tests/closure_043.phpt | 20 ++------ Zend/tests/closure_call.phpt | 2 +- Zend/zend_closures.c | 98 +++++++++++++++++++----------------- 4 files changed, 58 insertions(+), 70 deletions(-) diff --git a/Zend/tests/closure_041.phpt b/Zend/tests/closure_041.phpt index 947517b4ed7..a7e9eab4828 100644 --- a/Zend/tests/closure_041.phpt +++ b/Zend/tests/closure_041.phpt @@ -53,9 +53,9 @@ $d = $nonstaticScoped->bindTo(null); $d(); echo "\n"; $d->bindTo($d); echo "After binding, with same-class instance for the bound ones", "\n"; -$d = $staticUnscoped->bindTo(new A); $d(); echo "\n"; +$d = $staticUnscoped->bindTo(new A); $d = $nonstaticUnscoped->bindTo(new A); $d(); echo " (should be scoped to dummy class)\n"; -$d = $staticScoped->bindTo(new A); $d(); echo "\n"; +$d = $staticScoped->bindTo(new A); $d = $nonstaticScoped->bindTo(new A); $d(); echo "\n"; echo "After binding, with different instance for the bound ones", "\n"; @@ -87,14 +87,10 @@ After binding, with same-class instance for the bound ones Warning: Cannot bind an instance to a static closure in %s on line %d scoped to A: bool(false) -bound: no -scoped to A: bool(false) bound: A (should be scoped to dummy class) Warning: Cannot bind an instance to a static closure in %s on line %d scoped to A: bool(true) -bound: no -scoped to A: bool(true) bound: A After binding, with different instance for the bound ones scoped to A: bool(false) diff --git a/Zend/tests/closure_043.phpt b/Zend/tests/closure_043.phpt index 98c88fda397..92b96575b52 100644 --- a/Zend/tests/closure_043.phpt +++ b/Zend/tests/closure_043.phpt @@ -26,16 +26,16 @@ $d = $staticUnscoped->bindTo(null, null); $d(); echo "\n"; $d = $staticScoped->bindTo(null, null); $d(); echo "\n"; echo "After binding, null scope, with instance", "\n"; -$d = $staticUnscoped->bindTo(new A, null); $d(); echo "\n"; -$d = $staticScoped->bindTo(new A, null); $d(); echo "\n"; +$d = $staticUnscoped->bindTo(new A, null); +$d = $staticScoped->bindTo(new A, null); echo "After binding, with scope, no instance", "\n"; $d = $staticUnscoped->bindTo(null, 'A'); $d(); echo "\n"; $d = $staticScoped->bindTo(null, 'A'); $d(); echo "\n"; echo "After binding, with scope, with instance", "\n"; -$d = $staticUnscoped->bindTo(new A, 'A'); $d(); echo "\n"; -$d = $staticScoped->bindTo(new A, 'A'); $d(); echo "\n"; +$d = $staticUnscoped->bindTo(new A, 'A'); +$d = $staticScoped->bindTo(new A, 'A'); echo "Done.\n"; @@ -57,14 +57,8 @@ bool(false) After binding, null scope, with instance Warning: Cannot bind an instance to a static closure in %s on line %d -bool(false) -bool(false) - Warning: Cannot bind an instance to a static closure in %s on line %d -bool(false) -bool(false) - After binding, with scope, no instance bool(true) bool(false) @@ -75,12 +69,6 @@ bool(false) After binding, with scope, with instance Warning: Cannot bind an instance to a static closure in %s on line %d -bool(true) -bool(false) - Warning: Cannot bind an instance to a static closure in %s on line %d -bool(true) -bool(false) - Done. diff --git a/Zend/tests/closure_call.phpt b/Zend/tests/closure_call.phpt index e8bed36aece..f665c67ff69 100644 --- a/Zend/tests/closure_call.phpt +++ b/Zend/tests/closure_call.phpt @@ -61,7 +61,7 @@ int(0) int(0) int(3) -Warning: Cannot bind closure to object of internal class stdClass in %s line %d +Warning: Cannot bind closure to scope of internal class stdClass in %s line %d NULL int(21) int(3) diff --git a/Zend/zend_closures.c b/Zend/zend_closures.c index 0ac4cc5e486..b1e21ed1e43 100644 --- a/Zend/zend_closures.c +++ b/Zend/zend_closures.c @@ -70,6 +70,50 @@ ZEND_METHOD(Closure, __invoke) /* {{{ */ } /* }}} */ +static zend_bool zend_valid_closure_binding( + zend_closure *closure, zval *newthis, zend_class_entry *scope) /* {{{ */ +{ + zend_function *func = &closure->func; + if (newthis) { + if (func->common.fn_flags & ZEND_ACC_STATIC) { + zend_error(E_WARNING, "Cannot bind an instance to a static closure"); + return 0; + } + + if (func->type == ZEND_INTERNAL_FUNCTION && func->common.scope && + !instanceof_function(Z_OBJCE_P(newthis), func->common.scope)) { + /* Binding incompatible $this to an internal method is not supported. */ + zend_error(E_WARNING, "Cannot bind internal method %s::%s() to object of class %s", + ZSTR_VAL(func->common.scope->name), + ZSTR_VAL(func->common.function_name), + ZSTR_VAL(Z_OBJCE_P(newthis)->name)); + return 0; + } + } else if (!(func->common.fn_flags & ZEND_ACC_STATIC) && func->common.scope + && func->type == ZEND_INTERNAL_FUNCTION) { + zend_error(E_WARNING, "Cannot unbind $this of internal method"); + return 0; + } + + if (scope && scope != func->common.scope && scope->type == ZEND_INTERNAL_CLASS) { + /* rebinding to internal class is not allowed */ + zend_error(E_WARNING, "Cannot bind closure to scope of internal class %s", + ZSTR_VAL(scope->name)); + return 0; + } + + if (func->type == ZEND_INTERNAL_FUNCTION && scope && func->common.scope && + !instanceof_function(scope, func->common.scope)) { + zend_error(E_WARNING, "Cannot bind function %s::%s to scope class %s", + ZSTR_VAL(func->common.scope->name), ZSTR_VAL(func->common.function_name), + ZSTR_VAL(scope->name)); + return 0; + } + + return 1; +} +/* }}} */ + /* {{{ proto mixed Closure::call(object to [, mixed parameter] [, mixed ...] ) Call closure, binding to a given object with its class as the scope */ ZEND_METHOD(Closure, call) @@ -90,26 +134,9 @@ ZEND_METHOD(Closure, call) zclosure = getThis(); closure = (zend_closure *) Z_OBJ_P(zclosure); - if (closure->func.common.fn_flags & ZEND_ACC_STATIC) { - zend_error(E_WARNING, "Cannot bind an instance to a static closure"); - return; - } - - if (closure->func.type == ZEND_INTERNAL_FUNCTION) { - /* verify that we aren't binding internal function to a wrong object */ - if ((closure->func.common.fn_flags & ZEND_ACC_STATIC) == 0 && - closure->func.common.scope && - !instanceof_function(Z_OBJCE_P(newthis), closure->func.common.scope)) { - zend_error(E_WARNING, "Cannot bind function %s::%s to object of class %s", ZSTR_VAL(closure->func.common.scope->name), ZSTR_VAL(closure->func.common.function_name), ZSTR_VAL(Z_OBJCE_P(newthis)->name)); - return; - } - } - newobj = Z_OBJ_P(newthis); - if (newobj->ce != closure->func.common.scope && newobj->ce->type == ZEND_INTERNAL_CLASS) { - /* rebinding to internal class is not allowed */ - zend_error(E_WARNING, "Cannot bind closure to object of internal class %s", ZSTR_VAL(newobj->ce->name)); + if (!zend_valid_closure_binding(closure, newthis, Z_OBJCE_P(newthis))) { return; } @@ -164,21 +191,11 @@ ZEND_METHOD(Closure, bind) zend_class_entry *ce, *called_scope; if (zend_parse_method_parameters(ZEND_NUM_ARGS(), getThis(), "Oo!|z", &zclosure, zend_ce_closure, &newthis, &scope_arg) == FAILURE) { - RETURN_NULL(); + return; } closure = (zend_closure *)Z_OBJ_P(zclosure); - if ((newthis != NULL) && (closure->func.common.fn_flags & ZEND_ACC_STATIC)) { - zend_error(E_WARNING, "Cannot bind an instance to a static closure"); - } - - if (newthis == NULL && !(closure->func.common.fn_flags & ZEND_ACC_STATIC) - && closure->func.common.scope && closure->func.type == ZEND_INTERNAL_FUNCTION) { - zend_error(E_WARNING, "Cannot unbind $this of internal method"); - return; - } - if (scope_arg != NULL) { /* scope argument was given */ if (Z_TYPE_P(scope_arg) == IS_OBJECT) { ce = Z_OBJCE_P(scope_arg); @@ -195,15 +212,14 @@ ZEND_METHOD(Closure, bind) } zend_string_release(class_name); } - if(ce && ce != closure->func.common.scope && ce->type == ZEND_INTERNAL_CLASS) { - /* rebinding to internal class is not allowed */ - zend_error(E_WARNING, "Cannot bind closure to scope of internal class %s", ZSTR_VAL(ce->name)); - return; - } } else { /* scope argument not given; do not change the scope by default */ ce = closure->func.common.scope; } + if (!zend_valid_closure_binding(closure, newthis, ce)) { + return; + } + if (newthis) { called_scope = Z_OBJCE_P(newthis); } else { @@ -580,19 +596,7 @@ ZEND_API void zend_create_closure(zval *res, zend_function *func, zend_class_ent closure->orig_internal_handler = closure->func.internal_function.handler; } closure->func.internal_function.handler = zend_closure_internal_handler; - /* verify that we aren't binding internal function to a wrong scope */ - if(func->common.scope != NULL) { - if(scope && !instanceof_function(scope, func->common.scope)) { - zend_error(E_WARNING, "Cannot bind function %s::%s to scope class %s", ZSTR_VAL(func->common.scope->name), ZSTR_VAL(func->common.function_name), ZSTR_VAL(scope->name)); - scope = NULL; - } - if(scope && this_ptr && (func->common.fn_flags & ZEND_ACC_STATIC) == 0 && - !instanceof_function(Z_OBJCE_P(this_ptr), closure->func.common.scope)) { - zend_error(E_WARNING, "Cannot bind function %s::%s to object of class %s", ZSTR_VAL(func->common.scope->name), ZSTR_VAL(func->common.function_name), ZSTR_VAL(Z_OBJCE_P(this_ptr)->name)); - scope = NULL; - this_ptr = NULL; - } - } else { + if (!func->common.scope) { /* if it's a free function, we won't set scope & this since they're meaningless */ this_ptr = NULL; scope = NULL;