Promote warnings to exceptions in ext/filter

kocsismate · kocsismate · commit cc35cfd2eb95 · 2020-08-24T17:34:32.000+02:00
Closes GH-5970
diff --git a/ext/filter/callback_filter.c b/ext/filter/callback_filter.c
@@ -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;
diff --git a/ext/filter/filter.c b/ext/filter/filter.c
@@ -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) {
@@ -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;
@@ -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) {
@@ -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;
@@ -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;
diff --git a/ext/filter/filter_private.h b/ext/filter/filter_private.h
@@ -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;	\
 
diff --git a/ext/filter/logical_filters.c b/ext/filter/logical_filters.c
@@ -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;
@@ -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;
@@ -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
 	}
 
@@ -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;
 	}
 
diff --git a/ext/filter/tests/017.phpt b/ext/filter/tests/017.phpt
@@ -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
diff --git a/ext/filter/tests/029.phpt b/ext/filter/tests/029.phpt
@@ -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 {
@@ -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) ""
diff --git a/ext/filter/tests/031.phpt b/ext/filter/tests/031.phpt
@@ -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)
@@ -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)
diff --git a/ext/filter/tests/039.phpt b/ext/filter/tests/039.phpt
@@ -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));
@@ -48,7 +55,7 @@ var_dump($a, $b);
 
 echo "Done\n";
 ?>
---EXPECTF--
+--EXPECT--
 -- (1)
 array(0) {
 }
@@ -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) {
   [""]=>
diff --git a/ext/filter/tests/040.phpt b/ext/filter/tests/040.phpt
@@ -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
diff --git a/ext/filter/tests/055.phpt b/ext/filter/tests/055.phpt
@@ -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)
@@ -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
diff --git a/ext/filter/tests/bug51368.phpt b/ext/filter/tests/bug51368.phpt
@@ -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
