Inconsistent behavior of `filter_var()` with `FILTER_REQUIRE_ARRAY` and `default` option for non-array inputs

[rubenferreira97]

Description

The following code:

<?php

var_dump(filter_var("a", FILTER_VALIDATE_INT, ['flags' => FILTER_REQUIRE_ARRAY, 'options' => ['default' => 'Failed']]));
var_dump(filter_var([1], FILTER_VALIDATE_INT, ['flags' => FILTER_REQUIRE_ARRAY, 'options' => ['default' => 'Failed']]));
var_dump(filter_var(["a"], FILTER_VALIDATE_INT, ['flags' => FILTER_REQUIRE_ARRAY, 'options' => ['default' => 'Failed']]));
var_dump(filter_var("a", FILTER_VALIDATE_INT, ['options' => ['default' => 'Failed']]));

Resulted in this output:

bool(false)
array(1) {[0]=> int(1)}
array(1) {[0]=> string(6) "Failed"}
string(6) "Failed"

But I expected this output instead:

array(1) {[0]=> string(6) "Failed"} or string(6) "Failed"
array(1) {[0]=> int(1)}
array(1) {[0]=> string(6) "Failed"}
string(6) "Failed"

Since the documentation says:

default, value to return if the filter fails

I was expecting to get at least some form of Failed.

PHP Version

PHP 8.3

Operating System

Windows 11

[nielsdos]

This happens because only php_zval_filter has a handle_default piece of code; however we don't even get there because we're hitting this:

php-src/ext/filter/filter.c

Lines 516 to 524 in a165f1f

	if (filter_flags & FILTER_REQUIRE_ARRAY) {
	zval_ptr_dtor(filtered);
	if (filter_flags & FILTER_NULL_ON_FAILURE) {
	ZVAL_NULL(filtered);
	} else {
	ZVAL_FALSE(filtered);
	}
	return;
	}

And this happens before php_zval_filter is called.

The question I'm having is whether it makes sense to change this; and I'd guess "yes it is" but would like input. Also note that since this is a behaviour change in a sensitive component, we may want to only fix this in 8.5 and up.

Looking at the C code it seems like this is an oversight instead of a design decision.

Looking at the docs; from https://www.php.net/manual/en/function.filter-var.php:

Returns the filtered data, or false if the filter fails.

Although that's inaccurate because NULL can be returned too via a flag; but we learn that a falsy return value means failure. And looking at the code example in the description of the $options parameter I read 'default' => 3, // value to return if the filter fails; so to me it seems that if default is provided we should indeed return that value instead of NULL/false.

Anyway, patching this is not hard: https://gist.github.com/nielsdos/4952717159d6050e29e0282619faf0b6
cc @cmb69 @Girgias Do you agree with the interpretation and think we should fix this (on master)?

[Girgias]

There are a bunch of bugs and inconsistent behaviours in ext/filter due to its architecture, so I'm not that surprised about finding this out.

But I think fixing it makes sense, and I would agree to have this only in master.
Not sure if this bug fix could land in 8.4 but maybe RMs would be fine with it.

[cmb69]

Thanks @nielsdos for the good analysis. I'm not really sure how default is supposed to work in combination with FILTER_REQUIRE_ARRAY. Possibly, it was actually meant to not apply, if the given value is not an array at all. Changing that may break existing code, so I would not fix it for PHP 8.4. It might be okay to fix that for maste (and possibly revert if we get user complaints). Alternatively, we leave it as is, and fix the documentation accordingly. I slightly lean towards the latter (given that this is likely long standing behavior).

Fictional snippet:

$options = ['flags' => FILTER_REQUIRE_ARRAY, 'options' => ['default' => null]];
if (($input = filter_var($_GET["ids"], FILTER_VALIDATE_INT, $options)) === false) {
    // not an array; handle error
}
$input = array_filter($input, null);

[rubenferreira97]

@cmb69, if default is not supposed to work with FILTER_REQUIRE_ARRAY, then the following example is inconsistent:

var_dump(filter_var(["a", 1, "b"], FILTER_VALIDATE_INT, ['flags' => FILTER_REQUIRE_ARRAY, 'options' => ['default' => 'Failed']])); 

It outputs:

array(3) {
  [0]=>
  string(6) "Failed"
  [1]=>
  int(1)
  [2]=>
  string(6) "Failed"
}

---

I might be biased because I opened the issue, but I think it would be useful if filter_var could accept a default value on validation errors for any flags.

In addition to being inconsistent, the reason for this is that false (the default value for failed validation) and null (the value for failed validation when FILTER_NULL_ON_FAILURE is applied) are valid inputs in some cases.

To disambiguate these cases, I would resort to the following solution:

const SENTINEL_FAIL = new stdClass();

$input = "invalid"; // valid $input is true, false, or null (unknown);
$state = filter_var($input, FILTER_VALIDATE_BOOL, ['options' => ['default' => SENTINEL_FAIL]]);
assert(is_array($state) ? !in_array(SENTINEL_FAIL, $state, true) : $state !== SENTINEL_FAIL);

// If filter_var just returned the default without caring if this is an array or not I would just need:
// assert($state !== SENTINEL_FAIL) but we might want to know which element failed so I understand this implementation.

$input = [true, "fail"]; // valid $input is an array of booleans.
$state = filter_var($input, FILTER_VALIDATE_BOOL, ['options' => ['default' => SENTINEL_FAIL]]);
assert(is_array($state) ? !in_array(SENTINEL_FAIL, $state, true) : $state !== SENTINEL_FAIL);

[cmb69]

if default is not supposed to work with FILTER_REQUIRE_ARRAY, then the following example is inconsistent:

I meant that it might be best if default only used for the array values; if the input is not an array, returning false isn't the worst option. (It seems to me that not properly catering to possible string/array input is a common issue with legacy code at least.)