Skip to content

Commit f513987

Browse files
authored
Check parameters on compact() and throw warning if not string or array of strings (#6921)
compact() is documented (https://www.php.net/manual/en/function.compact) as a variadic function accepting parameters which are strings or arrays of strings referencing defined symbols. In actuality, passing nonsense parameters e.g. compact(true, 42) merely returns an empty array. I propose throwing a warning in these cases, to prevent silent bugs.
1 parent 32aff25 commit f513987

File tree

3 files changed

+23
-13
lines changed

3 files changed

+23
-13
lines changed

ext/standard/array.c

+6-3
Original file line numberDiff line numberDiff line change
@@ -2447,7 +2447,7 @@ PHP_FUNCTION(extract)
24472447
}
24482448
/* }}} */
24492449

2450-
static void php_compact_var(HashTable *eg_active_symbol_table, zval *return_value, zval *entry) /* {{{ */
2450+
static void php_compact_var(HashTable *eg_active_symbol_table, zval *return_value, zval *entry, uint32_t pos) /* {{{ */
24512451
{
24522452
zval *value_ptr, data;
24532453

@@ -2475,11 +2475,14 @@ static void php_compact_var(HashTable *eg_active_symbol_table, zval *return_valu
24752475
Z_PROTECT_RECURSION_P(entry);
24762476
}
24772477
ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(entry), value_ptr) {
2478-
php_compact_var(eg_active_symbol_table, return_value, value_ptr);
2478+
php_compact_var(eg_active_symbol_table, return_value, value_ptr, pos);
24792479
} ZEND_HASH_FOREACH_END();
24802480
if (Z_REFCOUNTED_P(entry)) {
24812481
Z_UNPROTECT_RECURSION_P(entry);
24822482
}
2483+
} else {
2484+
php_error_docref(NULL, E_WARNING, "Argument #%d must be string or array of strings, %s given", pos, zend_zval_type_name(entry));
2485+
return;
24832486
}
24842487
}
24852488
/* }}} */
@@ -2512,7 +2515,7 @@ PHP_FUNCTION(compact)
25122515
}
25132516

25142517
for (i = 0; i < num_args; i++) {
2515-
php_compact_var(symbol_table, return_value, &args[i]);
2518+
php_compact_var(symbol_table, return_value, &args[i], i + 1);
25162519
}
25172520
}
25182521
/* }}} */

ext/standard/tests/array/compact.phpt

+15
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,13 @@ $location_vars = array("c\\u0327ity", "state");
1111

1212
$result = compact("event", $location_vars);
1313
var_dump($result);
14+
15+
$result = compact(true);
16+
$foo = 'bar';
17+
$bar = 'baz';
18+
$result = compact($foo, [42]);
19+
var_dump($result);
20+
1421
?>
1522
--EXPECTF--
1623
Warning: compact(): Undefined variable $c\u0327ity in %s on line %d
@@ -20,3 +27,11 @@ array(2) {
2027
["state"]=>
2128
string(2) "CA"
2229
}
30+
31+
Warning: compact(): Argument #1 must be string or array of strings, bool given in %s on line %d
32+
33+
Warning: compact(): Argument #2 must be string or array of strings, int given in %s on line %d
34+
array(1) {
35+
["bar"]=>
36+
string(3) "baz"
37+
}

ext/standard/tests/array/compact_basic.phpt

+2-10
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,8 @@ $f="string";
1919
var_dump (compact(array("a", "b", "c", "d", "e", "f")));
2020
// simple parameter test
2121
var_dump (compact("a", "b", "c", "d", "e", "f"));
22-
var_dump (compact(array("keyval"=>"a", "b"=>"b", "c"=>1)));
23-
24-
// cases which should not yield any output.
25-
var_dump (compact(array(10, 0.3, true, array(20), NULL)));
26-
var_dump (compact(10, 0.3, true, array(20), NULL));
27-
var_dump (compact(array("g")));
22+
var_dump (compact(array("keyval"=>"a", "b"=>"b")));
23+
var_dump(compact(array("g")));
2824

2925
echo "Done";
3026
?>
@@ -70,10 +66,6 @@ array(2) {
7066
["b"]=>
7167
float(0.2)
7268
}
73-
array(0) {
74-
}
75-
array(0) {
76-
}
7769

7870
Warning: compact(): Undefined variable $g in %s on line %d
7971
array(0) {

0 commit comments

Comments
 (0)