com php-src: Improve 517b55362 (scope rebinding on method Closures) Now it is completely impossible to rebind a scoped method Closure (only the kind
you get from ReflectionMethod::getClosure()) to a foreign scope Adding a lot of tests to ensure this... Also, properly return NULL in case the Closure
could not be created instead of some crippled unbound Closure: Zend/tests/bug70630.phpt Zend/tests/closure_061.phpt Zend/tests/closure_062.phpt
Zend/t

From: Date: Sun, 04 Oct 2015 22:56:55 +0000
Subject: com php-src: Improve 517b55362 (scope rebinding on method Closures) Now it is completely impossible to rebind a scoped method Closure (only the kind
you get from ReflectionMethod::getClosure()) to a foreign scope Adding a lot of tests to ensure this... Also, properly return NULL in case the Closure
could not be created instead of some crippled unbound Closure: Zend/tests/bug70630.phpt Zend/tests/closure_061.phpt Zend/tests/closure_062.phpt
Zend/t
Groups: php.cvs 
Request: Send a blank email to php-cvs+get-89289@lists.php.net to get a copy of this message
Commit: 881c50252066132f83e190325e344f532be19033 Author: Bob Weinand <bobwei9@hotmail.com> Mon, 5 Oct 2015 00:52:10 +0200 Parents: a549a3ac179ec98e2e706e51f3df70e3bda553b2 Branches: PHP-7.0 Link: http://git.php.net/?p=php-src.git;a=commitdiff;h=881c50252066132f83e190325e344f532be19033 Log: Improve 517b55362 (scope rebinding on method Closures) Now it is completely impossible to rebind a scoped method Closure (only the kind you get from ReflectionMethod::getClosure()) to a foreign scope Adding a lot of tests to ensure this... Also, properly return NULL in case the Closure could not be created instead of some crippled unbound Closure Changed paths: M Zend/tests/bug70630.phpt M Zend/tests/closure_061.phpt A Zend/tests/closure_062.phpt A Zend/tests/closure_063.phpt M Zend/zend_closures.c

diff --git a/Zend/tests/bug70630.phpt b/Zend/tests/bug70630.phpt index 3dabca6..a46c460 100644 --- a/Zend/tests/bug70630.phpt +++ b/Zend/tests/bug70630.phpt @@ -17,8 +17,8 @@ foreach (["substr", "foo"] as $fn) { Warning: Cannot bind function substr to an object in %s on line %d -Warning: Cannot bind function substr to an object in %s on line %d +Warning: Cannot bind function substr to an object or class in %s on line %d Warning: Cannot bind function foo to an object in %s on line %d -Warning: Cannot bind function foo to an object in %s on line %d +Warning: Cannot bind function foo to an object or class in %s on line %d diff --git a/Zend/tests/closure_061.phpt b/Zend/tests/closure_061.phpt index a195956..342b05c 100644 --- a/Zend/tests/closure_061.phpt +++ b/Zend/tests/closure_061.phpt @@ -56,9 +56,8 @@ string(3) "bar" Warning: Cannot bind function foo::initClass to object of class baz in %s on line %d -Fatal error: Uncaught Error: Using $this when not in object context in %s:%d +Fatal error: Uncaught Error: Function name must be a string in %s:%d Stack trace: -#0 %s(%d): initClass() -#1 %s(%d): callMethodOn('foo', 'initClass', Object(baz)) -#2 {main} +#0 %s(%d): callMethodOn('foo', 'initClass', Object(baz)) +#1 {main} thrown in %s on line %d diff --git a/Zend/tests/closure_062.phpt b/Zend/tests/closure_062.phpt new file mode 100644 index 0000000..574b43a --- /dev/null +++ b/Zend/tests/closure_062.phpt @@ -0,0 +1,39 @@ +--TEST-- +Force failure with Closure binding to unknown scopes/$this with Closure::call() +--FILE-- +<?php + +function foo() { print "FAIL\n"; } +$x = (new ReflectionFunction("foo"))->getClosure(); +$x->call(new stdClass); + +class a { function foo() { echo get_class($this), "\n"; } } +$x = (new ReflectionMethod("a", "foo"))->getClosure(new a); +$x->call(new stdClass); + +class b extends a {} +$x = (new ReflectionMethod("a", "foo"))->getClosure(new a); +$x->call(new b); + +class c extends stdClass { function foo() { echo get_class($this), "\n"; } } +$x = (new ReflectionMethod("c", "foo"))->getClosure(new c); +$x->call(new stdClass); + +$x = (new ReflectionMethod("a", "foo"))->getClosure(new a); +$x->call(new a); + +class d { function foo() { print "Success\n"; yield; } } +$x = (new ReflectionMethod("d", "foo"))->getClosure(new d); +$x->call(new d)->current(); + +?> +--EXPECTF-- + +Warning: Cannot bind function foo to an object in %s on line %d + +Warning: Cannot bind function a::foo to object of class stdClass in %s on line %d +b + +Warning: Cannot bind function c::foo to object of class stdClass in %s on line %d +a +Success diff --git a/Zend/tests/closure_063.phpt b/Zend/tests/closure_063.phpt new file mode 100644 index 0000000..e854b57 --- /dev/null +++ b/Zend/tests/closure_063.phpt @@ -0,0 +1,53 @@ +--TEST-- +Force failure with Closure binding to unknown scopes/$this with Closure::bind() +--FILE-- +<?php + +function foo() { print "FAIL\n"; } +$x = (new ReflectionFunction("foo"))->getClosure(); +$x->bindTo(new stdClass); + +class a { function foo() { echo get_class($this), "\n"; } } +$x = (new ReflectionMethod("a", "foo"))->getClosure(new a); +$x->bindTo(new stdClass); + +class c extends stdClass { function foo() { echo get_class($this), "\n"; } } +$x = (new ReflectionMethod("c", "foo"))->getClosure(new c); +$x->bindTo(new stdClass); + +class b extends a {} +$x = (new ReflectionMethod("a", "foo"))->getClosure(new a); +$x->bindTo(new b)(); +$x->bindTo(new b, "a")(); +$x->bindTo(new b, "c"); + +$x = (new ReflectionMethod("a", "foo"))->getClosure(new a); +$x->bindTo(new a)(); + +class d { function foo() { print "Success\n"; yield; } } +class e extends d {} +$x = (new ReflectionMethod("d", "foo"))->getClosure(new d); +$x->bindTo(new d)()->current(); +$x->bindTo(new e)()->current(); +$x->bindTo(new e, "d")()->current(); +$x->bindTo(new e, "e"); + +?> +--EXPECTF-- + +Warning: Cannot bind function foo to an object or class in %s on line %d + +Warning: Cannot bind function a::foo to object of class stdClass in %s on line %d + +Warning: Cannot bind function c::foo to object of class stdClass in %s on line %d +b +b + +Warning: Cannot bind function a::foo to scope class c in %s on line %d +a +Success +Success +Success + +Warning: Cannot bind function d::foo to scope class e in %s on line %d + diff --git a/Zend/zend_closures.c b/Zend/zend_closures.c index 8de7b6e..c06aac6 100644 --- a/Zend/zend_closures.c +++ b/Zend/zend_closures.c @@ -126,17 +126,23 @@ ZEND_METHOD(Closure, call) if (fci_cache.function_handler->common.fn_flags & ZEND_ACC_GENERATOR) { zval new_closure; - zend_create_closure(&new_closure, fci_cache.function_handler, Z_OBJCE_P(newthis), closure->called_scope, newthis); + zend_create_closure(&new_closure, fci_cache.function_handler, closure->func.common.scope, closure->called_scope, &closure->this_ptr); closure = (zend_closure *) Z_OBJ(new_closure); + closure->func.common.fn_flags |= ZEND_ACC_PUBLIC; + zval_ptr_dtor_nogc(&closure->this_ptr); + ZVAL_COPY(&closure->this_ptr, newthis); fci_cache.function_handler = &closure->func; } else { memcpy(&my_function, fci_cache.function_handler, fci_cache.function_handler->type == ZEND_USER_FUNCTION ? sizeof(zend_op_array) : sizeof(zend_internal_function)); - /* use scope of passed object */ - my_function.common.scope = Z_OBJCE_P(newthis); fci_cache.function_handler = &my_function; + } + + if (closure->func.type == ZEND_USER_FUNCTION && (closure->func.common.fn_flags & ZEND_ACC_REAL_CLOSURE)) { + /* use scope of passed object */ + fci_cache.function_handler->common.scope = Z_OBJCE_P(newthis); /* Runtime cache relies on bound scope to be immutable, hence we need a separate rt cache in case scope changed */ - if (ZEND_USER_CODE(my_function.type) && closure->func.common.scope != Z_OBJCE_P(newthis)) { + if (closure->func.common.scope != Z_OBJCE_P(newthis)) { my_function.op_array.run_time_cache = emalloc(my_function.op_array.cache_size); memset(my_function.op_array.run_time_cache, 0, my_function.op_array.cache_size); } @@ -149,7 +155,7 @@ ZEND_METHOD(Closure, call) if (fci_cache.function_handler->common.fn_flags & ZEND_ACC_GENERATOR) { /* copied upon generator creation */ --GC_REFCOUNT(&closure->std); - } else if (ZEND_USER_CODE(my_function.type) && closure->func.common.scope != Z_OBJCE_P(newthis)) { + } else if (ZEND_USER_CODE(my_function.type) && (closure->func.common.fn_flags & ZEND_ACC_REAL_CLOSURE) && closure->func.common.scope != Z_OBJCE_P(newthis)) { efree(my_function.op_array.run_time_cache); } } @@ -204,31 +210,18 @@ ZEND_METHOD(Closure, bind) called_scope = ce; } - /* verify that we aren't binding methods to a wrong object */ - if (closure->func.type != ZEND_USER_FUNCTION || (closure->func.common.fn_flags & ZEND_ACC_REAL_CLOSURE) == 0) { - if (!closure->func.common.scope) { - if (ce) { - zend_error(E_WARNING, "Cannot bind function %s to an object", ZSTR_VAL(closure->func.common.function_name)); - return; - } - } else if (!ce) { - zend_error(E_WARNING, "Cannot bind function %s::%s to no class", ZSTR_VAL(closure->func.common.scope->name), ZSTR_VAL(closure->func.common.function_name)); - return; - } else if (!instanceof_function(ce, closure->func.common.scope)) { - zend_error(E_WARNING, "Cannot bind function %s::%s to class %s", ZSTR_VAL(closure->func.common.scope->name), ZSTR_VAL(closure->func.common.function_name), ZSTR_VAL(ce->name)); - return; - } - } - zend_create_closure(return_value, &closure->func, ce, called_scope, newthis); - new_closure = (zend_closure *) Z_OBJ_P(return_value); - /* Runtime cache relies on bound scope to be immutable, hence we need a separate rt cache in case scope changed */ - if (ZEND_USER_CODE(closure->func.type) && (closure->func.common.scope != new_closure->func.common.scope || (closure->func.op_array.fn_flags & ZEND_ACC_NO_RT_ARENA))) { - new_closure->func.op_array.run_time_cache = emalloc(new_closure->func.op_array.cache_size); - memset(new_closure->func.op_array.run_time_cache, 0, new_closure->func.op_array.cache_size); + if (Z_TYPE_P(return_value) != IS_NULL) { + new_closure = (zend_closure *) Z_OBJ_P(return_value); + + /* Runtime cache relies on bound scope to be immutable, hence we need a separate rt cache in case scope changed */ + if (ZEND_USER_CODE(closure->func.type) && (closure->func.common.scope != new_closure->func.common.scope || (closure->func.op_array.fn_flags & ZEND_ACC_NO_RT_ARENA))) { + new_closure->func.op_array.run_time_cache = emalloc(new_closure->func.op_array.cache_size); + memset(new_closure->func.op_array.run_time_cache, 0, new_closure->func.op_array.cache_size); - new_closure->func.op_array.fn_flags |= ZEND_ACC_NO_RT_ARENA; + new_closure->func.op_array.fn_flags |= ZEND_ACC_NO_RT_ARENA; + } } } /* }}} */ @@ -374,8 +367,7 @@ static zend_object *zend_closure_clone(zval *zobject) /* {{{ */ zend_closure *closure = (zend_closure *)Z_OBJ_P(zobject); zval result; - zend_create_closure(&result, &closure->func, - closure->func.common.scope, closure->called_scope, &closure->this_ptr); + zend_create_closure(&result, &closure->func, closure->func.common.scope, closure->called_scope, &closure->this_ptr); return Z_OBJ(result); } /* }}} */ @@ -533,10 +525,37 @@ void zend_register_closure_ce(void) /* {{{ */ } /* }}} */ +/* res will be set to IS_NULL or contains a Closure object */ ZEND_API void zend_create_closure(zval *res, zend_function *func, zend_class_entry *scope, zend_class_entry *called_scope, zval *this_ptr) /* {{{ */ { zend_closure *closure; + if ((scope == NULL) && this_ptr && (Z_TYPE_P(this_ptr) != IS_UNDEF)) { + /* use dummy scope if we're binding an object without specifying a scope */ + /* maybe it would be better to create one for this purpose */ + scope = zend_ce_closure; + } + + if (func->type != ZEND_USER_FUNCTION || (func->common.fn_flags & ZEND_ACC_REAL_CLOSURE) == 0) { + /* verify that we aren't binding internal function to a wrong scope */ + if (func->common.scope != NULL) { + if (scope && 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)); + ZVAL_NULL(res); + return; + } + if (scope && this_ptr && (func->common.fn_flags & ZEND_ACC_STATIC) == 0 && !instanceof_function(Z_OBJCE_P(this_ptr), 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)); + ZVAL_NULL(res); + return; + } + } else if (scope) { + zend_error(E_WARNING, "Cannot bind function %s to an object or class", ZSTR_VAL(func->common.function_name)); + ZVAL_NULL(res); + return; + } + } + object_init_ex(res, zend_ce_closure); closure = (zend_closure *) Z_OBJ_P(res); @@ -545,12 +564,6 @@ ZEND_API void zend_create_closure(zval *res, zend_function *func, zend_class_ent closure->func.common.prototype = (zend_function *) closure; closure->func.common.fn_flags |= ZEND_ACC_CLOSURE; - if ((scope == NULL) && this_ptr && (Z_TYPE_P(this_ptr) != IS_UNDEF)) { - /* use dummy scope if we're binding an object without specifying a scope */ - /* maybe it would be better to create one for this purpose */ - scope = zend_ce_closure; - } - if (closure->func.type == ZEND_USER_FUNCTION) { if (closure->func.op_array.static_variables) { HashTable *static_variables = closure->func.op_array.static_variables; @@ -567,24 +580,6 @@ ZEND_API void zend_create_closure(zval *res, zend_function *func, zend_class_ent (*closure->func.op_array.refcount)++; } } - if (closure->func.type != ZEND_USER_FUNCTION || (func->common.fn_flags & ZEND_ACC_REAL_CLOSURE) == 0) { - /* 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; - } - } else { - /* if it's a free function, we won't set scope & this since they're meaningless */ - this_ptr = NULL; - scope = NULL; - } - } ZVAL_UNDEF(&closure->this_ptr); /* Invariant:
« previous php.cvs (#89289) next »