Skip to content

Commit 34f727e

Browse files
GirgiasGirgias
committed
Use ZPP check for string|int|null arguments in array_column()
1 parent 7b74fc7 commit 34f727e

5 files changed

+146
-75
lines changed

‎ext/standard/array.c

Lines changed: 33 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -4083,103 +4083,81 @@ PHP_FUNCTION(array_count_values)
40834083
}
40844084
/* }}} */
40854085

4086-
/* {{{ array_column_param_helper
4087-
* Specialized conversion rules for array_column() function
4088-
*/
4089-
static inline
4090-
zend_bool array_column_param_helper(zval *param, int parameter_number) {
4091-
switch (Z_TYPE_P(param)) {
4092-
case IS_DOUBLE:
4093-
convert_to_long_ex(param);
4094-
/* fallthrough */
4095-
case IS_LONG:
4096-
return 1;
4097-
4098-
case IS_OBJECT:
4099-
if (!try_convert_to_string(param)) {
4100-
return 0;
4101-
}
4102-
/* fallthrough */
4103-
case IS_STRING:
4104-
return 1;
4105-
4106-
default:
4107-
zend_argument_type_error(parameter_number, "must be of type string|int, %s given", zend_zval_type_name(param));
4108-
return 0;
4109-
}
4110-
}
4111-
/* }}} */
4112-
4113-
static inline zval *array_column_fetch_prop(zval *data, zval *name, zval *rv) /* {{{ */
4086+
static inline zval *array_column_fetch_prop(zval *data, zend_string *name_str, zend_long name_long, zval *rv) /* {{{ */
41144087
{
41154088
zval *prop = NULL;
41164089

41174090
if (Z_TYPE_P(data) == IS_OBJECT) {
4091+
zend_string *tmp_str;
4092+
/* If name is an integer convert integer to string */
4093+
if (name_str == NULL) {
4094+
tmp_str = zend_long_to_str(name_long);
4095+
} else {
4096+
tmp_str = zend_string_copy(name_str);
4097+
}
41184098
/* The has_property check is first performed in "exists" mode (which returns true for
41194099
* properties that are null but exist) and then in "has" mode to handle objects that
41204100
* implement __isset (which is not called in "exists" mode). */
4121-
zend_string *str, *tmp_str;
4122-
4123-
str = zval_get_tmp_string(name, &tmp_str);
4124-
if (Z_OBJ_HANDLER_P(data, has_property)(Z_OBJ_P(data), str, ZEND_PROPERTY_EXISTS, NULL)
4125-
|| Z_OBJ_HANDLER_P(data, has_property)(Z_OBJ_P(data), str, ZEND_PROPERTY_ISSET, NULL)) {
4126-
prop = Z_OBJ_HANDLER_P(data, read_property)(Z_OBJ_P(data), str, BP_VAR_R, NULL, rv);
4101+
if (Z_OBJ_HANDLER_P(data, has_property)(Z_OBJ_P(data), tmp_str, ZEND_PROPERTY_EXISTS, NULL)
4102+
|| Z_OBJ_HANDLER_P(data, has_property)(Z_OBJ_P(data), tmp_str, ZEND_PROPERTY_ISSET, NULL)) {
4103+
prop = Z_OBJ_HANDLER_P(data, read_property)(Z_OBJ_P(data), tmp_str, BP_VAR_R, NULL, rv);
41274104
if (prop) {
41284105
ZVAL_DEREF(prop);
41294106
if (prop != rv) {
41304107
Z_TRY_ADDREF_P(prop);
41314108
}
41324109
}
41334110
}
4134-
zend_tmp_string_release(tmp_str);
4111+
zend_string_release(tmp_str);
41354112
} else if (Z_TYPE_P(data) == IS_ARRAY) {
4136-
if (Z_TYPE_P(name) == IS_STRING) {
4137-
prop = zend_symtable_find(Z_ARRVAL_P(data), Z_STR_P(name));
4138-
} else if (Z_TYPE_P(name) == IS_LONG) {
4139-
prop = zend_hash_index_find(Z_ARRVAL_P(data), Z_LVAL_P(name));
4113+
/* Name is a string */
4114+
if (name_str != NULL) {
4115+
prop = zend_symtable_find(Z_ARRVAL_P(data), name_str);
4116+
} else {
4117+
prop = zend_hash_index_find(Z_ARRVAL_P(data), name_long);
41404118
}
41414119
if (prop) {
41424120
ZVAL_DEREF(prop);
41434121
Z_TRY_ADDREF_P(prop);
41444122
}
41454123
}
41464124

4147-
41484125
return prop;
41494126
}
41504127
/* }}} */
41514128

4152-
/* {{{ proto array array_column(array input, mixed column_key[, mixed index_key])
4129+
/* {{{ proto array array_column(array input, string|int|null column_key[, string|int|null index_key])
41534130
Return the values from a single column in the input array, identified by the
41544131
value_key and optionally indexed by the index_key */
41554132
PHP_FUNCTION(array_column)
41564133
{
41574134
HashTable *input;
41584135
zval *colval, *data, rv;
4159-
zval *column = NULL, *index = NULL;
4136+
zend_string *column_str = NULL;
4137+
zend_long column_long;
4138+
zend_bool column_is_null = 0;
4139+
zend_string *index_str = NULL;
4140+
zend_long index_long;
4141+
zend_bool index_is_null = 1;
41604142

41614143
ZEND_PARSE_PARAMETERS_START(2, 3)
41624144
Z_PARAM_ARRAY_HT(input)
4163-
Z_PARAM_ZVAL_EX(column, 1, 0)
4145+
Z_PARAM_STR_OR_LONG_OR_NULL(column_str, column_long, column_is_null)
41644146
Z_PARAM_OPTIONAL
4165-
Z_PARAM_ZVAL_EX(index, 1, 0)
4147+
Z_PARAM_STR_OR_LONG_OR_NULL(index_str, index_long, index_is_null)
41664148
ZEND_PARSE_PARAMETERS_END();
41674149

4168-
if ((column && !array_column_param_helper(column, 2)) ||
4169-
(index && !array_column_param_helper(index, 3))) {
4170-
RETURN_THROWS();
4171-
}
4172-
41734150
array_init_size(return_value, zend_hash_num_elements(input));
4174-
if (!index) {
4151+
/* Index param is not passed */
4152+
if (index_is_null) {
41754153
zend_hash_real_init_packed(Z_ARRVAL_P(return_value));
41764154
ZEND_HASH_FILL_PACKED(Z_ARRVAL_P(return_value)) {
41774155
ZEND_HASH_FOREACH_VAL(input, data) {
41784156
ZVAL_DEREF(data);
4179-
if (!column) {
4157+
if (column_is_null) {
41804158
Z_TRY_ADDREF_P(data);
41814159
colval = data;
4182-
} else if ((colval = array_column_fetch_prop(data, column, &rv)) == NULL) {
4160+
} else if ((colval = array_column_fetch_prop(data, column_str, column_long, &rv)) == NULL) {
41834161
continue;
41844162
}
41854163
ZEND_HASH_FILL_ADD(colval);
@@ -4189,18 +4167,18 @@ PHP_FUNCTION(array_column)
41894167
ZEND_HASH_FOREACH_VAL(input, data) {
41904168
ZVAL_DEREF(data);
41914169

4192-
if (!column) {
4170+
if (column_is_null) {
41934171
Z_TRY_ADDREF_P(data);
41944172
colval = data;
4195-
} else if ((colval = array_column_fetch_prop(data, column, &rv)) == NULL) {
4173+
} else if ((colval = array_column_fetch_prop(data, column_str, column_long, &rv)) == NULL) {
41964174
continue;
41974175
}
41984176

41994177
/* Failure will leave keyval alone which will land us on the final else block below
42004178
* which is to append the value as next_index
42014179
*/
42024180
zval rv;
4203-
zval *keyval = array_column_fetch_prop(data, index, &rv);
4181+
zval *keyval = array_column_fetch_prop(data, index_str, index_long, &rv);
42044182

42054183
if (keyval) {
42064184
switch (Z_TYPE_P(keyval)) {

‎ext/standard/basic_functions.stub.php

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -176,11 +176,7 @@ function array_values(array $arg): array {}
176176

177177
function array_count_values(array $arg): array {}
178178

179-
/**
180-
* @param int|string|null $column_key
181-
* @param int|string|null $index_key
182-
*/
183-
function array_column(array $arg, $column_key, $index_key = null): array {}
179+
function array_column(array $arg, int|string|null $column_key, int|string|null $index_key = null): array {}
184180

185181
function array_reverse(array $input, bool $preserve_keys = false): array {}
186182

‎ext/standard/basic_functions_arginfo.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -245,8 +245,8 @@ ZEND_END_ARG_INFO()
245245

246246
ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_array_column, 0, 2, IS_ARRAY, 0)
247247
ZEND_ARG_TYPE_INFO(0, arg, IS_ARRAY, 0)
248-
ZEND_ARG_INFO(0, column_key)
249-
ZEND_ARG_INFO_WITH_DEFAULT_VALUE(0, index_key, "null")
248+
ZEND_ARG_TYPE_MASK(0, column_key, MAY_BE_LONG|MAY_BE_STRING|MAY_BE_NULL, NULL)
249+
ZEND_ARG_TYPE_MASK(0, index_key, MAY_BE_LONG|MAY_BE_STRING|MAY_BE_NULL, "null")
250250
ZEND_END_ARG_INFO()
251251

252252
ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_array_reverse, 0, 1, IS_ARRAY, 0)

‎ext/standard/tests/array/array_column_error.phpt renamed to ‎ext/standard/tests/array/array_column_scalar_index_strict_types.phpt

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,40 +1,49 @@
11
--TEST--
2-
Test array_column() function: error conditions
2+
Test array_column(): Index argument with various types in strict type mode
33
--FILE--
44
<?php
5+
declare(strict_types=1);
56
/* Prototype:
67
* array array_column(array $input, mixed $column_key[, mixed $index_key]);
78
* Description:
89
* Returns an array containing all the values from
910
* the specified "column" in a two-dimensional array.
1011
*/
1112

12-
echo "*** Testing array_column() : error conditions ***\n";
1313
echo "\n-- Testing array_column() column key parameter should be a string or an integer (testing bool) --\n";
1414
try {
15-
var_dump(array_column(array(), true));
15+
var_dump(array_column([['php7', 'foo'], ['php8', 'bar']], false));
16+
} catch (\TypeError $e) {
17+
echo $e->getMessage() . "\n";
18+
}
19+
try {
20+
var_dump(array_column([['php7', 'foo'], ['php8', 'bar']], true));
1621
} catch (\TypeError $e) {
1722
echo $e->getMessage() . "\n";
1823
}
19-
2024

2125
echo "\n-- Testing array_column() column key parameter should be a string or integer (testing array) --\n";
2226
try {
23-
var_dump(array_column(array(), array()));
27+
var_dump(array_column([['php7', 'foo'], ['php8', 'bar']], array()));
2428
} catch (\TypeError $e) {
2529
echo $e->getMessage() . "\n";
2630
}
2731

2832
echo "\n-- Testing array_column() index key parameter should be a string or an integer (testing bool) --\n";
2933
try {
30-
var_dump(array_column(array(), 'foo', true));
34+
var_dump(array_column([['php' => 7, 'foo'], ['php' => 8, 'bar']], 'php', false));
35+
} catch (\TypeError $e) {
36+
echo $e->getMessage() . "\n";
37+
}
38+
try {
39+
var_dump(array_column([['php' => 7, 'foo'], ['php' => 8, 'bar']], 'php', true));
3140
} catch (\TypeError $e) {
3241
echo $e->getMessage() . "\n";
3342
}
3443

3544
echo "\n-- Testing array_column() index key parameter should be a string or integer (testing array) --\n";
3645
try {
37-
var_dump(array_column(array(), 'foo', array()));
46+
var_dump(array_column([['php' => 7, 'foo'], ['php' => 8, 'bar']], 'php', array()));
3847
} catch (\TypeError $e) {
3948
echo $e->getMessage() . "\n";
4049
}
@@ -43,18 +52,18 @@ try {
4352

4453
DONE
4554
--EXPECT--
46-
*** Testing array_column() : error conditions ***
47-
4855
-- Testing array_column() column key parameter should be a string or an integer (testing bool) --
49-
array_column(): Argument #2 ($column_key) must be of type string|int, bool given
56+
array_column(): Argument #2 ($column_key) must be of type string|int|null, bool given
57+
array_column(): Argument #2 ($column_key) must be of type string|int|null, bool given
5058

5159
-- Testing array_column() column key parameter should be a string or integer (testing array) --
52-
array_column(): Argument #2 ($column_key) must be of type string|int, array given
60+
array_column(): Argument #2 ($column_key) must be of type string|int|null, array given
5361

5462
-- Testing array_column() index key parameter should be a string or an integer (testing bool) --
55-
array_column(): Argument #3 ($index_key) must be of type string|int, bool given
63+
array_column(): Argument #3 ($index_key) must be of type string|int|null, bool given
64+
array_column(): Argument #3 ($index_key) must be of type string|int|null, bool given
5665

5766
-- Testing array_column() index key parameter should be a string or integer (testing array) --
58-
array_column(): Argument #3 ($index_key) must be of type string|int, array given
67+
array_column(): Argument #3 ($index_key) must be of type string|int|null, array given
5968

6069
DONE

‎ext/standard/tests/array/array_column_scalar_index_weak_types.phpt

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
--TEST--
2+
Test array_column(): Index argument with various types in weak type mode
3+
--FILE--
4+
<?php
5+
/* Prototype:
6+
* array array_column(array $input, mixed $column_key[, mixed $index_key]);
7+
* Description:
8+
* Returns an array containing all the values from
9+
* the specified "column" in a two-dimensional array.
10+
*/
11+
12+
echo "\n-- Testing array_column() column key parameter should be a string or an integer (testing bool) --\n";
13+
try {
14+
var_dump(array_column([['php7', 'foo'], ['php8', 'bar']], false));
15+
} catch (\TypeError $e) {
16+
echo $e->getMessage() . "\n";
17+
}
18+
try {
19+
var_dump(array_column([['php7', 'foo'], ['php8', 'bar']], true));
20+
} catch (\TypeError $e) {
21+
echo $e->getMessage() . "\n";
22+
}
23+
24+
echo "\n-- Testing array_column() column key parameter should be a string or integer (testing array) --\n";
25+
try {
26+
var_dump(array_column([['php7', 'foo'], ['php8', 'bar']], array()));
27+
} catch (\TypeError $e) {
28+
echo $e->getMessage() . "\n";
29+
}
30+
31+
echo "\n-- Testing array_column() index key parameter should be a string or an integer (testing bool) --\n";
32+
try {
33+
var_dump(array_column([['php' => 7, 'foo'], ['php' => 8, 'bar']], 'php', false));
34+
} catch (\TypeError $e) {
35+
echo $e->getMessage() . "\n";
36+
}
37+
try {
38+
var_dump(array_column([['php' => 7, 'foo'], ['php' => 8, 'bar']], 'php', true));
39+
} catch (\TypeError $e) {
40+
echo $e->getMessage() . "\n";
41+
}
42+
43+
echo "\n-- Testing array_column() index key parameter should be a string or integer (testing array) --\n";
44+
try {
45+
var_dump(array_column([['php' => 7, 'foo'], ['php' => 8, 'bar']], 'php', array()));
46+
} catch (\TypeError $e) {
47+
echo $e->getMessage() . "\n";
48+
}
49+
50+
?>
51+
52+
DONE
53+
--EXPECT--
54+
-- Testing array_column() column key parameter should be a string or an integer (testing bool) --
55+
array(2) {
56+
[0]=>
57+
string(4) "php7"
58+
[1]=>
59+
string(4) "php8"
60+
}
61+
array(2) {
62+
[0]=>
63+
string(3) "foo"
64+
[1]=>
65+
string(3) "bar"
66+
}
67+
68+
-- Testing array_column() column key parameter should be a string or integer (testing array) --
69+
array_column(): Argument #2 ($column_key) must be of type string|int|null, array given
70+
71+
-- Testing array_column() index key parameter should be a string or an integer (testing bool) --
72+
array(2) {
73+
["foo"]=>
74+
int(7)
75+
["bar"]=>
76+
int(8)
77+
}
78+
array(2) {
79+
[0]=>
80+
int(7)
81+
[1]=>
82+
int(8)
83+
}
84+
85+
-- Testing array_column() index key parameter should be a string or integer (testing array) --
86+
array_column(): Argument #3 ($index_key) must be of type string|int|null, array given
87+
88+
DONE

0 commit comments

Comments
 (0)