Skip to content

Check parameters on compact() and throw TypeError if not string or array of strings #6921

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
May 11, 2021
Merged

Check parameters on compact() and throw TypeError if not string or array of strings #6921

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e4672fd
Check parameters on compact() and throw TypeError
dwgebler Apr 28, 2021
f35c7e0
remove old test cases which no longer yield empty output
dwgebler Apr 28, 2021
da2118c
remove integer val from array test
dwgebler Apr 28, 2021
aada56e
address gpb's review comments
dwgebler Apr 28, 2021
97b22e4
change typeerror message and corresponding tests
dwgebler Apr 28, 2021
1f62ace
revert to warning on invalid parameter
dwgebler May 8, 2021
3655199
restore undefined variable test
dwgebler May 8, 2021
2361002
add var dump to test
dwgebler May 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions ext/standard/array.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2447,7 +2447,7 @@ PHP_FUNCTION(extract)
}
/* }}} */

static void php_compact_var(HashTable *eg_active_symbol_table, zval *return_value, zval *entry) /* {{{ */
static void php_compact_var(HashTable *eg_active_symbol_table, zval *return_value, zval *entry, uint32_t pos) /* {{{ */
{
zval *value_ptr, data;

Expand Down Expand Up @@ -2475,11 +2475,14 @@ static void php_compact_var(HashTable *eg_active_symbol_table, zval *return_valu
Z_PROTECT_RECURSION_P(entry);
}
ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(entry), value_ptr) {
php_compact_var(eg_active_symbol_table, return_value, value_ptr);
php_compact_var(eg_active_symbol_table, return_value, value_ptr, pos);
} ZEND_HASH_FOREACH_END();
if (Z_REFCOUNTED_P(entry)) {
Z_UNPROTECT_RECURSION_P(entry);
}
} else {
php_error_docref(NULL, E_WARNING, "Argument #%d must be string or array of strings, %s given", pos, zend_zval_type_name(entry));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error message is a bit imprecise when it comes to values in arrays, but I think it's good enough to go on...

return;
}
}
/* }}} */
Expand Down Expand Up @@ -2512,7 +2515,7 @@ PHP_FUNCTION(compact)
}

for (i = 0; i < num_args; i++) {
php_compact_var(symbol_table, return_value, &args[i]);
php_compact_var(symbol_table, return_value, &args[i], i + 1);
}
}
/* }}} */
Expand Down
15 changes: 15 additions & 0 deletions ext/standard/tests/array/compact.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@ $location_vars = array("c\\u0327ity", "state");

$result = compact("event", $location_vars);
var_dump($result);

$result = compact(true);
$foo = 'bar';
$bar = 'baz';
$result = compact($foo, [42]);
var_dump($result);

?>
--EXPECTF--
Warning: compact(): Undefined variable $c\u0327ity in %s on line %d
Expand All @@ -20,3 +27,11 @@ array(2) {
["state"]=>
string(2) "CA"
}

Warning: compact(): Argument #1 must be string or array of strings, bool given in %s on line %d

Warning: compact(): Argument #2 must be string or array of strings, int given in %s on line %d
array(1) {
["bar"]=>
string(3) "baz"
}
12 changes: 2 additions & 10 deletions ext/standard/tests/array/compact_basic.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,8 @@ $f="string";
var_dump (compact(array("a", "b", "c", "d", "e", "f")));
// simple parameter test
var_dump (compact("a", "b", "c", "d", "e", "f"));
var_dump (compact(array("keyval"=>"a", "b"=>"b", "c"=>1)));

// cases which should not yield any output.
var_dump (compact(array(10, 0.3, true, array(20), NULL)));
var_dump (compact(10, 0.3, true, array(20), NULL));
var_dump (compact(array("g")));
var_dump (compact(array("keyval"=>"a", "b"=>"b")));
var_dump(compact(array("g")));

echo "Done";
?>
Expand Down Expand Up @@ -70,10 +66,6 @@ array(2) {
["b"]=>
float(0.2)
}
array(0) {
}
array(0) {
}

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