From 3df306de94dd94d6520c6f95e96c692c3aa8173a Mon Sep 17 00:00:00 2001 From: =?utf8?q?M=C3=A1t=C3=A9=20Kocsis?= Date: Mon, 17 Aug 2020 22:37:20 +0200 Subject: [PATCH] Promote warnings to exceptions in ext/pcntl Closes GH-6004 --- ext/pcntl/pcntl.c | 39 +++++++++++--------- ext/pcntl/pcntl.stub.php | 2 +- ext/pcntl/pcntl_arginfo.h | 2 +- ext/pcntl/tests/pcntl_getpriority_error.phpt | 23 ++++++++++++ ext/pcntl/tests/pcntl_setpriority_error.phpt | 23 ++++++++++++ ext/pcntl/tests/pcntl_signal.phpt | 32 ++++++++++------ ext/pcntl/tests/pcntl_unshare_01.phpt | 4 +- ext/pcntl/tests/pcntl_unshare_02.phpt | 2 +- ext/pcntl/tests/pcntl_unshare_04.phpt | 20 ++++++++++ 9 files changed, 113 insertions(+), 34 deletions(-) create mode 100644 ext/pcntl/tests/pcntl_getpriority_error.phpt create mode 100644 ext/pcntl/tests/pcntl_setpriority_error.phpt create mode 100644 ext/pcntl/tests/pcntl_unshare_04.phpt diff --git a/ext/pcntl/pcntl.c b/ext/pcntl/pcntl.c index 624f2f0db3..0df11aa388 100644 --- a/ext/pcntl/pcntl.c +++ b/ext/pcntl/pcntl.c @@ -892,9 +892,14 @@ PHP_FUNCTION(pcntl_signal) RETURN_THROWS(); } - if (signo < 1 || signo >= NSIG) { - php_error_docref(NULL, E_WARNING, "Invalid signal"); - RETURN_FALSE; + if (signo < 1) { + zend_argument_value_error(1, "must be greater than or equal to 1"); + RETURN_THROWS(); + } + + if (signo >= NSIG) { + zend_argument_value_error(1, "must be less than %d", NSIG); + RETURN_THROWS(); } if (!PCNTL_G(spares)) { @@ -920,8 +925,8 @@ PHP_FUNCTION(pcntl_signal) /* Special long value case for SIG_DFL and SIG_IGN */ if (Z_TYPE_P(handle) == IS_LONG) { if (Z_LVAL_P(handle) != (zend_long) SIG_DFL && Z_LVAL_P(handle) != (zend_long) SIG_IGN) { - php_error_docref(NULL, E_WARNING, "Invalid value for handle argument specified"); - RETURN_FALSE; + zend_argument_value_error(2, "must be either SIG_DFL or SIG_IGN when an integer value is given"); + RETURN_THROWS(); } if (php_signal(signo, (Sigfunc *) Z_LVAL_P(handle), (int) restart_syscalls) == (void *)SIG_ERR) { PCNTL_G(last_error) = errno; @@ -935,10 +940,11 @@ PHP_FUNCTION(pcntl_signal) if (!zend_is_callable_ex(handle, NULL, 0, NULL, NULL, &error)) { zend_string *func_name = zend_get_callable_name(handle); PCNTL_G(last_error) = EINVAL; - php_error_docref(NULL, E_WARNING, "Specified handler \"%s\" is not callable (%s)", ZSTR_VAL(func_name), error); + + zend_argument_type_error(2, "must be of type callable|int, %s given", zend_zval_type_name(handle)); zend_string_release_ex(func_name, 0); efree(error); - RETURN_FALSE; + RETURN_THROWS(); } ZEND_ASSERT(!error); @@ -966,8 +972,8 @@ PHP_FUNCTION(pcntl_signal_get_handler) } if (signo < 1 || signo > 32) { - php_error_docref(NULL, E_WARNING, "Invalid signal"); - RETURN_FALSE; + zend_argument_value_error(1, "must be between 1 and 32"); + RETURN_THROWS(); } if ((prev_handle = zend_hash_index_find(&PCNTL_G(php_signal_table), signo)) != NULL) { @@ -1197,8 +1203,8 @@ PHP_FUNCTION(pcntl_getpriority) php_error_docref(NULL, E_WARNING, "Error %d: No process was located using the given parameters", errno); break; case EINVAL: - php_error_docref(NULL, E_WARNING, "Error %d: Invalid identifier flag", errno); - break; + zend_argument_value_error(2, "must be one of PRIO_PGRP, PRIO_USER, or PRIO_PROCESS"); + RETURN_THROWS(); default: php_error_docref(NULL, E_WARNING, "Unknown error %d has occurred", errno); break; @@ -1231,8 +1237,8 @@ PHP_FUNCTION(pcntl_setpriority) php_error_docref(NULL, E_WARNING, "Error %d: No process was located using the given parameters", errno); break; case EINVAL: - php_error_docref(NULL, E_WARNING, "Error %d: Invalid identifier flag", errno); - break; + zend_argument_value_error(3, "must be one of PRIO_PGRP, PRIO_USER, or PRIO_PROCESS"); + RETURN_THROWS(); case EPERM: php_error_docref(NULL, E_WARNING, "Error %d: A process was located, but neither its effective nor real user ID matched the effective user ID of the caller", errno); break; @@ -1400,19 +1406,18 @@ PHP_FUNCTION(pcntl_async_signals) PHP_FUNCTION(pcntl_unshare) { zend_long flags; - int ret; ZEND_PARSE_PARAMETERS_START(1, 1) Z_PARAM_LONG(flags) ZEND_PARSE_PARAMETERS_END(); - ret = unshare(flags); - if (ret == -1) { + if (unshare(flags) == -1) { PCNTL_G(last_error) = errno; switch (errno) { #ifdef EINVAL case EINVAL: - php_error_docref(NULL, E_WARNING, "Error %d: Invalid flag specified", errno); + zend_argument_value_error(1, "must be a combination of CLONE_* flags"); + RETURN_THROWS(); break; #endif #ifdef ENOMEM diff --git a/ext/pcntl/pcntl.stub.php b/ext/pcntl/pcntl.stub.php index 0c56b04966..2c3cd64aad 100644 --- a/ext/pcntl/pcntl.stub.php +++ b/ext/pcntl/pcntl.stub.php @@ -19,7 +19,7 @@ function pcntl_wait(&$status, int $options = 0, &$rusage = []): int {} /** @param callable|int $handler */ function pcntl_signal(int $signo, $handler, bool $restart_syscalls = true): bool {} -/** @return mixed */ +/** @return callable|int */ function pcntl_signal_get_handler(int $signo) {} function pcntl_signal_dispatch(): bool {} diff --git a/ext/pcntl/pcntl_arginfo.h b/ext/pcntl/pcntl_arginfo.h index 4c34a762c8..fb0fe5fa08 100644 --- a/ext/pcntl/pcntl_arginfo.h +++ b/ext/pcntl/pcntl_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: df744f88533ce9b84864fa2aa4dd7a5b7373231d */ + * Stub hash: 306208d94ba3bf6f8112f868a332e99717bc07fa */ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_pcntl_fork, 0, 0, IS_LONG, 0) ZEND_END_ARG_INFO() diff --git a/ext/pcntl/tests/pcntl_getpriority_error.phpt b/ext/pcntl/tests/pcntl_getpriority_error.phpt new file mode 100644 index 0000000000..5276d4df77 --- /dev/null +++ b/ext/pcntl/tests/pcntl_getpriority_error.phpt @@ -0,0 +1,23 @@ +--TEST-- +pcntl_getpriority() - Wrong process identifier +--SKIPIF-- + +--FILE-- +getMessage() . "\n"; +} + +?> +--EXPECT-- +pcntl_getpriority(): Argument #2 ($process_identifier) must be one of PRIO_PGRP, PRIO_USER, or PRIO_PROCESS diff --git a/ext/pcntl/tests/pcntl_setpriority_error.phpt b/ext/pcntl/tests/pcntl_setpriority_error.phpt new file mode 100644 index 0000000000..d354ab2bc5 --- /dev/null +++ b/ext/pcntl/tests/pcntl_setpriority_error.phpt @@ -0,0 +1,23 @@ +--TEST-- +pcntl_setpriority() - Wrong process identifier +--SKIPIF-- + +--FILE-- +getMessage() . "\n"; +} + +?> +--EXPECT-- +pcntl_setpriority(): Argument #3 ($process_identifier) must be one of PRIO_PGRP, PRIO_USER, or PRIO_PROCESS diff --git a/ext/pcntl/tests/pcntl_signal.phpt b/ext/pcntl/tests/pcntl_signal.phpt index cd55d9ad96..4b94064e2a 100644 --- a/ext/pcntl/tests/pcntl_signal.phpt +++ b/ext/pcntl/tests/pcntl_signal.phpt @@ -18,10 +18,24 @@ posix_kill(posix_getpid(), SIGUSR1); pcntl_signal_dispatch(); var_dump(pcntl_signal(SIGALRM, SIG_IGN)); -var_dump(pcntl_signal(-1, -1)); -var_dump(pcntl_signal(-1, function(){})); -var_dump(pcntl_signal(SIGALRM, "not callable")); +try { + pcntl_signal(-1, -1); +} catch (ValueError $exception) { + echo $exception->getMessage() . "\n"; +} + +try { + pcntl_signal(-1, function(){}); +} catch (ValueError $exception) { + echo $exception->getMessage() . "\n"; +} + +try { + pcntl_signal(SIGALRM, "not callable"); +} catch (TypeError $exception) { + echo $exception->getMessage() . "\n"; +} /* test freeing queue in RSHUTDOWN */ posix_kill(posix_getpid(), SIGTERM); @@ -31,13 +45,7 @@ echo "ok\n"; signal dispatched got signal from %r\d+|nobody%r bool(true) - -Warning: pcntl_signal(): Invalid signal %s -bool(false) - -Warning: pcntl_signal(): Invalid signal %s -bool(false) - -Warning: pcntl_signal(): Specified handler "not callable" is not callable (%s) in %s -bool(false) +pcntl_signal(): Argument #1 ($signo) must be greater than or equal to 1 +pcntl_signal(): Argument #1 ($signo) must be greater than or equal to 1 +pcntl_signal(): Argument #2 ($handler) must be of type callable|int, string given ok diff --git a/ext/pcntl/tests/pcntl_unshare_01.phpt b/ext/pcntl/tests/pcntl_unshare_01.phpt index 6debaace58..fcbf112e4d 100644 --- a/ext/pcntl/tests/pcntl_unshare_01.phpt +++ b/ext/pcntl/tests/pcntl_unshare_01.phpt @@ -7,9 +7,9 @@ if (!extension_loaded("posix")) die("skip posix extension not available"); if (!function_exists("pcntl_unshare")) die("skip pcntl_unshare is not available"); if (!defined("CLONE_NEWUSER")) die("skip flag unavailable"); if (@pcntl_unshare(CLONE_NEWUSER) == false && pcntl_get_last_error() == PCNTL_EPERM) { - die("skip Insufficient previleges to use CLONE_NEWUSER"); + die("skip Insufficient privileges to use CLONE_NEWUSER"); } - +?> --FILE-- --FILE-- +--FILE-- +getMessage() . "\n"; +} + +?> +--EXPECT-- +pcntl_unshare(): Argument #1 ($flags) must be a combination of CLONE_* flags -- 2.40.0