From fa78e17724e8bedfcea93a6f960ec052d0978437 Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Mon, 16 May 2022 16:53:13 -0600 Subject: [PATCH] Stop closing stderr and stdout streams (#8569) 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/test.c | 6 ++++++ ext/zend_test/tests/gh8575.phpt | 14 ++++++++++++++ sapi/cli/php_cli.c | 14 ++++++++------ 4 files changed, 31 insertions(+), 6 deletions(-) create mode 100644 ext/zend_test/tests/gh8575.phpt diff --git a/NEWS b/NEWS index bb7a0a444c5..722d5a7ce47 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,9 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? 2022, PHP 8.0.20 +- CLI: + . Fixed bug GH-8575 (CLI closes standard streams too early). (Levi Morrison) + - Core: . Fixed Haiku ZTS builds. (David Carlier) diff --git a/ext/zend_test/test.c b/ext/zend_test/test.c index 69578e0ad1a..f4004a505b7 100644 --- a/ext/zend_test/test.c +++ b/ext/zend_test/test.c @@ -42,6 +42,7 @@ ZEND_BEGIN_MODULE_GLOBALS(zend_test) int observer_show_opcode; int observer_nesting_depth; int replace_zend_execute_ex; + zend_bool print_stderr_mshutdown; HashTable global_weakmap; ZEND_END_MODULE_GLOBALS(zend_test) @@ -407,6 +408,7 @@ PHP_INI_BEGIN() STD_PHP_INI_BOOLEAN("zend_test.observer.show_init_backtrace", "0", PHP_INI_SYSTEM, OnUpdateBool, observer_show_init_backtrace, zend_zend_test_globals, zend_test_globals) STD_PHP_INI_BOOLEAN("zend_test.observer.show_opcode", "0", PHP_INI_SYSTEM, OnUpdateBool, observer_show_opcode, zend_zend_test_globals, zend_test_globals) 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.print_stderr_mshutdown", "0", PHP_INI_SYSTEM, OnUpdateBool, print_stderr_mshutdown, zend_zend_test_globals, zend_test_globals) PHP_INI_END() static zend_observer_fcall_handlers observer_fcall_init(zend_execute_data *execute_data); @@ -526,6 +528,10 @@ PHP_MSHUTDOWN_FUNCTION(zend_test) UNREGISTER_INI_ENTRIES(); } + 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 00000000000..8cf1d68dcab --- /dev/null +++ b/ext/zend_test/tests/gh8575.phpt @@ -0,0 +1,14 @@ +--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 62bc619db63..545ac19e84d 100644 --- a/sapi/cli/php_cli.c +++ b/sapi/cli/php_cli.c @@ -539,6 +539,14 @@ static void cli_register_file_handles(void) /* {{{ */ 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); @@ -546,12 +554,6 @@ static void cli_register_file_handles(void) /* {{{ */ return; } -#if PHP_DEBUG - /* do not close stdout and stderr */ - s_out->flags |= PHP_STREAM_FLAG_NO_CLOSE; - s_err->flags |= PHP_STREAM_FLAG_NO_CLOSE; -#endif - s_in_process = s_in; php_stream_to_zval(s_in, &ic.value);