From e463fc476f8962175de316b2b4b9aa46d5204fae Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Mon, 16 May 2022 16:56:14 -0600 Subject: [PATCH 1/6] Stop closing stderr and stdout streams Extensions may (and do) write to stderr in mshutdown and similar. In the best case, with the stderr stream closed, it's just swallowed. However, some libraries will do things like try to detect color, and these will outright fail and cause an error path to be taken. --- NEWS | 1 + ext/zend_test/php_test.h | 1 + ext/zend_test/test.c | 5 +++++ ext/zend_test/tests/gh8575.phpt | 11 +++++++++++ sapi/cli/php_cli.c | 22 ++++++++++++---------- 5 files changed, 30 insertions(+), 10 deletions(-) create mode 100644 ext/zend_test/tests/gh8575.phpt diff --git a/NEWS b/NEWS index c20c9196400c..c11e4adaef34 100644 --- a/NEWS +++ b/NEWS @@ -4,6 +4,7 @@ PHP NEWS - CLI: . Fixed bug #81496 (Server logs incorrect request method). (lauri) + . Fixed GH-8575 (CLI closes standard streams too early). (Levi Morrison) - Core: . Fixed bug #81380 (Observer may not be initialized properly). (krakjoe) diff --git a/ext/zend_test/php_test.h b/ext/zend_test/php_test.h index e51854699caa..a08c080ee373 100644 --- a/ext/zend_test/php_test.h +++ b/ext/zend_test/php_test.h @@ -51,6 +51,7 @@ ZEND_BEGIN_MODULE_GLOBALS(zend_test) HashTable global_weakmap; int replace_zend_execute_ex; int register_passes; + bool print_stderr_mshutdown; zend_test_fiber *active_fiber; ZEND_END_MODULE_GLOBALS(zend_test) diff --git a/ext/zend_test/test.c b/ext/zend_test/test.c index 0a0f12e79fe2..0cc2dcae2822 100644 --- a/ext/zend_test/test.c +++ b/ext/zend_test/test.c @@ -495,6 +495,7 @@ static ZEND_METHOD(ZendTestChildClassWithMethodWithParameterAttribute, override) PHP_INI_BEGIN() STD_PHP_INI_BOOLEAN("zend_test.replace_zend_execute_ex", "0", PHP_INI_SYSTEM, OnUpdateBool, replace_zend_execute_ex, zend_zend_test_globals, zend_test_globals) STD_PHP_INI_BOOLEAN("zend_test.register_passes", "0", PHP_INI_SYSTEM, OnUpdateBool, register_passes, zend_zend_test_globals, zend_test_globals) + STD_PHP_INI_BOOLEAN("zend_test.print_stderr_mshutdown", "0", PHP_INI_SYSTEM, OnUpdateBool, print_stderr_mshutdown, zend_zend_test_globals, zend_test_globals) PHP_INI_END() void (*old_zend_execute_ex)(zend_execute_data *execute_data); @@ -620,6 +621,10 @@ PHP_MSHUTDOWN_FUNCTION(zend_test) zend_test_observer_shutdown(SHUTDOWN_FUNC_ARGS_PASSTHRU); + if (ZT_G(print_stderr_mshutdown)) { + fprintf(stderr, "[zend-test] MSHUTDOWN\n"); + } + return SUCCESS; } diff --git a/ext/zend_test/tests/gh8575.phpt b/ext/zend_test/tests/gh8575.phpt new file mode 100644 index 000000000000..4d947611a194 --- /dev/null +++ b/ext/zend_test/tests/gh8575.phpt @@ -0,0 +1,11 @@ +--TEST-- +CLI: stderr is available in mshutdown +--SKIPIF-- + +--INI-- +zend_test.print_stderr_mshutdown=1 +--FILE-- +==DONE== +--EXPECTF-- +==DONE== +[zend-test] MSHUTDOWN diff --git a/sapi/cli/php_cli.c b/sapi/cli/php_cli.c index 66a1b9ad86b0..0d30d63ec759 100644 --- a/sapi/cli/php_cli.c +++ b/sapi/cli/php_cli.c @@ -526,7 +526,7 @@ static void php_cli_usage(char *argv0) static php_stream *s_in_process = NULL; -static void cli_register_file_handles(bool no_close) /* {{{ */ +static void cli_register_file_handles(void) /* {{{ */ { php_stream *s_in, *s_out, *s_err; php_stream_context *sc_in=NULL, *sc_out=NULL, *sc_err=NULL; @@ -536,6 +536,14 @@ static void cli_register_file_handles(bool no_close) /* {{{ */ s_out = php_stream_open_wrapper_ex("php://stdout", "wb", 0, NULL, sc_out); s_err = php_stream_open_wrapper_ex("php://stderr", "wb", 0, NULL, sc_err); + /* Release stream resources, but don't free the underlying handles. Othewrise, + * extensions which write to stderr or company during mshutdown/gshutdown + * won't have the expected functionality. + */ + if (s_in) s_in->flags |= PHP_STREAM_FLAG_NO_CLOSE; + if (s_out) s_out->flags |= PHP_STREAM_FLAG_NO_CLOSE; + if (s_err) s_err->flags |= PHP_STREAM_FLAG_NO_CLOSE; + if (s_in==NULL || s_out==NULL || s_err==NULL) { if (s_in) php_stream_close(s_in); if (s_out) php_stream_close(s_out); @@ -543,12 +551,6 @@ static void cli_register_file_handles(bool no_close) /* {{{ */ return; } - if (no_close) { - s_in->flags |= PHP_STREAM_FLAG_NO_CLOSE; - s_out->flags |= PHP_STREAM_FLAG_NO_CLOSE; - s_err->flags |= PHP_STREAM_FLAG_NO_CLOSE; - } - s_in_process = s_in; php_stream_to_zval(s_in, &ic.value); @@ -954,7 +956,7 @@ static int do_cli(int argc, char **argv) /* {{{ */ switch (behavior) { case PHP_MODE_STANDARD: if (script_file) { - cli_register_file_handles(/* no_close */ PHP_DEBUG || num_repeats > 1); + cli_register_file_handles(); } if (interactive) { @@ -989,7 +991,7 @@ static int do_cli(int argc, char **argv) /* {{{ */ } break; case PHP_MODE_CLI_DIRECT: - cli_register_file_handles(/* no_close */ PHP_DEBUG || num_repeats > 1); + cli_register_file_handles(); zend_eval_string_ex(exec_direct, NULL, "Command line code", 1); break; @@ -1004,7 +1006,7 @@ static int do_cli(int argc, char **argv) /* {{{ */ file_handle.filename = NULL; } - cli_register_file_handles(/* no_close */ PHP_DEBUG || num_repeats > 1); + cli_register_file_handles(); if (exec_begin) { zend_eval_string_ex(exec_begin, NULL, "Command line begin code", 1); From 036df639bbaa18a4cf80451d121163fffedc47bd Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Wed, 18 May 2022 10:50:39 -0600 Subject: [PATCH 2/6] [ci skip] Adjust NEWS entry --- NEWS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS b/NEWS index c11e4adaef34..2c91b73b08aa 100644 --- a/NEWS +++ b/NEWS @@ -4,7 +4,7 @@ PHP NEWS - CLI: . Fixed bug #81496 (Server logs incorrect request method). (lauri) - . Fixed GH-8575 (CLI closes standard streams too early). (Levi Morrison) + . Fixed bug GH-8575 (CLI closes standard streams too early). (Levi Morrison) - Core: . Fixed bug #81380 (Observer may not be initialized properly). (krakjoe) From 24ef9276e0a04f03eba13d14696b8ebc53231fae Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Wed, 18 May 2022 11:04:54 -0600 Subject: [PATCH 3/6] Guard gh8575 for CLI only --- ext/zend_test/tests/gh8575.phpt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ext/zend_test/tests/gh8575.phpt b/ext/zend_test/tests/gh8575.phpt index 4d947611a194..e83b57c520e6 100644 --- a/ext/zend_test/tests/gh8575.phpt +++ b/ext/zend_test/tests/gh8575.phpt @@ -1,7 +1,9 @@ --TEST-- CLI: stderr is available in mshutdown --SKIPIF-- - + +--EXTENSIONS-- +zend-test --INI-- zend_test.print_stderr_mshutdown=1 --FILE-- From 78cd7165fd2d1b63ff182e97dbd8ea07abf11460 Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Wed, 18 May 2022 11:13:14 -0600 Subject: [PATCH 4/6] Extension name is zend_test, not zend-test --- ext/zend_test/test.c | 2 +- ext/zend_test/tests/gh8575.phpt | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ext/zend_test/test.c b/ext/zend_test/test.c index 0cc2dcae2822..c0d46784a256 100644 --- a/ext/zend_test/test.c +++ b/ext/zend_test/test.c @@ -622,7 +622,7 @@ PHP_MSHUTDOWN_FUNCTION(zend_test) zend_test_observer_shutdown(SHUTDOWN_FUNC_ARGS_PASSTHRU); if (ZT_G(print_stderr_mshutdown)) { - fprintf(stderr, "[zend-test] MSHUTDOWN\n"); + fprintf(stderr, "[zend_test] MSHUTDOWN\n"); } return SUCCESS; diff --git a/ext/zend_test/tests/gh8575.phpt b/ext/zend_test/tests/gh8575.phpt index e83b57c520e6..23acd2467778 100644 --- a/ext/zend_test/tests/gh8575.phpt +++ b/ext/zend_test/tests/gh8575.phpt @@ -3,11 +3,11 @@ CLI: stderr is available in mshutdown --SKIPIF-- --EXTENSIONS-- -zend-test +zend_test --INI-- zend_test.print_stderr_mshutdown=1 --FILE-- ==DONE== --EXPECTF-- ==DONE== -[zend-test] MSHUTDOWN +[zend_test] MSHUTDOWN From 901c0336eda47132b626f1f9fb36e85388a76e0b Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Wed, 18 May 2022 11:21:34 -0600 Subject: [PATCH 5/6] Use 'skip' not 'skip:' --- ext/zend_test/tests/gh8575.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/zend_test/tests/gh8575.phpt b/ext/zend_test/tests/gh8575.phpt index 23acd2467778..82f26cef3852 100644 --- a/ext/zend_test/tests/gh8575.phpt +++ b/ext/zend_test/tests/gh8575.phpt @@ -1,7 +1,7 @@ --TEST-- CLI: stderr is available in mshutdown --SKIPIF-- - + --EXTENSIONS-- zend_test --INI-- From 11abbef3f1297f9a0eab78743d90b41c7615d095 Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Wed, 18 May 2022 11:39:54 -0600 Subject: [PATCH 6/6] Remove one too many parens --- ext/zend_test/tests/gh8575.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/zend_test/tests/gh8575.phpt b/ext/zend_test/tests/gh8575.phpt index 82f26cef3852..cacf8249f310 100644 --- a/ext/zend_test/tests/gh8575.phpt +++ b/ext/zend_test/tests/gh8575.phpt @@ -1,7 +1,7 @@ --TEST-- CLI: stderr is available in mshutdown --SKIPIF-- - + --EXTENSIONS-- zend_test --INI--