Skip to content

ext/filter: Narrow the return type of filter_var_array() to array|false - #23403

Open
lacatoire wants to merge 1 commit into
php:masterfrom
lacatoire:fix/filter-var-array-return-type
Open

ext/filter: Narrow the return type of filter_var_array() to array|false#23403
lacatoire wants to merge 1 commit into
php:masterfrom
lacatoire:fix/filter-var-array-return-type

Conversation

@lacatoire

Copy link
Copy Markdown
Member

filter_var_array() declares array|false|null, but null is unreachable.

The function has exactly two exits: RETURN_FALSE when the filter id is unknown, and php_filter_array_handler(), which establishes an array on both of its branches before doing anything else — ZVAL_DUP() of the input, which is a Z_PARAM_ARRAY, or array_init(). The handler's remaining exits are RETURN_THROWS().

The sibling filter_input_array() is declared identically and does return null, deliberately, since its source superglobal may not exist:

if (!array_input) {
	RETURN_NULL();
}

That case has no equivalent in filter_var_array(), whose source is a required array parameter. The two signatures look symmetrical but only one of them can produce null.

Verification

Swept every registered filter from filter_list(), as the global int form and inside an options array, across six flag combinations including FILTER_NULL_ON_FAILURE, FILTER_FORCE_ARRAY and FILTER_REQUIRE_SCALAR, against inputs chosen to make filters fail, with $add_empty both ways. 3024 calls on 8.5.4: array 2848 times, TypeError 88, ValueError 88, plus false for the unknown filter id. null never appears.

Worth noting that null remains an ordinary element value in the returned array, which is likely where the wider union came from:

filter_var_array(['a' => 'x'], ['a' => ['filter' => FILTER_VALIDATE_BOOLEAN, 'flags' => FILTER_NULL_ON_FAILURE]]);

That is the value type, not the return type.

Only the stub changes; the arginfo is regenerated and the edit is confined to arginfo_filter_var_array, filter_input_array keeps MAY_BE_NULL.

Note

This is behaviour-preserving on the engine side, but it does tighten what userland is told, and it shows up in ReflectionFunction::getReturnType(). Whether that belongs in a minor or should wait for a major is the maintainers' call.

The stub declares array|false|null, but null is unreachable. The function
has two exits: RETURN_FALSE for an unknown filter id, and the array handler,
which establishes an array on both of its branches before doing anything
else. Its remaining exits throw.

The sibling filter_input_array() is declared identically and does return
null, deliberately, because its source superglobal may not exist. That case
has no equivalent here, where the source is a required array parameter.

null remains an ordinary element value in the returned array, which is
likely where the wider union came from; that is the value type, not the
return type.

@LamentXU123 LamentXU123 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi, In zend_func_infos.h L98

	F1("filter_var_array", MAY_BE_ARRAY|MAY_BE_ARRAY_KEY_LONG|MAY_BE_ARRAY_KEY_STRING|MAY_BE_ARRAY_OF_ANY|MAY_BE_ARRAY_OF_REF|MAY_BE_FALSE|MAY_BE_NULL),

MAY_BE_NULL is still in there. So you will need to also remove this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants