Skip to content

fix(site): keep site commands working when pcntl or shell_exec is disabled - #523

Open
mrrobot47 wants to merge 1 commit into
EasyEngine:developfrom
mrrobot47:fix/guard-optional-php-functions
Open

mrrobot47 wants to merge 1 commit into
EasyEngine:developfrom
mrrobot47:fix/guard-optional-php-functions

Conversation

@mrrobot47

Copy link
Copy Markdown
Member

Part of EasyEngine/easyengine#1956, companion to EasyEngine/easyengine#1957.

Problem

On PHP 8 a function listed in disable_functions is removed, and calling it is a fatal Call to undefined function. The EE_Site_Command constructor called pcntl_signal() unconditionally, so on a host whose CLI php.ini disables pcntl_* (HestiaCP does) every ee site … command died:

PHP Fatal error:  Uncaught Error: Call to undefined function EE\Site\Type\pcntl_signal() in .../site-command/src/helper/class-ee-site.php:87

A PHP built without pcntl failed the same way, and pcntl_signal_dispatch() in the SSL lock wait had the same problem.

Change

  • pcntl_signal() and pcntl_signal_dispatch() are called only when they exist. Without them a site command still runs; it only loses the rollback on SIGINT/SIGTERM/SIGHUP/SIGUSR1, which is logged as a debug line. Ctrl+C still ends the SSL lock wait, because the default signal action applies when no handler is installed. With pcntl available nothing changes (same handlers, same order). function_exists( 'pcntl_signal' ) also keeps the SIG* constants from being evaluated when the extension isn't loaded.
  • ee site share refuses up front with a clear error when shell_exec() is disabled, instead of a fatal after the existing share was already torn down. A plain --disable (curl and killall only) still works.

Why pcntl is optional

The signal handlers only add a rollback when a command is interrupted, which is a nice-to-have compared with not being able to run any site command. So the core check (EasyEngine/easyengine#1957) treats pcntl_signal, pcntl_signal_dispatch and shell_exec as optional and only refuses on what ee can't work without. The other unguarded calls the audit found here (posix_* in site types, php_uname, getmypid, disk_free_space, curl_*, openssl_*, chown, symlink, gethostname) are needed by site create, SSL, backup and clone, so they stay required and core refuses up front with an error that names them.

Merge order

Merge and tag this first, then core: the core PR relies on this guard for HestiaCP hosts.

Tests

  • Throwaway containers (ubuntu:24.04, ondrej PHP 8.3 and 7.4, HestiaCP's own CLI disable_functions line, Docker stubbed): before, ee site list / ee site create are a fatal on PHP 8.3; after, they run (No sites found!, site creation proceeds).
  • Unit tests (the local PHPUnit layer, not in this repo yet; PHP 7.4 and 8.5, with pcntl absent, disabled and available): a site command is constructed and the SSL lock wait ends cleanly without pcntl, and the rollback handlers are still registered when pcntl is available. They fail on develop with Call to undefined function pcntl_signal().
  • Behat (fork CI, green): the site-command suite ran in core's CI with this branch, on PHP 7.4–8.5 and with HestiaCP's disable_functions on 7.4, 8.3 and 8.5.

…abled

On PHP 8 a function listed in disable_functions is removed, so the unconditional pcntl_signal() calls in the EE_Site_Command constructor were a fatal "Call to undefined function" for every ee site command on hosts whose CLI php.ini disables pcntl_* (HestiaCP does this; EasyEngine/easyengine#1956). A PHP built without pcntl failed the same way.

pcntl_signal() and pcntl_signal_dispatch() are now called only when they exist. Without them a site command still runs; it only loses the rollback on SIGINT/SIGTERM/SIGHUP/SIGUSR1 (logged as a debug line), and Ctrl+C still ends the SSL lock wait because the default signal action applies. With pcntl available nothing changes.

ee site share now refuses up front with a clear error when shell_exec() is disabled, instead of a fatal after the old share was already torn down; a plain --disable still works.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant