php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Request #40947 filter_var_aray() fails with a "common" filter
Submitted: 2007-03-29 10:25 UTC Modified: 2007-04-04 20:51 UTC
From: exaton at free dot fr Assigned: pajoye (profile)
Status: Closed Package: Feature/Change Request
PHP Version: 5.2.1 OS: WinXP SP2, Debian
Private report: No CVE-ID: None
 [2007-03-29 10:25 UTC] exaton at free dot fr
Description:
------------
[Tested on Windows XP SP2 / Apache 2.0.55 and on a Debian with kernel 2.6.16 / Apache 2.0.54 / PHP having the Suhosin-Patch applied].

Hello,

While reading http://phpro.org/tutorials/Filtering-Data-with-PHP.html, a tutorial on using the new PHP Filter extension, I came across a simple test case that fails unexpectedly.

$a = array(10, "109", "", "-1234", "some text", "asdf234asdfgs", array());
var_dump(filter_var_array($a, FILTER_VALIDATE_INT));

Produces bool(false). I would have expected it to produce the same result at :

var_dump(filter_var($a, FILTER_VALIDATE_INT, array(
	'flags' => FILTER_REQUIRE_ARRAY
)));

which displays array(7) { [0]=>  int(10) [1]=>  int(109) [2]=>  bool(false) [3]=>  int(-1234) [4]=>  bool(false) [5]=>  bool(false) [6]=>  array(0) { } } (int values for valid int entries, bool(false) for invalid entries, as shown in the tutorial).

The manual entry for filter_var_array() indicates, regarding the second parameter : "This parameter can be also an integer holding a filter constant. Then all values in the input array are filtered by this filter.".

I looked at the code of ext/filter in CVS and saw that this behavior seems to be applied, boiling everything down to a single function. I could not follow the code precisely enough (for lack of experience with  PHP internals) to locate a significant divergence, however.

On another note, the manual entry for filter_var_array() provides a code sample containing some constants which do not appear on the manual entry for the Filter extension ; namely, FILTER_FLAG_ARRAY and FILTER_FLAG_SCALAR. In the manual entry for filter_input_array(), which gives the same code sample, these constants have been changed to the existing FILTER_REQUIRE_ARRAY and FILTER_REQUIRE_SCALAR.

Thanks in advance !


Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2007-03-29 12:02 UTC] pajoye@php.net
What you are looking for is:

$c = filter_var($a, FILTER_VALIDATE_INT,FILTER_REQUIRE_ARRAY  );

(Use FILTER_FORCE_ARRAY if you like to always get an array in return, no matter if $a is an array or not).

filter_var_array works on a argument => definition basis, not to apply the same filter to all input.
 [2007-03-29 12:28 UTC] exaton at free dot fr
Hi Tony, Derick, Pierre-Alain,

Thank you for your extremely speedy feedback on this issue. I believe that it does still remain, however.

Regarding filter_var($a, FILTER_VALIDATE_INT, FILTER_REQUIRE_ARRAY) : that is indeed the second piece of code that I presented, to illustrate the expected result. I had placed the FILTER_REQUIRE_ARRAY flag in an option array, superfluously but to the same effect, I agree.

filter_var_array() certainly does work on an argument => definition basis, but as the manual states (re. my quote), "This [second parameter to the filter_var_array() function] can be also an integer holding a filter constant. Then all values in the input array are filtered by this filter.".

A good look at the underlying code in CVS confirms this. I am looking at .../php-src/ext/filter/filter.c?revision=1.52.2.38&view=markup and I see :

Lines 818-820, within PHP_FUNCTION(filter_var_array), the test Z_TYPE_PP(op) == IS_LONG && !PHP_FILTER_ID_EXISTS(Z_LVAL_PP(op)) clearly states that op, in essence the second parameter, may be an integer (that test is next to || Z_TYPE_PP(op) != IS_ARRAY, which allows....

Hey I've found the bug ^^

OK, so those lines say "if the second parameter (op) is a number but does not refer to an existing filter ID, ***or*** if op is not an array, then RETURN_FALSE". That should be *and*, not or ! If the second parameter is not an existing filter ID constant *and* if it is not an array, then it is not valid.

I was going to follow the rest of the trail, through to php_filter_array_handler() (called on line 823) and then php_filter_call() (called on line 649 with FILTER_REQUIRE_ARRAY, hence the equivalence of the two pieces of PHP code, they converge here)... but I think that the problem is on line 819, where there should be && and not ||.

Thanks again :)
 [2007-03-29 12:39 UTC] tony2001@php.net
Pierre, I believe the function that filters array should return array by default without any additional flags.
The documentation agrees with me, so something definitely should be fixed - either the docs or the function.
I prefer fixing the function and Derick said he'll take a look at it.

 [2007-03-29 12:45 UTC] pajoye@php.net
"filter_var_array() certainly does work on an argument => definition
basis, but as the manual states (re. my quote),"

There is a mistake in the manual. It was not thought and implemented to follow this definition.

The common function is used to filter an array of value with one or many filters but it is not related to the public API (filter_var and filter_var_array).

"filter_var_array() certainly does work on an argument => definition
basis, but as the manual states (re. my quote),"

There is BC reason here. The initial code allows or not array of values (of same type).

"Lines 818-820, within PHP_FUNCTION(filter_var_array), the test
Z_TYPE_PP(op) == IS_LONG && !PHP_FILTER_ID_EXISTS(Z_LVAL_PP(op)) clearly states that op, in essence the second parameter, may be an integer (that test is next to || Z_TYPE_PP(op) != IS_ARRAY, which allows...."

"IS_LONG && EXISTS" means: The filter ID must be an integer and the ID must exist. It accpepts array as the filter options/flags/id can be given as an array, in this case the ID validation is done later.

In any case, if you provide an invalid filter ID, the complete call is invalid as your code obviously has a (critical) bug.

"Hey I've found the bug ^^"

Sorry, this is not a bug but the expected behavior :)

If you like to filter an array of values using one filter, please use filter_var or provide a one definition.
 [2007-03-29 12:55 UTC] pajoye@php.net
Derick, there is no bug here. Please keep it closed.

Or do you have something that I missed?
 [2007-03-29 12:58 UTC] exaton at free dot fr
Erm, fine, I can live with using filter_var() instead of filter_var_array() for this purpose, I'm not a big fan of multiple ways of doing the same thing anyway.

I would like to get back to lines 818-819 of filter.c, though :

"OK, so those lines say "if the second parameter (op) is a number but
does not refer to an existing filter ID, ***or*** if op is not an array, then RETURN_FALSE". That should be *and*, not or ! If the second
parameter is not an existing filter ID constant *and* if it is not an
array, then it is not valid."

""IS_LONG && EXISTS" means: The filter ID must be an integer and the ID must exist. It accpepts array as the filter options/flags/id can be
given as an array, in this case the ID validation is done later."

The fact is, it /does not/ accept an array : || Z_TYPE_PP(op) != IS_ARRAY) { RETURN_FALSE; }. I.e., "if the second parameter to filter_var_array() is not an array, (forgetting whatever else has been tested just before), then return false". I say again that it should be --> && Z_TYPE_PP(op) != IS_ARRAY ... :)

Pierre, Re your message at 13:55 UTC, I think that is what has been missed.
 [2007-03-29 13:03 UTC] exaton at free dot fr
OK damn it, this typo is killing me as well. Responding to myself here :

It's not that lines 818-819 don't accept an array -- it's that they ONLY accept an array (because of the || instead of &&). But why test whether, if the second param IS_LONG, it is an existing ID, then ?
 [2007-03-29 13:46 UTC] pajoye@php.net
"It's not that lines 818-819 don't accept an array -- it's that they ONLY accept an array (because of the || instead of &&). But why test whether, if the second param IS_LONG, it is an existing ID, then ?"

Ok, if you consider that passing only an integer is valid, then there is a bug in this code (the expression is wrong). However it was never thought or implemented to accept only a filter ID (to be applied to all values). filter_var or filter_input has this feature.

I don't see a problem to accept this behavior (then we will have multiple ways to achieve the same goal). Here is the patch against 5.2:

http://blog.thepimp.net/misc/filter_52_array_allow_id.txt

I'll wait Ilia's opinion on it (we discussed this exact thing a while back), if he agrees, I'll apply the patch.

Changed to feature request.
 [2007-03-29 13:52 UTC] derick@php.net
Just commit this, it was supposed to work like this from the start.
 [2007-03-29 13:57 UTC] exaton at free dot fr
Thank you for taking the time to look at this, Pierre. My thanks to the others as well.

As it happens, I believe I will follow your recommendation anyway and use filter_var() for the purpose studied here, reserving filter_var_array() for argument => definition fine-grained validation. But I was intrigued by the "semi"-typo in the code.

So long all, keep up the excellent work !

David Holmes
 [2007-03-29 14:29 UTC] pajoye@php.net
"Just commit this, it was supposed to work like this from the start."

No, it was not. I wrote this function in the fist place.

I will commit it later tonight.
 [2007-04-04 20:51 UTC] pajoye@php.net
This bug has been fixed in CVS.

Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.
 
Thank you for the report, and for helping us make PHP better.


 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Mon Dec 30 14:01:28 2024 UTC