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 c20c9196400c0..c11e4adaef34a 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 e51854699caa2..a08c080ee3730 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 0a0f12e79fe2d..0cc2dcae28220 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 0000000000000..4d947611a194b --- /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 66a1b9ad86b09..0d30d63ec7597 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 c11e4adaef34a..2c91b73b08aaa 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 4d947611a194b..e83b57c520e68 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 0cc2dcae28220..c0d46784a2563 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 e83b57c520e68..23acd24677784 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 23acd24677784..82f26cef3852d 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 82f26cef3852d..cacf8249f3104 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--