summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMáté Kocsis <kocsismate@woohoolabs.com>2020-08-17 22:37:20 +0200
committerMáté Kocsis <kocsismate@woohoolabs.com>2020-08-25 13:02:13 +0200
commit3df306de94dd94d6520c6f95e96c692c3aa8173a (patch)
treea5b3f45099f2dc6923a148edafb73ed30cefd244
parent4a98b644131597f6744789355d10aa55a66eee8e (diff)
downloadphp-git-3df306de94dd94d6520c6f95e96c692c3aa8173a.tar.gz
Promote warnings to exceptions in ext/pcntl
Closes GH-6004
-rw-r--r--ext/pcntl/pcntl.c39
-rw-r--r--ext/pcntl/pcntl.stub.php2
-rw-r--r--ext/pcntl/pcntl_arginfo.h2
-rw-r--r--ext/pcntl/tests/pcntl_getpriority_error.phpt23
-rw-r--r--ext/pcntl/tests/pcntl_setpriority_error.phpt23
-rw-r--r--ext/pcntl/tests/pcntl_signal.phpt32
-rw-r--r--ext/pcntl/tests/pcntl_unshare_01.phpt4
-rw-r--r--ext/pcntl/tests/pcntl_unshare_02.phpt2
-rw-r--r--ext/pcntl/tests/pcntl_unshare_04.phpt20
9 files changed, 113 insertions, 34 deletions
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--
+<?php
+if (!extension_loaded('pcntl')) {
+ die('skip ext/pcntl not loaded');
+}
+if (!function_exists('pcntl_getpriority')) {
+ die('skip pcntl_getpriority doesn\'t exist');
+}
+?>
+--FILE--
+<?php
+
+try {
+ pcntl_getpriority(null, 42);
+} catch (ValueError $exception) {
+ echo $exception->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--
+<?php
+if (!extension_loaded('pcntl')) {
+ die('skip ext/pcntl not loaded');
+}
+if (!function_exists('pcntl_setpriority')) {
+ die('skip pcntl_setpriority doesn\'t exist');
+}
+?>
+--FILE--
+<?php
+
+try {
+ pcntl_setpriority(0, null, 42);
+} catch (ValueError $exception) {
+ echo $exception->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--
<?php
diff --git a/ext/pcntl/tests/pcntl_unshare_02.phpt b/ext/pcntl/tests/pcntl_unshare_02.phpt
index 7cc9341154..fdf07bc81c 100644
--- a/ext/pcntl/tests/pcntl_unshare_02.phpt
+++ b/ext/pcntl/tests/pcntl_unshare_02.phpt
@@ -15,7 +15,7 @@ if (posix_getuid() !== 0 &&
if (@pcntl_unshare(CLONE_NEWPID) == false && pcntl_get_last_error() == PCNTL_EPERM) {
die("skip Insufficient privileges for CLONE_NEWPID");
}
-
+?>
--FILE--
<?php
diff --git a/ext/pcntl/tests/pcntl_unshare_04.phpt b/ext/pcntl/tests/pcntl_unshare_04.phpt
new file mode 100644
index 0000000000..fdd47d5d66
--- /dev/null
+++ b/ext/pcntl/tests/pcntl_unshare_04.phpt
@@ -0,0 +1,20 @@
+--TEST--
+pcntl_unshare() with wrong flag
+--SKIPIF--
+<?php
+if (!extension_loaded("pcntl")) die("skip");
+if (!extension_loaded("posix")) die("skip posix extension not available");
+if (!function_exists("pcntl_unshare")) die("skip pcntl_unshare is not available");
+?>
+--FILE--
+<?php
+
+try {
+ pcntl_unshare(42);
+} catch (ValueError $exception) {
+ echo $exception->getMessage() . "\n";
+}
+
+?>
+--EXPECT--
+pcntl_unshare(): Argument #1 ($flags) must be a combination of CLONE_* flags