Skip to content

Commit

Permalink
Promote warnings to exceptions in ext/filter
Browse files Browse the repository at this point in the history
Closes GH-5970
  • Loading branch information
kocsismate committed Aug 24, 2020
1 parent c557c41 commit cc35cfd
Show file tree
Hide file tree
Showing 11 changed files with 102 additions and 70 deletions.
2 changes: 1 addition & 1 deletion ext/filter/callback_filter.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ void php_filter_callback(PHP_INPUT_FILTER_PARAM_DECL)
int status;

if (!option_array || !zend_is_callable(option_array, 0, NULL)) {
php_error_docref(NULL, E_WARNING, "First argument is expected to be a valid callback");
zend_type_error("%s(): Option must be a valid callback", get_active_function_name());
zval_ptr_dtor(value);
ZVAL_NULL(value);
return;
Expand Down
23 changes: 15 additions & 8 deletions ext/filter/filter.c
Original file line number Diff line number Diff line change
Expand Up @@ -487,8 +487,8 @@ static zval *php_filter_get_storage(zend_long arg)/* {{{ */
array_ptr = !Z_ISUNDEF(IF_G(env_array)) ? &IF_G(env_array) : &PG(http_globals)[TRACK_VARS_ENV];
break;
default:
php_error_docref(NULL, E_WARNING, "Unknown source");
break;
zend_argument_value_error(1, "must be an INPUT_* constant");
return NULL;
}

if (array_ptr && Z_TYPE_P(array_ptr) != IS_ARRAY) {
Expand All @@ -512,6 +512,9 @@ PHP_FUNCTION(filter_has_var)
}

array_ptr = php_filter_get_storage(arg);
if (EG(exception)) {
RETURN_THROWS();
}

if (array_ptr && zend_hash_exists(Z_ARRVAL_P(array_ptr), var)) {
RETURN_TRUE;
Expand Down Expand Up @@ -614,14 +617,12 @@ static void php_filter_array_handler(zval *input, zval *op, zval *return_value,

ZEND_HASH_FOREACH_STR_KEY_VAL(Z_ARRVAL_P(op), arg_key, arg_elm) {
if (arg_key == NULL) {
php_error_docref(NULL, E_WARNING, "Numeric keys are not allowed in the definition array");
zval_ptr_dtor(return_value);
RETURN_FALSE;
zend_argument_type_error(2, "must contain only string keys");
RETURN_THROWS();
}
if (ZSTR_LEN(arg_key) == 0) {
php_error_docref(NULL, E_WARNING, "Empty keys are not allowed in the definition array");
zval_ptr_dtor(return_value);
RETURN_FALSE;
zend_argument_value_error(2, "cannot contain empty keys");
RETURN_THROWS();
}
if ((tmp = zend_hash_find(Z_ARRVAL_P(input), arg_key)) == NULL) {
if (add_empty) {
Expand Down Expand Up @@ -658,6 +659,9 @@ PHP_FUNCTION(filter_input)
}

input = php_filter_get_storage(fetch_from);
if (EG(exception)) {
RETURN_THROWS();
}

if (!input || (tmp = zend_hash_find(Z_ARRVAL_P(input), var)) == NULL) {
zend_long filter_flags = 0;
Expand Down Expand Up @@ -731,6 +735,9 @@ PHP_FUNCTION(filter_input_array)
}

array_input = php_filter_get_storage(fetch_from);
if (EG(exception)) {
RETURN_THROWS();
}

if (!array_input) {
zend_long filter_flags = 0;
Expand Down
15 changes: 9 additions & 6 deletions ext/filter/filter_private.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,15 @@
|| (id >= FILTER_VALIDATE_ALL && id <= FILTER_VALIDATE_LAST) \
|| id == FILTER_CALLBACK)

#define RETURN_VALIDATION_FAILED \
zval_ptr_dtor(value); \
if (flags & FILTER_NULL_ON_FAILURE) { \
ZVAL_NULL(value); \
} else { \
ZVAL_FALSE(value); \
#define RETURN_VALIDATION_FAILED \
if (EG(exception)) { \
return; \
} else if (flags & FILTER_NULL_ON_FAILURE) { \
zval_ptr_dtor(value); \
ZVAL_NULL(value); \
} else { \
zval_ptr_dtor(value); \
ZVAL_FALSE(value); \
} \
return; \

Expand Down
8 changes: 4 additions & 4 deletions ext/filter/logical_filters.c
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ void php_filter_float(PHP_INPUT_FILTER_PARAM_DECL) /* {{{ */

if (decimal_set) {
if (decimal_len != 1) {
php_error_docref(NULL, E_WARNING, "Decimal separator must be one char");
zend_value_error("%s(): \"decimal\" option must be one character long", get_active_function_name());
RETURN_VALIDATION_FAILED
} else {
dec_sep = *decimal;
Expand All @@ -371,7 +371,7 @@ void php_filter_float(PHP_INPUT_FILTER_PARAM_DECL) /* {{{ */

if (thousand_set) {
if (thousand_len < 1) {
php_error_docref(NULL, E_WARNING, "Thousand separator must be at least one char");
zend_value_error("%s(): \"thousand\" option cannot be empty", get_active_function_name());
RETURN_VALIDATION_FAILED
} else {
tsd_sep = thousand;
Expand Down Expand Up @@ -472,7 +472,7 @@ void php_filter_validate_regexp(PHP_INPUT_FILTER_PARAM_DECL) /* {{{ */
FETCH_STR_OPTION(regexp, "regexp");

if (!regexp_set) {
php_error_docref(NULL, E_WARNING, "'regexp' option missing");
zend_value_error("%s(): \"regexp\" option is missing", get_active_function_name());
RETURN_VALIDATION_FAILED
}

Expand Down Expand Up @@ -919,7 +919,7 @@ void php_filter_validate_mac(PHP_INPUT_FILTER_PARAM_DECL) /* {{{ */
FETCH_STRING_OPTION(exp_separator, "separator");

if (exp_separator_set && exp_separator_len != 1) {
php_error_docref(NULL, E_WARNING, "Separator must be exactly one character long");
zend_value_error("%s(): \"separator\" option must be one character long", get_active_function_name());
RETURN_VALIDATION_FAILED;
}

Expand Down
12 changes: 7 additions & 5 deletions ext/filter/tests/017.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,19 @@ var_dump(filter_var("data", FILTER_VALIDATE_REGEXP, array("options"=>array("rege
var_dump(filter_var("data", FILTER_VALIDATE_REGEXP, array("options"=>array("regexp"=>'/^d(.*)/'))));
var_dump(filter_var("data", FILTER_VALIDATE_REGEXP, array("options"=>array("regexp"=>'/blah/'))));
var_dump(filter_var("data", FILTER_VALIDATE_REGEXP, array("options"=>array("regexp"=>'/\[/'))));
var_dump(filter_var("data", FILTER_VALIDATE_REGEXP));
try {
filter_var("data", FILTER_VALIDATE_REGEXP);
} catch (ValueError $exception) {
echo $exception->getMessage() . "\n";
}

echo "Done\n";
?>
--EXPECTF--
--EXPECT--
string(4) "data"
bool(false)
string(4) "data"
bool(false)
bool(false)

Warning: filter_var(): 'regexp' option missing in %s on line %d
bool(false)
filter_var(): "regexp" option is missing
Done
33 changes: 21 additions & 12 deletions ext/filter/tests/029.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,24 @@ function test($var) {
var_dump(filter_var("data", FILTER_CALLBACK, array("options"=>"test")));
var_dump(filter_var("~!@#$%^&*()_QWERTYUIOPASDFGHJKLZXCVBNM<>>?\"}{:", FILTER_CALLBACK, array("options"=>"test")));
var_dump(filter_var("", FILTER_CALLBACK, array("options"=>"test")));
var_dump(filter_var("qwe", FILTER_CALLBACK, array("options"=>"no such func")));
var_dump(filter_var("qwe", FILTER_CALLBACK, array("options"=>"")));
var_dump(filter_var("qwe", FILTER_CALLBACK));

try {
filter_var("qwe", FILTER_CALLBACK, array("options"=>"no such func"));
} catch (TypeError $exception) {
echo $exception->getMessage() . "\n";
}

try {
filter_var("qwe", FILTER_CALLBACK, array("options"=>""));
} catch (TypeError $exception) {
echo $exception->getMessage() . "\n";
}

try {
filter_var("qwe", FILTER_CALLBACK);
} catch (TypeError $exception) {
echo $exception->getMessage() . "\n";
}

/* Simple class method callback */
class test_class {
Expand Down Expand Up @@ -62,15 +77,9 @@ echo "Done\n";
string(4) "DATA"
string(46) "~!@#$%^&*()_QWERTYUIOPASDFGHJKLZXCVBNM<>>?"}{:"
string(0) ""

Warning: filter_var(): First argument is expected to be a valid callback in %s on line %d
NULL

Warning: filter_var(): First argument is expected to be a valid callback in %s on line %d
NULL

Warning: filter_var(): First argument is expected to be a valid callback in %s on line %d
NULL
filter_var(): Option must be a valid callback
filter_var(): Option must be a valid callback
filter_var(): Option must be a valid callback
string(4) "data"
string(46) "~!@#$%^&*()_qwertyuiopasdfghjklzxcvbnm<>>?"}{:"
string(0) ""
Expand Down
13 changes: 7 additions & 6 deletions ext/filter/tests/031.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,15 @@ $floats = array(

echo "\ncustom decimal:\n";
foreach ($floats as $float => $dec) {
$out = filter_var($float, FILTER_VALIDATE_FLOAT, array("options"=>array('decimal' => $dec)));
var_dump($out);
try {
var_dump(filter_var($float, FILTER_VALIDATE_FLOAT, array("options"=>array('decimal' => $dec))));
} catch (ValueError $exception) {
echo $exception->getMessage() . "\n";
}
}

?>
--EXPECTF--
--EXPECT--
float(1.234)
float(1.234)
float(1.234)
Expand All @@ -52,7 +55,5 @@ custom decimal:
bool(false)
float(1.234)
float(1.234)

Warning: filter_var(): Decimal separator must be one char in %s on line %d
bool(false)
filter_var(): "decimal" option must be one character long
bool(false)
23 changes: 13 additions & 10 deletions ext/filter/tests/039.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,16 @@ var_dump(filter_var_array(array(), array("var_name"=>-1)));
var_dump(filter_var_array(array("var_name"=>""), array("var_name"=>-1)));

echo "-- (5)\n";
var_dump(filter_var_array(array("var_name"=>""), array("var_name"=>-1, "asdas"=>"asdasd", "qwe"=>"rty", ""=>"")));
var_dump(filter_var_array(array("asdas"=>"text"), array("var_name"=>-1, "asdas"=>"asdasd", "qwe"=>"rty", ""=>"")));

try {
filter_var_array(array("var_name"=>""), array("var_name"=>-1, "asdas"=>"asdasd", "qwe"=>"rty", ""=>""));
} catch (ValueError $exception) {
echo $exception->getMessage() . "\n";
}
try {
filter_var_array(array("asdas"=>"text"), array("var_name"=>-1, "asdas"=>"asdasd", "qwe"=>"rty", ""=>""));
} catch (ValueError $exception) {
echo $exception->getMessage() . "\n";
}

$a = array(""=>""); $b = -1;
var_dump(filter_var_array($a, $b));
Expand All @@ -48,7 +55,7 @@ var_dump($a, $b);

echo "Done\n";
?>
--EXPECTF--
--EXPECT--
-- (1)
array(0) {
}
Expand Down Expand Up @@ -86,12 +93,8 @@ array(1) {
string(0) ""
}
-- (5)

Warning: filter_var_array(): Empty keys are not allowed in the definition array in %s on line %d
bool(false)

Warning: filter_var_array(): Empty keys are not allowed in the definition array in %s on line %d
bool(false)
filter_var_array(): Argument #2 ($options) cannot contain empty keys
filter_var_array(): Argument #2 ($options) cannot contain empty keys
bool(false)
array(1) {
[""]=>
Expand Down
13 changes: 8 additions & 5 deletions ext/filter/tests/040.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,24 @@ var_dump(filter_has_var(INPUT_GET, "a"));
var_dump(filter_has_var(INPUT_GET, "c"));
var_dump(filter_has_var(INPUT_GET, "abc"));
var_dump(filter_has_var(INPUT_GET, "cc"));
var_dump(filter_has_var(-1, "cc"));
try {
filter_has_var(-1, "cc");
} catch (ValueError $exception) {
echo $exception->getMessage() . "\n";
}

var_dump(filter_has_var(0, "cc"));

echo "Done\n";
?>
--EXPECTF--
--EXPECT--
bool(false)
bool(true)
bool(true)
bool(true)
bool(true)
bool(false)
bool(false)

Warning: filter_has_var(): Unknown source in %s on line %d
bool(false)
filter_has_var(): Argument #1 ($type) must be an INPUT_* constant
bool(false)
Done
16 changes: 8 additions & 8 deletions ext/filter/tests/055.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,16 @@ $values = Array(
array("01-23-45-67-89-ab", array("options" => array("separator" => ""))),
);
foreach ($values as $value) {
var_dump(filter_var($value[0], FILTER_VALIDATE_MAC, $value[1]));
try {
var_dump(filter_var($value[0], FILTER_VALIDATE_MAC, $value[1]));
} catch (ValueError $exception) {
echo $exception->getMessage() . "\n";
}
}

echo "Done\n";
?>
--EXPECTF--
--EXPECT--
string(17) "01-23-45-67-89-ab"
string(17) "01-23-45-67-89-ab"
bool(false)
Expand All @@ -39,10 +43,6 @@ string(17) "01:23:45:67:89:aB"
bool(false)
bool(false)
string(14) "0123.4567.89ab"

Warning: filter_var(): Separator must be exactly one character long in %s055.php on line %d
bool(false)

Warning: filter_var(): Separator must be exactly one character long in %s055.php on line %d
bool(false)
filter_var(): "separator" option must be one character long
filter_var(): "separator" option must be one character long
Done
14 changes: 9 additions & 5 deletions ext/filter/tests/bug51368.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,15 @@ var_dump(
filter_var('1 234.567', FILTER_VALIDATE_FLOAT, $options)
);
$options = ['flags' => FILTER_FLAG_ALLOW_THOUSAND, 'options' => ['thousand' => '']];
var_dump(filter_var('12345', FILTER_VALIDATE_FLOAT, $options));

try {
filter_var('12345', FILTER_VALIDATE_FLOAT, $options);
} catch (ValueError $exception) {
echo $exception->getMessage() . "\n";
}

?>
--EXPECTF--
--EXPECT--
float(1000)
float(1234.567)

Warning: filter_var(): Thousand separator must be at least one char in %s on line %d
bool(false)
filter_var(): "thousand" option cannot be empty

0 comments on commit cc35cfd

Please sign in to comment.