Skip to content

Promote warnings to exceptions in ext/pcntl #6004

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 22 additions & 16 deletions ext/pcntl/pcntl.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand All @@ -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;
Expand All @@ -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);

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -1231,7 +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);
zend_argument_value_error(3, "must be one of PRIO_PGRP, PRIO_USER, or PRIO_PROCESS");
RETURN_THROWS();
break;
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);
Expand Down Expand Up @@ -1400,19 +1407,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
Expand Down
2 changes: 1 addition & 1 deletion ext/pcntl/pcntl.stub.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}
Expand Down
2 changes: 1 addition & 1 deletion ext/pcntl/pcntl_arginfo.h
Original file line number Diff line number Diff line change
@@ -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()
Expand Down
23 changes: 23 additions & 0 deletions ext/pcntl/tests/pcntl_getpriority_error.phpt
Original file line number Diff line number Diff line change
@@ -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
23 changes: 23 additions & 0 deletions ext/pcntl/tests/pcntl_setpriority_error.phpt
Original file line number Diff line number Diff line change
@@ -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
32 changes: 20 additions & 12 deletions ext/pcntl/tests/pcntl_signal.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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
4 changes: 2 additions & 2 deletions ext/pcntl/tests/pcntl_unshare_01.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion ext/pcntl/tests/pcntl_unshare_02.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
20 changes: 20 additions & 0 deletions ext/pcntl/tests/pcntl_unshare_04.phpt
Original file line number Diff line number Diff line change
@@ -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