From 12d191853ec2cd46ed87badb08e054dda32c051b Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Mon, 16 May 2022 16:54:47 -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 | 3 +++ 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, 33 insertions(+), 9 deletions(-) create mode 100644 ext/zend_test/tests/gh8575.phpt diff --git a/NEWS b/NEWS index ea8ee5879e72..33b29ac625a5 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,9 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? ????, PHP 8.1.4 +- CLI: + . Fixed GH-8575 (CLI closes standard streams too early). (Levi Morrison) + - Core: . Fixed Haiku ZTS build. (David Carlier) . Fixed bug GH-8059 arginfo not regenerated for extension. (Remi) 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 0dc6ddd5128c..6ef7b86ac830 100644 --- a/ext/zend_test/test.c +++ b/ext/zend_test/test.c @@ -393,6 +393,7 @@ static ZEND_METHOD(ZendTestNS2_ZendSubNS_Foo, method) 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); @@ -463,6 +464,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 0ad53e813c94..c2de8bd5230e 100644 --- a/sapi/cli/php_cli.c +++ b/sapi/cli/php_cli.c @@ -538,6 +538,16 @@ 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 (no_close) { + 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); @@ -545,12 +555,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); @@ -956,7 +960,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(/* no_close */ true); } if (interactive) { @@ -991,7 +995,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(/* no_close */ true); zend_eval_string_ex(exec_direct, NULL, "Command line code", 1); break; @@ -1006,7 +1010,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(/* no_close */ true); if (exec_begin) { zend_eval_string_ex(exec_begin, NULL, "Command line begin code", 1); From d1aededf82eb39890485c30f3bc89aed788b9f62 Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Wed, 18 May 2022 10:50:09 -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 33b29ac625a5..f4fbf4bd755f 100644 --- a/NEWS +++ b/NEWS @@ -3,7 +3,7 @@ PHP NEWS ?? ??? ????, PHP 8.1.4 - CLI: - . 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 Haiku ZTS build. (David Carlier) From 80d13041ae89c25fab9a89ff9da031c36508855d Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Wed, 18 May 2022 11:03:24 -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 37ebd293b2950cd17960ea4ebd8222e868401820 Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Wed, 18 May 2022 11:09:08 -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 6ef7b86ac830..44491c68c64c 100644 --- a/ext/zend_test/test.c +++ b/ext/zend_test/test.c @@ -465,7 +465,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 6f641ecd9224bfe16aa30e8307118768852cca1d Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Wed, 18 May 2022 11:21:18 -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 4f916a663aefe381d73a869bc2eca51c78120f70 Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Wed, 18 May 2022 11:39:37 -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--