diff --git a/cli.go b/cli.go index 96821a2392..f160f80c45 100644 --- a/cli.go +++ b/cli.go @@ -2,11 +2,47 @@ package frankenphp // #include "frankenphp.h" import "C" -import "unsafe" + +import ( + "log/slog" + "unsafe" +) + +// cliUsageErrorExitCode is the exit status returned by ExecuteScriptCLI and +// ExecutePHPCode when the call cannot be executed, for instance when FrankenPHP +// is already running in the current process. +const cliUsageErrorExitCode = 1 + +// refuseCLIWhileRunning reports whether a CLI execution must be refused because +// FrankenPHP is already running in this process. ExecuteScriptCLI/ExecutePHPCode +// start the embedded PHP CLI SAPI, and doing so on top of the already-started +// FrankenPHP SAPI corrupts the PHP engine and crashes the whole process with a +// segmentation fault. Run the PHP CLI in a dedicated process instead. +// See https://github.com/php/frankenphp/issues/2342. +func refuseCLIWhileRunning() bool { + if !isRunning { + return false + } + + globalLogger.LogAttrs(globalCtx, slog.LevelError, "the PHP CLI cannot be executed while FrankenPHP is running; run ExecuteScriptCLI/ExecutePHPCode in a dedicated process instead") + + return true +} // ExecuteScriptCLI executes the PHP script passed as parameter. // It returns the exit status code of the script. +// +// It must not be called while FrankenPHP is running in the same process (that is, +// after a successful [Init] and before [Shutdown]): the embedded PHP CLI SAPI +// cannot coexist with the running FrankenPHP SAPI. Run it before [Init], after +// [Shutdown], or in a separate process (for example the same binary in "php-cli" +// mode) instead. While FrankenPHP is running the call is refused: an error is +// logged and a non-zero exit status is returned. func ExecuteScriptCLI(script string, args []string) int { + if refuseCLIWhileRunning() { + return cliUsageErrorExitCode + } + // Ensure extensions are registered before CLI execution registerExtensions() @@ -19,7 +55,17 @@ func ExecuteScriptCLI(script string, args []string) int { return int(C.frankenphp_execute_script_cli(cScript, argc, (**C.char)(unsafe.Pointer(&argv[0])), false)) } +// ExecutePHPCode evaluates the PHP code passed as parameter. +// It returns the exit status code of the code. +// +// Like [ExecuteScriptCLI], it must not be called while FrankenPHP is running in +// the same process; in that case the call is refused, an error is logged, and a +// non-zero exit status is returned. func ExecutePHPCode(phpCode string) int { + if refuseCLIWhileRunning() { + return cliUsageErrorExitCode + } + // Ensure extensions are registered before CLI execution registerExtensions() diff --git a/cli_internal_test.go b/cli_internal_test.go new file mode 100644 index 0000000000..44f40fb999 --- /dev/null +++ b/cli_internal_test.go @@ -0,0 +1,26 @@ +package frankenphp + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Unit coverage for the guard added for +// https://github.com/php/frankenphp/issues/2342: the embedded PHP CLI SAPI must +// not be started while FrankenPHP is already running, otherwise the process +// crashes with a segmentation fault. +func TestRefuseCLIWhileRunning(t *testing.T) { + // When FrankenPHP is not running, the CLI helpers proceed normally. + assert.False(t, refuseCLIWhileRunning()) + + require.NoError(t, Init()) + defer Shutdown() + + // While running, the guard trips and the public entrypoints refuse cleanly + // with the usage exit code instead of crashing the process. + assert.True(t, refuseCLIWhileRunning()) + assert.Equal(t, cliUsageErrorExitCode, ExecuteScriptCLI("testdata/command.php", []string{"testdata/command.php"})) + assert.Equal(t, cliUsageErrorExitCode, ExecutePHPCode("echo 'noop';")) +} diff --git a/cli_test.go b/cli_test.go index 964bb49907..5e1187a486 100644 --- a/cli_test.go +++ b/cli_test.go @@ -10,6 +10,7 @@ import ( "github.com/dunglas/frankenphp" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestExecuteScriptCLI(t *testing.T) { @@ -67,6 +68,29 @@ func TestExecuteScriptCLISignals(t *testing.T) { assert.Contains(t, string(stdoutStderr), "ok") } +// Regression test for https://github.com/php/frankenphp/issues/2342. Calling +// ExecuteScriptCLI (or ExecutePHPCode) while FrankenPHP is already running in the +// same process used to boot the embedded PHP CLI SAPI on top of the running +// FrankenPHP SAPI, corrupting the engine and crashing the whole process with a +// segmentation fault. It must now be refused cleanly with a non-zero exit status. +func TestExecuteScriptCLIWhileRunning(t *testing.T) { + if _, err := os.Stat("internal/testcli/testcli"); err != nil { + t.Skip("internal/testcli/testcli has not been compiled, run `cd internal/testcli/ && go build`") + } + + cmd := exec.Command("internal/testcli/testcli", "-init-first", "testdata/command.php") + stdoutStderr, err := cmd.CombinedOutput() + + var exitError *exec.ExitError + require.ErrorAs(t, err, &exitError, "output: %s", stdoutStderr) + + // The process must exit cleanly, not be killed by a signal (before the fix + // it crashed with SIGSEGV, reported by ProcessState.Exited() == false). + assert.True(t, exitError.Exited(), + "process was killed by a signal instead of exiting cleanly: %s\noutput: %s", exitError, stdoutStderr) + assert.Equal(t, 1, exitError.ExitCode(), "output: %s", stdoutStderr) +} + func ExampleExecuteScriptCLI() { if len(os.Args) <= 1 { log.Println("Usage: my-program script.php") diff --git a/internal/testcli/main.go b/internal/testcli/main.go index c03c836c4d..0a436eb7f4 100644 --- a/internal/testcli/main.go +++ b/internal/testcli/main.go @@ -17,5 +17,18 @@ func main() { os.Exit(frankenphp.ExecutePHPCode(os.Args[2])) } + // "-init-first script.php" starts FrankenPHP before running the CLI script, + // to exercise the guard against executing the PHP CLI while FrankenPHP is + // already running (https://github.com/php/frankenphp/issues/2342). + if len(os.Args) == 3 && os.Args[1] == "-init-first" { + if err := frankenphp.Init(); err != nil { + log.Fatalln(err) + } + + status := frankenphp.ExecuteScriptCLI(os.Args[2], os.Args[2:]) + frankenphp.Shutdown() + os.Exit(status) + } + os.Exit(frankenphp.ExecuteScriptCLI(os.Args[1], os.Args)) }