From 6d12aec0a5205175319b1651c79966d704ec46b8 Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Mon, 16 May 2022 16:53:13 -0600 Subject: [PATCH 1/5] 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/test.c | 6 ++++++ ext/zend_test/tests/gh8575.phpt | 11 +++++++++++ sapi/cli/php_cli.c | 14 ++++++++------ 4 files changed, 28 insertions(+), 6 deletions(-) create mode 100644 ext/zend_test/tests/gh8575.phpt diff --git a/NEWS b/NEWS index a2c9c35a7a7f..eff98c9c8059 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,9 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? 2022, PHP 8.0.20 +- CLI: + . Fixed 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 69578e0ad1a5..f4004a505b77 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 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 62bc619db63d..545ac19e84d3 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); From eb36eae35b8bf17ee9bec10c7721314461b0cad7 Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Wed, 18 May 2022 10:49:11 -0600 Subject: [PATCH 2/5] [ci skip] Adjust NEWS entry --- NEWS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS b/NEWS index eff98c9c8059..dbe20dd6bece 100644 --- a/NEWS +++ b/NEWS @@ -3,7 +3,7 @@ PHP NEWS ?? ??? 2022, PHP 8.0.20 - 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 builds. (David Carlier) From d5300134c988484e5a972054fa39ac2eba81ca0d Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Wed, 18 May 2022 11:02:26 -0600 Subject: [PATCH 3/5] Guard gh8575 for CLI only --- ext/zend_test/tests/gh8575.phpt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ext/zend_test/tests/gh8575.phpt b/ext/zend_test/tests/gh8575.phpt index 4d947611a194..d3fd21cb48a7 100644 --- a/ext/zend_test/tests/gh8575.phpt +++ b/ext/zend_test/tests/gh8575.phpt @@ -1,7 +1,10 @@ --TEST-- CLI: stderr is available in mshutdown --SKIPIF-- - + --INI-- zend_test.print_stderr_mshutdown=1 --FILE-- From 36ac8a89741793fcf36f6f5d5b4fec67b9a51855 Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Wed, 18 May 2022 11:20:48 -0600 Subject: [PATCH 4/5] Use 'skip' not 'skip:' --- ext/zend_test/tests/gh8575.phpt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/zend_test/tests/gh8575.phpt b/ext/zend_test/tests/gh8575.phpt index d3fd21cb48a7..b5593657e732 100644 --- a/ext/zend_test/tests/gh8575.phpt +++ b/ext/zend_test/tests/gh8575.phpt @@ -2,8 +2,8 @@ CLI: stderr is available in mshutdown --SKIPIF-- --INI-- zend_test.print_stderr_mshutdown=1 From 12ea8325c6436144733f8bcd704c6a342f9ca4d9 Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Wed, 18 May 2022 11:39:01 -0600 Subject: [PATCH 5/5] 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 b5593657e732..8cf1d68dcab3 100644 --- a/ext/zend_test/tests/gh8575.phpt +++ b/ext/zend_test/tests/gh8575.phpt @@ -3,7 +3,7 @@ CLI: stderr is available in mshutdown --SKIPIF-- --INI-- zend_test.print_stderr_mshutdown=1