From: Nikita Popov <nikita.ppv@gmail.com>
Date: Tue, 7 Jul 2020 07:08:56 +0000 (+0200)
Subject: Don't allow separation in array functions
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=dadb92ea35688a82bec7d4dca5a6edd446c50576;p=php

Don't allow separation in array functions

The only case here that might be *somewhat* sensible is the userdata
argument of array_walk(), which could be used to keep persistent state
between callback invokations -- with the WTF moment that the final
result after the walk finishes will be unchanged. Nowdays, this is
much better achieved using a closure with a use-by-reference.
---

diff --git a/ext/standard/array.c b/ext/standard/array.c
index 530e72c4af..3eb06bd3c4 100644
--- a/ext/standard/array.c
+++ b/ext/standard/array.c
@@ -921,7 +921,7 @@ static inline int php_array_user_compare_unstable(Bucket *f, Bucket *s) /* {{{ *
 	BG(user_compare_fci).param_count = 2;
 	BG(user_compare_fci).params = args;
 	BG(user_compare_fci).retval = &retval;
-	BG(user_compare_fci).no_separation = 0;
+	BG(user_compare_fci).no_separation = 1;
 	call_failed = zend_call_function(&BG(user_compare_fci), &BG(user_compare_fci_cache)) == FAILURE || Z_TYPE(retval) == IS_UNDEF;
 	zval_ptr_dtor(&args[1]);
 	zval_ptr_dtor(&args[0]);
@@ -1063,7 +1063,7 @@ static inline int php_array_user_key_compare_unstable(Bucket *f, Bucket *s) /* {
 	BG(user_compare_fci).param_count = 2;
 	BG(user_compare_fci).params = args;
 	BG(user_compare_fci).retval = &retval;
-	BG(user_compare_fci).no_separation = 0;
+	BG(user_compare_fci).no_separation = 1;
 	call_failed = zend_call_function(&BG(user_compare_fci), &BG(user_compare_fci_cache)) == FAILURE || Z_TYPE(retval) == IS_UNDEF;
 	zval_ptr_dtor(&args[1]);
 	zval_ptr_dtor(&args[0]);
@@ -1374,7 +1374,7 @@ static int php_array_walk(zval *array, zval *userdata, int recursive) /* {{{ */
 	BG(array_walk_fci).retval = &retval;
 	BG(array_walk_fci).param_count = userdata ? 3 : 2;
 	BG(array_walk_fci).params = args;
-	BG(array_walk_fci).no_separation = 0;
+	BG(array_walk_fci).no_separation = 1;
 
 	zend_hash_internal_pointer_reset_ex(target_hash, &pos);
 	ht_iter = zend_hash_iterator_add(target_hash, pos);
@@ -4546,7 +4546,7 @@ static int zval_user_compare(zval *a, zval *b) /* {{{ */
 	BG(user_compare_fci).param_count = 2;
 	BG(user_compare_fci).params = args;
 	BG(user_compare_fci).retval = &retval;
-	BG(user_compare_fci).no_separation = 0;
+	BG(user_compare_fci).no_separation = 1;
 
 	if (zend_call_function(&BG(user_compare_fci), &BG(user_compare_fci_cache)) == SUCCESS && Z_TYPE(retval) != IS_UNDEF) {
 		zend_long ret = zval_get_long(&retval);
@@ -5906,7 +5906,7 @@ PHP_FUNCTION(array_reduce)
 
 	fci.retval = &retval;
 	fci.param_count = 2;
-	fci.no_separation = 0;
+	fci.no_separation = 1;
 
 	ZEND_HASH_FOREACH_VAL(htbl, operand) {
 		ZVAL_COPY_VALUE(&args[0], return_value);
@@ -5959,7 +5959,7 @@ PHP_FUNCTION(array_filter)
 
 	if (ZEND_FCI_INITIALIZED(fci)) {
 		have_callback = 1;
-		fci.no_separation = 0;
+		fci.no_separation = 1;
 		fci.retval = &retval;
 		if (use_type == ARRAY_FILTER_USE_BOTH) {
 			fci.param_count = 2;
@@ -6062,7 +6062,7 @@ PHP_FUNCTION(array_map)
 			fci.retval = &result;
 			fci.param_count = 1;
 			fci.params = &arg;
-			fci.no_separation = 0;
+			fci.no_separation = 1;
 
 			ZVAL_COPY(&arg, zv);
 			ret = zend_call_function(&fci, &fci_cache);
@@ -6151,7 +6151,7 @@ PHP_FUNCTION(array_map)
 				fci.retval = &result;
 				fci.param_count = n_arrays;
 				fci.params = params;
-				fci.no_separation = 0;
+				fci.no_separation = 1;
 
 				if (zend_call_function(&fci, &fci_cache) != SUCCESS || Z_TYPE(result) == IS_UNDEF) {
 					efree(array_pos);
diff --git a/ext/standard/tests/array/array_filter_variation7.phpt b/ext/standard/tests/array/array_filter_variation7.phpt
index d2a6255160..805f314244 100644
--- a/ext/standard/tests/array/array_filter_variation7.phpt
+++ b/ext/standard/tests/array/array_filter_variation7.phpt
@@ -14,10 +14,6 @@ $input = array(0, 1, -1, 10, 100, 1000, 'Hello', null);
 echo "Anonymous callback function with regular parameter and statement\n";
 var_dump( array_filter($input, function($input) { return ($input > 1); }) );
 
-// anonymous callback function with reference
-echo "Anonymous callback function with reference parameter\n";
-var_dump( array_filter($input, function(&$input) { return ($input < 1); }) );
-
 // anonymous callback function with null argument
 echo "Anonymous callback function with null argument\n";
 var_dump( array_filter($input, function() { return true; }) );
@@ -39,17 +35,6 @@ array(3) {
   [5]=>
   int(1000)
 }
-Anonymous callback function with reference parameter
-array(4) {
-  [0]=>
-  int(0)
-  [2]=>
-  int(-1)
-  [6]=>
-  string(5) "Hello"
-  [7]=>
-  NULL
-}
 Anonymous callback function with null argument
 array(8) {
   [0]=>
diff --git a/ext/standard/tests/array/array_map_variation2.phpt b/ext/standard/tests/array/array_map_variation2.phpt
index 0d93b33c81..c88e6fb2e9 100644
--- a/ext/standard/tests/array/array_map_variation2.phpt
+++ b/ext/standard/tests/array/array_map_variation2.phpt
@@ -8,9 +8,7 @@ $arr = array("k1" => "v1","k2"=>"v2");
 $arr[]=&$arr["k1"];
 $arr[]=&$arr;
 function cb1 ($a) {var_dump ($a);return array ($a);};
-function cb2 (&$a) {var_dump ($a);return array (&$a);};
 var_dump( array_map("cb1", $arr));
-var_dump( array_map("cb2", $arr,$arr));
 var_dump( array_map(null,  $arr));
 var_dump( array_map(null, $arr, $arr));
 
@@ -66,50 +64,6 @@ array(4) {
     }
   }
 }
-string(2) "v1"
-string(2) "v2"
-string(2) "v1"
-array(4) {
-  ["k1"]=>
-  &string(2) "v1"
-  ["k2"]=>
-  string(2) "v2"
-  [0]=>
-  &string(2) "v1"
-  [1]=>
-  *RECURSION*
-}
-array(4) {
-  [0]=>
-  array(1) {
-    [0]=>
-    &string(2) "v1"
-  }
-  [1]=>
-  array(1) {
-    [0]=>
-    string(2) "v2"
-  }
-  [2]=>
-  array(1) {
-    [0]=>
-    &string(2) "v1"
-  }
-  [3]=>
-  array(1) {
-    [0]=>
-    &array(4) {
-      ["k1"]=>
-      &string(2) "v1"
-      ["k2"]=>
-      string(2) "v2"
-      [0]=>
-      &string(2) "v1"
-      [1]=>
-      *RECURSION*
-    }
-  }
-}
 array(4) {
   ["k1"]=>
   &string(2) "v1"
diff --git a/ext/standard/tests/array/array_user_key_compare.phpt b/ext/standard/tests/array/array_user_key_compare.phpt
index 69bc37f48a..3901c3d2e2 100644
--- a/ext/standard/tests/array/array_user_key_compare.phpt
+++ b/ext/standard/tests/array/array_user_key_compare.phpt
@@ -15,5 +15,8 @@ uksort($arr, "array_compare");
 var_dump($a);
 
 ?>
---EXPECT--
+--EXPECTF--
+Warning: array_compare(): Argument #1 ($key1) must be passed by reference, value given in %s on line %d
+
+Warning: array_compare(): Argument #2 ($key2) must be passed by reference, value given in %s on line %d
 string(1) "B"
diff --git a/ext/standard/tests/array/array_walk_closure.phpt b/ext/standard/tests/array/array_walk_closure.phpt
index 6cc49cf9d3..68e56568e7 100644
--- a/ext/standard/tests/array/array_walk_closure.phpt
+++ b/ext/standard/tests/array/array_walk_closure.phpt
@@ -107,17 +107,23 @@ array(2) {
 bool(true)
 
 closure with array
+
+Warning: {closure}(): Argument #3 ($udata) must be passed by reference, value given in %s on line %d
 array(1) {
   ["sum"]=>
   int(42)
 }
+
+Warning: {closure}(): Argument #3 ($udata) must be passed by reference, value given in %s on line %d
 array(1) {
   ["sum"]=>
-  int(43)
+  int(42)
 }
+
+Warning: {closure}(): Argument #3 ($udata) must be passed by reference, value given in %s on line %d
 array(1) {
   ["sum"]=>
-  int(45)
+  int(42)
 }
 bool(true)
 End result:int(42)
@@ -139,14 +145,20 @@ bool(true)
 End result:int(48)
 
 closure with object
+
+Warning: {closure}(): Argument #3 ($udata) must be passed by reference, value given in %s on line %d
 object(stdClass)#1 (1) {
   ["sum"]=>
   int(42)
 }
+
+Warning: {closure}(): Argument #3 ($udata) must be passed by reference, value given in %s on line %d
 object(stdClass)#1 (1) {
   ["sum"]=>
   int(43)
 }
+
+Warning: {closure}(): Argument #3 ($udata) must be passed by reference, value given in %s on line %d
 object(stdClass)#1 (1) {
   ["sum"]=>
   int(45)
diff --git a/ext/standard/tests/array/bug28739.phpt b/ext/standard/tests/array/bug28739.phpt
index f7530973ea..df4f6d352d 100644
--- a/ext/standard/tests/array/bug28739.phpt
+++ b/ext/standard/tests/array/bug28739.phpt
@@ -6,8 +6,8 @@ class p {
    public $x;
    function __construct($x){$this->x=$x;}
 }
-function a(&$a, &$b){var_dump(__FUNCTION__);return $a->x - $b->x;}
-function b(&$a, &$b){var_dump(__FUNCTION__);return $a->x - $b->x;}
+function a($a, $b){var_dump(__FUNCTION__);return $a->x - $b->x;}
+function b($a, $b){var_dump(__FUNCTION__);return $a->x - $b->x;}
 
 $p1 = array(new p(2), new p(1), new p(0));
 $p2 = array(new p(0), new p(2), new p(3));
diff --git a/ext/standard/tests/array/bug39576.phpt b/ext/standard/tests/array/bug39576.phpt
index 3ac9756941..61600fa578 100644
--- a/ext/standard/tests/array/bug39576.phpt
+++ b/ext/standard/tests/array/bug39576.phpt
@@ -34,6 +34,14 @@ echo "Done\n";
 ?>
 --EXPECTF--
 Notice: Only variables should be passed by reference in %s on line %d
+
+Warning: test(): Argument #3 ($columns) must be passed by reference, value given in %s on line %d
+
+Warning: test(): Argument #3 ($columns) must be passed by reference, value given in %s on line %d
+
+Warning: test(): Argument #3 ($columns) must be passed by reference, value given in %s on line %d
+
+Warning: test(): Argument #3 ($columns) must be passed by reference, value given in %s on line %d
 object(Test)#%d (4) {
   ["_table"]=>
   string(0) ""
diff --git a/ext/standard/tests/array/bug52719.phpt b/ext/standard/tests/array/bug52719.phpt
index b3f0a31101..2cfeb734b4 100644
--- a/ext/standard/tests/array/bug52719.phpt
+++ b/ext/standard/tests/array/bug52719.phpt
@@ -11,5 +11,8 @@ array_walk_recursive(
 );
 echo "Done";
 ?>
---EXPECT--
+--EXPECTF--
+Warning: {closure}(): Argument #3 ($userdata) must be passed by reference, value given in %s on line %d
+
+Warning: {closure}(): Argument #3 ($userdata) must be passed by reference, value given in %s on line %d
 Done
diff --git a/ext/standard/tests/array/uasort_variation7.phpt b/ext/standard/tests/array/uasort_variation7.phpt
index fd92a9f856..ec3c6cdf39 100644
--- a/ext/standard/tests/array/uasort_variation7.phpt
+++ b/ext/standard/tests/array/uasort_variation7.phpt
@@ -34,7 +34,7 @@ var_dump($array_arg);
 
 echo "Done"
 ?>
---EXPECT--
+--EXPECTF--
 *** Testing uasort() : anonymous function as 'cmp_function' ***
 -- Anonymous 'cmp_function' with parameters passed by value --
 bool(true)
@@ -51,6 +51,22 @@ array(5) {
   int(100)
 }
 -- Anonymous 'cmp_function' with parameters passed by reference --
+
+Warning: {closure}(): Argument #1 ($value1) must be passed by reference, value given in %s on line %d
+
+Warning: {closure}(): Argument #2 ($value2) must be passed by reference, value given in %s on line %d
+
+Warning: {closure}(): Argument #1 ($value1) must be passed by reference, value given in %s on line %d
+
+Warning: {closure}(): Argument #2 ($value2) must be passed by reference, value given in %s on line %d
+
+Warning: {closure}(): Argument #1 ($value1) must be passed by reference, value given in %s on line %d
+
+Warning: {closure}(): Argument #2 ($value2) must be passed by reference, value given in %s on line %d
+
+Warning: {closure}(): Argument #1 ($value1) must be passed by reference, value given in %s on line %d
+
+Warning: {closure}(): Argument #2 ($value2) must be passed by reference, value given in %s on line %d
 bool(true)
 array(4) {
   ["a"]=>
diff --git a/ext/standard/tests/array/uasort_variation9.phpt b/ext/standard/tests/array/uasort_variation9.phpt
deleted file mode 100644
index 05e98032c8..0000000000
--- a/ext/standard/tests/array/uasort_variation9.phpt
+++ /dev/null
@@ -1,69 +0,0 @@
---TEST--
-Test uasort() function : usage variations - 'cmp_function' with reference argument
---FILE--
-<?php
-/* Testing uasort() functionality with comparison function having arguments as reference
- */
-
-echo "*** Testing uasort() : 'cmp_function' with reference arguments ***\n";
-
-// comparison function
-function cmp(&$value1, &$value2)
-{
-  if($value1 == $value2) {
-    return 0;
-  }
-  else if($value1 > $value2) {
-    return 1;
-  }
-  else
-    return -1;
-}
-
-// Int array with default keys
-$int_values = array(1, 8, 9, 3, 2, 6, 7);
-echo "-- Passing integer values to 'cmp_function' --\n";
-var_dump( uasort($int_values, 'cmp') );
-var_dump($int_values);
-
-// String array with default keys
-$string_values = array("Mango", "Apple", "Orange", "Banana");
-echo "-- Passing string values to 'cmp_function' --\n";
-var_dump( uasort($string_values, 'cmp') );
-var_dump($string_values);
-
-echo "Done"
-?>
---EXPECT--
-*** Testing uasort() : 'cmp_function' with reference arguments ***
--- Passing integer values to 'cmp_function' --
-bool(true)
-array(7) {
-  [0]=>
-  int(1)
-  [4]=>
-  int(2)
-  [3]=>
-  int(3)
-  [5]=>
-  int(6)
-  [6]=>
-  int(7)
-  [1]=>
-  int(8)
-  [2]=>
-  int(9)
-}
--- Passing string values to 'cmp_function' --
-bool(true)
-array(4) {
-  [1]=>
-  string(5) "Apple"
-  [3]=>
-  string(6) "Banana"
-  [0]=>
-  string(5) "Mango"
-  [2]=>
-  string(6) "Orange"
-}
-Done
diff --git a/ext/standard/tests/array/usort_variation7.phpt b/ext/standard/tests/array/usort_variation7.phpt
index aa814245de..02e2cac7f2 100644
--- a/ext/standard/tests/array/usort_variation7.phpt
+++ b/ext/standard/tests/array/usort_variation7.phpt
@@ -32,7 +32,7 @@ echo "\n-- Anonymous 'cmp_function' with parameters passed by reference --\n";
 var_dump( usort($array_arg, $cmp_function) );
 var_dump($array_arg);
 ?>
---EXPECT--
+--EXPECTF--
 *** Testing usort() : usage variation ***
 
 -- Anonymous 'cmp_function' with parameters passed by value --
@@ -51,6 +51,22 @@ array(5) {
 }
 
 -- Anonymous 'cmp_function' with parameters passed by reference --
+
+Warning: {closure}(): Argument #1 ($value1) must be passed by reference, value given in %s on line %d
+
+Warning: {closure}(): Argument #2 ($value2) must be passed by reference, value given in %s on line %d
+
+Warning: {closure}(): Argument #1 ($value1) must be passed by reference, value given in %s on line %d
+
+Warning: {closure}(): Argument #2 ($value2) must be passed by reference, value given in %s on line %d
+
+Warning: {closure}(): Argument #1 ($value1) must be passed by reference, value given in %s on line %d
+
+Warning: {closure}(): Argument #2 ($value2) must be passed by reference, value given in %s on line %d
+
+Warning: {closure}(): Argument #1 ($value1) must be passed by reference, value given in %s on line %d
+
+Warning: {closure}(): Argument #2 ($value2) must be passed by reference, value given in %s on line %d
 bool(true)
 array(4) {
   [0]=>