Don't allow unbound scoped closures; make ->call used passed object as scope

7 files changed, 31 insertions, 80 deletions

diff --git a/Zend/tests/closure_bind_unbound_scoped.phpt b/Zend/tests/closure_bind_unbound_scoped.phpt
deleted file mode 100644
index 2975d39ae6..0000000000
--- a/Zend/tests/closure_bind_unbound_scoped.phpt
+++ /dev/null
@@ -1,52 +0,0 @@
---TEST--
-Closure::bind and ::bindTo unbound_scoped parameter
---FILE--
-<?php
-
-$foo = function () {};
-$foo = $foo->bindTo(NULL, "StdClass", true);
-// We need to check that you can call an unbound scoped closure without binding it (good idea or no)
-$foo();
-
-class FooBar {
-    private $x = 3;
-}
-$foo = function () {
-    var_dump($this->x);
-};
-
-// This should create a closure scoped to FooBar but unbound, not static
-$foo = $foo->bindTo(NULL, "FooBar", true);
-
-// As it is unbound, not static, this will work
-$foo->call(new FooBar);
-
-$foo = function () {
-    var_dump($this->x);
-};
-// Make sure ::bind() works the same
-$foo = Closure::bind($foo, NULL, "FooBar", true);
-$foo->call(new FooBar);
-
-// Ensure that binding and having it unscoped will not prevent an E_NOTICE for non-static methods called statically
-class Qux {
-    function notStatic() {}
-}
-$x = new ReflectionMethod("Qux::notStatic");
-$x = $x->getClosure(new Qux);
-$x = $x->bindTo(NULL, "Qux", true);
-$x();
-
-// Ensure that binding and having it unscoped will not prevent fatal error for trying to call an internal class's method with $this as NULL
-$x = new ReflectionMethod("Closure::bindTo");
-$x = $x->getClosure(function () {});
-$x = $x->bindTo(NULL, "Closure", true);
-$x();
-?>
---EXPECTF--
-int(3)
-int(3)
-
-Strict Standards: Non-static method Qux::notStatic() should not be called statically in %s on line 35
-
-Fatal error: Non-static method Closure::bindTo() cannot be called statically in %s on line 41
\ No newline at end of file
diff --git a/Zend/tests/closure_call.phpt b/Zend/tests/closure_call.phpt
index 7a65817189..d53b6077ee 100644
--- a/Zend/tests/closure_call.phpt
+++ b/Zend/tests/closure_call.phpt
@@ -19,7 +19,10 @@ $foobar = new Foo;
 $foobar->x = 3;
 
 var_dump($qux());
+
 var_dump($qux->call($foo));
+
+// Try on an object other than the one already bound
 var_dump($qux->call($foobar));
 
 
@@ -30,6 +33,7 @@ $bar = function () {
 $elePHPant = new StdClass;
 $elePHPant->x = 7;
 
+// Try on a StdClass
 var_dump($bar->call($elePHPant));
 
 
@@ -37,12 +41,25 @@ $beta = function ($z) {
     return $this->x * $z;
 };
 
+// Ensure argument passing works
 var_dump($beta->call($elePHPant, 3));
 
+// Ensure ->call calls with scope of passed object
+class FooBar {
+    private $x = 3;
+}
+
+$foo = function () {
+    var_dump($this->x);
+};
+
+$foo->call(new FooBar);
+
 ?>
 --EXPECT--
 int(0)
 int(0)
 int(3)
 int(7)
-int(21)
\ No newline at end of file
+int(21)
+int(3)
\ No newline at end of file
diff --git a/Zend/zend_closures.c b/Zend/zend_closures.c
index dd1921efc1..203fe1d572 100644
--- a/Zend/zend_closures.c
+++ b/Zend/zend_closures.c
@@ -76,6 +76,7 @@ ZEND_METHOD(Closure, call) /* {{{ */
 	zend_fcall_info_cache fci_cache;
 	zval *my_params;
 	zend_uint my_param_count = 0;
+	zend_function my_function;
 
 	if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "o*", &newthis, &my_params, &my_param_count) == FAILURE) {
 		RETURN_NULL();
@@ -108,6 +109,11 @@ ZEND_METHOD(Closure, call) /* {{{ */
 	fci.param_count = my_param_count;
 	fci.object = fci_cache.object = Z_OBJ_P(newthis);
 	fci_cache.initialized = 1;
+	
+	my_function = *fci_cache.function_handler;
+	/* use scope of passed object */
+	my_function.common.scope = Z_OBJCE_P(newthis);
+	fci_cache.function_handler = &my_function;
 
 	if (zend_call_function(&fci, &fci_cache TSRMLS_CC) == SUCCESS && Z_TYPE(closure_result) != IS_UNDEF) {
 		ZVAL_COPY_VALUE(return_value, &closure_result);
@@ -115,16 +121,15 @@ ZEND_METHOD(Closure, call) /* {{{ */
 }
 /* }}} */
 
-/* {{{ proto Closure Closure::bind(Closure $old, object $to [, mixed $scope = "static" ] [, bool $unbound_scoped = false ] )
+/* {{{ proto Closure Closure::bind(Closure $old, object $to [, mixed $scope = "static" ] )
    Create a closure from another one and bind to another object and scope */
 ZEND_METHOD(Closure, bind)
 {
 	zval *newthis, *zclosure, *scope_arg = NULL;
 	zend_closure *closure;
 	zend_class_entry *ce;
-	zend_bool unbound_scoped = false;
 
-	if (zend_parse_method_parameters(ZEND_NUM_ARGS() TSRMLS_CC, getThis(), "Oo!|zb", &zclosure, zend_ce_closure, &newthis, &scope_arg, &unbound_scoped) == FAILURE) {
+	if (zend_parse_method_parameters(ZEND_NUM_ARGS() TSRMLS_CC, getThis(), "Oo!|z", &zclosure, zend_ce_closure, &newthis, &scope_arg) == FAILURE) {
 		RETURN_NULL();
 	}
 
@@ -155,7 +160,7 @@ ZEND_METHOD(Closure, bind)
 		ce = closure->func.common.scope;
 	}
 
-	zend_create_closure_ex(return_value, &closure->func, ce, newthis, unbound_scoped TSRMLS_CC);
+	zend_create_closure(return_value, &closure->func, ce, newthis TSRMLS_CC);
 }
 /* }}} */
 
@@ -415,14 +420,12 @@ ZEND_METHOD(Closure, __construct)
 ZEND_BEGIN_ARG_INFO_EX(arginfo_closure_bindto, 0, 0, 1)
 	ZEND_ARG_INFO(0, newthis)
 	ZEND_ARG_INFO(0, newscope)
-	ZEND_ARG_INFO(0, unbound_scoped)
 ZEND_END_ARG_INFO()
 
 ZEND_BEGIN_ARG_INFO_EX(arginfo_closure_bind, 0, 0, 2)
 	ZEND_ARG_INFO(0, closure)
 	ZEND_ARG_INFO(0, newthis)
 	ZEND_ARG_INFO(0, newscope)
-	ZEND_ARG_INFO(0, unbound_scoped)
 ZEND_END_ARG_INFO()
 
 ZEND_BEGIN_ARG_INFO_EX(arginfo_closure_call, 0, 0, 1)
@@ -469,11 +472,6 @@ void zend_register_closure_ce(TSRMLS_D) /* {{{ */
 
 ZEND_API void zend_create_closure(zval *res, zend_function *func, zend_class_entry *scope, zval *this_ptr TSRMLS_DC) /* {{{ */
 {
-	zend_create_closure_ex(res, func, scope, this_ptr, 0 TSRMLS_CC);
-}
-
-ZEND_API void zend_create_closure_ex(zval *res, zend_function *func, zend_class_entry *scope, zval *this_ptr, zend_bool unbound_scoped TSRMLS_DC) /* {{{ */
-{
 	zend_closure *closure;
 
 	object_init_ex(res, zend_ce_closure);
@@ -522,22 +520,14 @@ ZEND_API void zend_create_closure_ex(zval *res, zend_function *func, zend_class_
 	ZVAL_UNDEF(&closure->this_ptr);
 	/* Invariants:
 	 * If the closure is unscoped, it is unbound.
-	 * If the closure is scoped, it may be static, bound or unbound */
+	 * If the closure is scoped, it is static or unbound */
 	closure->func.common.scope = scope;
 	if (scope) {
 		closure->func.common.fn_flags |= ZEND_ACC_PUBLIC;
 		if (this_ptr && Z_TYPE_P(this_ptr) == IS_OBJECT && (closure->func.common.fn_flags & ZEND_ACC_STATIC) == 0) {
 			ZVAL_COPY(&closure->this_ptr, this_ptr);
- 		/* We assume that a static scoped closure is desired if given NULL to bind to
- 		   If an unbound scoped closure is desired, the parameter must be set to 1*/
- 		} else if (!unbound_scoped) {
+ 		} else {
 			closure->func.common.fn_flags |= ZEND_ACC_STATIC;
-		/* Unbound but scoped was explicitly specified and we wish to avoid E_ERROR when calling without object
-		   We don't do this if it has implict allowed static (i.e. is a method, should E_STRICT)
-		   Nor do we do this if it's an internal function (which would blow up if $this was NULL)
-		   (In that case, we're actually creating a closure which can't be called without apply) */
-		} else if ((func->common.fn_flags & ZEND_ACC_ALLOW_STATIC) == 0 && func->type == ZEND_USER_FUNCTION) {
-			closure->func.common.fn_flags |= ZEND_ACC_ALLOW_STATIC_EXPLICIT;
 		}
 	}
 }
diff --git a/Zend/zend_closures.h b/Zend/zend_closures.h
index 0f9ab5c5b6..4f8ae6dd91 100644
--- a/Zend/zend_closures.h
+++ b/Zend/zend_closures.h
@@ -29,7 +29,6 @@ void zend_register_closure_ce(TSRMLS_D);
 extern ZEND_API zend_class_entry *zend_ce_closure;
 
 ZEND_API void zend_create_closure(zval *res, zend_function *op_array, zend_class_entry *scope, zval *this_ptr TSRMLS_DC);
-ZEND_API void zend_create_closure_ex(zval *res, zend_function *op_array, zend_class_entry *scope, zval *this_ptr, zend_bool unbound_scoped TSRMLS_DC);
 ZEND_API zend_function *zend_get_closure_invoke_method(zend_object *obj TSRMLS_DC);
 ZEND_API const zend_function *zend_get_closure_method_def(zval *obj TSRMLS_DC);
 ZEND_API zval* zend_get_closure_this_ptr(zval *obj TSRMLS_DC);
diff --git a/Zend/zend_compile.h b/Zend/zend_compile.h
index c59e3cfa82..fa333545f4 100644
--- a/Zend/zend_compile.h
+++ b/Zend/zend_compile.h
@@ -170,9 +170,6 @@ typedef struct _zend_try_catch_element {
 /* method flag (bc only), any method that has this flag can be used statically and non statically. */
 #define ZEND_ACC_ALLOW_STATIC	0x10000
 
-/* method flag (for explicitly unbound scoped closures which shouldn't warn) */
-#define ZEND_ACC_ALLOW_STATIC_EXPLICIT	0x20000000
-
 /* shadow of parent's private method/property */
 #define ZEND_ACC_SHADOW 0x20000
 
diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h
index c4f962b6dc..7749197a74 100644
--- a/Zend/zend_vm_def.h
+++ b/Zend/zend_vm_def.h
@@ -2640,7 +2640,7 @@ ZEND_VM_HANDLER(60, ZEND_DO_FCALL, ANY, ANY)
 			if (UNEXPECTED(EG(exception) != NULL)) {
 				HANDLE_EXCEPTION();
 			}
-		} else if (!(fbc->common.fn_flags & ZEND_ACC_ALLOW_STATIC_EXPLICIT)){
+		} else {
 			/* FIXME: output identifiers properly */
 			/* An internal function assumes $this is present and won't check that. So PHP would crash by allowing the call. */
 			zend_error_noreturn(E_ERROR, "Non-static method %s::%s() cannot be called statically", fbc->common.scope->name->val, fbc->common.function_name->val);
diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h
index 81263598d8..c2e59c1441 100644
--- a/Zend/zend_vm_execute.h
+++ b/Zend/zend_vm_execute.h
@@ -533,7 +533,7 @@ static int ZEND_FASTCALL  ZEND_DO_FCALL_SPEC_HANDLER(ZEND_OPCODE_HANDLER_ARGS)
 			if (UNEXPECTED(EG(exception) != NULL)) {
 				HANDLE_EXCEPTION();
 			}
-		} else if (!(fbc->common.fn_flags & ZEND_ACC_ALLOW_STATIC_EXPLICIT)){
+		} else {
 			/* FIXME: output identifiers properly */
 			/* An internal function assumes $this is present and won't check that. So PHP would crash by allowing the call. */
 			zend_error_noreturn(E_ERROR, "Non-static method %s::%s() cannot be called statically", fbc->common.scope->name->val, fbc->common.function_name->val);