From f4c3fde89284d5cf49abc88a5845d9c83907a253 Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Tue, 24 Jun 2025 13:49:34 -0700 Subject: [PATCH 1/8] Deprecate returning non-string values from a user output handler https://wiki.php.net/rfc/deprecations_php_8_4 --- UPGRADING | 6 +++++ main/output.c | 39 ++++++++++++++++++++-------- tests/output/bug60768.phpt | 2 +- tests/output/ob_start_basic_002.phpt | 8 ++++-- 4 files changed, 41 insertions(+), 14 deletions(-) diff --git a/UPGRADING b/UPGRADING index 4d968ef4daf8..d1dadf1f9bda 100644 --- a/UPGRADING +++ b/UPGRADING @@ -258,6 +258,12 @@ PHP 8.5 UPGRADE NOTES 4. Deprecated Functionality ======================================== +- Core: + . Returning a non-string from a user output handler is deprecated. The + deprecation warning will bypass user output handlers to ensure it is + visible. + RFC: https://wiki.php.net/rfc/deprecations_php_8_4 + - Hash: . The MHASH_* constants have been deprecated. These have been overlooked when the mhash*() function family has been deprecated per diff --git a/main/output.c b/main/output.c index ef6be672d1c1..827a94b0ab10 100644 --- a/main/output.c +++ b/main/output.c @@ -948,6 +948,7 @@ static inline php_output_handler_status_t php_output_handler_op(php_output_handl if (handler->flags & PHP_OUTPUT_HANDLER_USER) { zval ob_args[2]; zval retval; + ZVAL_UNDEF(&retval); /* ob_data */ ZVAL_STRINGL(&ob_args[0], handler->buffer.data, handler->buffer.used); @@ -959,17 +960,33 @@ static inline php_output_handler_status_t php_output_handler_op(php_output_handl handler->func.user->fci.params = ob_args; handler->func.user->fci.retval = &retval; -#define PHP_OUTPUT_USER_SUCCESS(retval) ((Z_TYPE(retval) != IS_UNDEF) && !(Z_TYPE(retval) == IS_FALSE)) - if (SUCCESS == zend_call_function(&handler->func.user->fci, &handler->func.user->fcc) && PHP_OUTPUT_USER_SUCCESS(retval)) { - /* user handler may have returned TRUE */ - status = PHP_OUTPUT_HANDLER_NO_DATA; - if (Z_TYPE(retval) != IS_FALSE && Z_TYPE(retval) != IS_TRUE) { - convert_to_string(&retval); - if (Z_STRLEN(retval)) { - context->out.data = estrndup(Z_STRVAL(retval), Z_STRLEN(retval)); - context->out.used = Z_STRLEN(retval); - context->out.free = 1; - status = PHP_OUTPUT_HANDLER_SUCCESS; + if (SUCCESS == zend_call_function(&handler->func.user->fci, &handler->func.user->fcc) && Z_TYPE(retval) != IS_UNDEF) { + if (Z_TYPE(retval) != IS_STRING) { + // Make sure that we don't get lost in an output buffer + int old_flags = OG(flags); + OG(flags) = old_flags & (~PHP_OUTPUT_ACTIVATED); + php_error_docref( + NULL, + E_DEPRECATED, + "Returning a non-string result from user output handler %s is deprecated", + ZSTR_VAL(handler->name) + ); + OG(flags) = old_flags; + } + if (Z_TYPE(retval) == IS_FALSE) { + /* call failed, pass internal buffer along */ + status = PHP_OUTPUT_HANDLER_FAILURE; + } else { + /* user handler may have returned TRUE */ + status = PHP_OUTPUT_HANDLER_NO_DATA; + if (Z_TYPE(retval) != IS_FALSE && Z_TYPE(retval) != IS_TRUE) { + convert_to_string(&retval); + if (Z_STRLEN(retval)) { + context->out.data = estrndup(Z_STRVAL(retval), Z_STRLEN(retval)); + context->out.used = Z_STRLEN(retval); + context->out.free = 1; + status = PHP_OUTPUT_HANDLER_SUCCESS; + } } } } else { diff --git a/tests/output/bug60768.phpt b/tests/output/bug60768.phpt index 4de7fbd69ff4..1ab702fa6468 100644 --- a/tests/output/bug60768.phpt +++ b/tests/output/bug60768.phpt @@ -5,7 +5,7 @@ Bug #60768 Output buffer not discarded global $storage; -ob_start(function($buffer) use (&$storage) { $storage .= $buffer; }, 20); +ob_start(function($buffer) use (&$storage) { $storage .= $buffer; return ''; }, 20); echo str_repeat("0", 20); // fill in the buffer diff --git a/tests/output/ob_start_basic_002.phpt b/tests/output/ob_start_basic_002.phpt index 700dab5d3c38..7207c2a95914 100644 --- a/tests/output/ob_start_basic_002.phpt +++ b/tests/output/ob_start_basic_002.phpt @@ -35,19 +35,23 @@ foreach ($callbacks as $callback) { } ?> ---EXPECT-- +--EXPECTF-- --> Use callback 'return_empty_string': --> Use callback 'return_false': My output. +Deprecated: ob_end_flush(): Returning a non-string result from user output handler return_false is deprecated in %s on line %d + --> Use callback 'return_null': + +Deprecated: ob_end_flush(): Returning a non-string result from user output handler return_null is deprecated in %s on line %d --> Use callback 'return_string': I stole your output. --> Use callback 'return_zero': 0 - +Deprecated: ob_end_flush(): Returning a non-string result from user output handler return_zero is deprecated in %s on line %d From 340c7fdbe69a8cb31905db991fc0aadf12e10110 Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Tue, 24 Jun 2025 14:35:46 -0700 Subject: [PATCH 2/8] Only disable the problematic output handler And update tests --- Zend/tests/concat/bug79836.phpt | 7 ++++++- Zend/tests/concat/bug79836_1.phpt | 5 ++++- Zend/tests/concat/bug79836_2.phpt | 5 ++++- Zend/tests/declare/gh18033_2.phpt | 3 ++- Zend/tests/gh11189.phpt | 2 ++ Zend/tests/gh11189_1.phpt | 2 ++ Zend/tests/gh16408.phpt | 7 ++++++- ext/session/tests/user_session_module/bug61728.phpt | 3 ++- main/output.c | 8 ++++---- sapi/cli/tests/gh8827-002.phpt | 12 +++++++++++- tests/output/ob_start_basic_002.phpt | 9 +++++---- 11 files changed, 48 insertions(+), 15 deletions(-) diff --git a/Zend/tests/concat/bug79836.phpt b/Zend/tests/concat/bug79836.phpt index 5fb07396762f..0383bd0fb636 100644 --- a/Zend/tests/concat/bug79836.phpt +++ b/Zend/tests/concat/bug79836.phpt @@ -14,5 +14,10 @@ $c .= []; ob_end_clean(); echo $counter . "\n"; ?> ---EXPECT-- +--EXPECTF-- +Deprecated: main(): Returning a non-string result from user output handler Closure::__invoke is deprecated in %s on line %d + +Deprecated: main(): Returning a non-string result from user output handler Closure::__invoke is deprecated in %s on line %d + +Deprecated: ob_end_clean(): Returning a non-string result from user output handler Closure::__invoke is deprecated in %s on line %d 3 diff --git a/Zend/tests/concat/bug79836_1.phpt b/Zend/tests/concat/bug79836_1.phpt index 86e7f4767184..28a04ad47f5c 100644 --- a/Zend/tests/concat/bug79836_1.phpt +++ b/Zend/tests/concat/bug79836_1.phpt @@ -14,5 +14,8 @@ $x = $c . $x; ob_end_clean(); echo "Done\n"; ?> ---EXPECT-- +--EXPECTF-- +Deprecated: main(): Returning a non-string result from user output handler Closure::__invoke is deprecated in %s on line %d + +Deprecated: ob_end_clean(): Returning a non-string result from user output handler Closure::__invoke is deprecated in %s on line %d Done diff --git a/Zend/tests/concat/bug79836_2.phpt b/Zend/tests/concat/bug79836_2.phpt index b02fcc13ea11..eeba0135468c 100644 --- a/Zend/tests/concat/bug79836_2.phpt +++ b/Zend/tests/concat/bug79836_2.phpt @@ -21,5 +21,8 @@ $x = $c . $xxx; ob_end_clean(); echo $x . "\n"; ?> ---EXPECT-- +--EXPECTF-- +Deprecated: X::__toString(): Returning a non-string result from user output handler Closure::__invoke is deprecated in %s on line %d + +Deprecated: ob_end_clean(): Returning a non-string result from user output handler Closure::__invoke is deprecated in %s on line %d abcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabc diff --git a/Zend/tests/declare/gh18033_2.phpt b/Zend/tests/declare/gh18033_2.phpt index 8fdcff1b51e6..45d506b9b1d8 100644 --- a/Zend/tests/declare/gh18033_2.phpt +++ b/Zend/tests/declare/gh18033_2.phpt @@ -13,4 +13,5 @@ ob_start(function() { ); }); ?> ---EXPECT-- +--EXPECTF-- +Deprecated: PHP Request Shutdown: Returning a non-string result from user output handler Closure::__invoke is deprecated in %s on line %d diff --git a/Zend/tests/gh11189.phpt b/Zend/tests/gh11189.phpt index f1c877f20ee4..84174023dee0 100644 --- a/Zend/tests/gh11189.phpt +++ b/Zend/tests/gh11189.phpt @@ -26,4 +26,6 @@ while (1) { ?> --EXPECTF-- Success +Deprecated: main(): Returning a non-string result from user output handler Closure::__invoke is deprecated in %s on line %d + Fatal error: Allowed memory size of %s bytes exhausted%s(tried to allocate %s bytes) in %s on line %d diff --git a/Zend/tests/gh11189_1.phpt b/Zend/tests/gh11189_1.phpt index 53727908e5e2..0d8b84d2667f 100644 --- a/Zend/tests/gh11189_1.phpt +++ b/Zend/tests/gh11189_1.phpt @@ -26,4 +26,6 @@ while (1) { ?> --EXPECTF-- Success +Deprecated: main(): Returning a non-string result from user output handler Closure::__invoke is deprecated in %s on line %d + Fatal error: Allowed memory size of %s bytes exhausted%s(tried to allocate %s bytes) in %s on line %d diff --git a/Zend/tests/gh16408.phpt b/Zend/tests/gh16408.phpt index f28c6435bfe7..de09c84803de 100644 --- a/Zend/tests/gh16408.phpt +++ b/Zend/tests/gh16408.phpt @@ -12,5 +12,10 @@ $c .= []; ob_end_clean(); echo $counter . "\n"; ?> ---EXPECT-- +--EXPECTF-- +Deprecated: main(): Returning a non-string result from user output handler Closure::__invoke is deprecated in %s on line %d + +Deprecated: main(): Returning a non-string result from user output handler Closure::__invoke is deprecated in %s on line %d + +Deprecated: ob_end_clean(): Returning a non-string result from user output handler Closure::__invoke is deprecated in %s on line %d 3 diff --git a/ext/session/tests/user_session_module/bug61728.phpt b/ext/session/tests/user_session_module/bug61728.phpt index fd79fa6b2bce..7bc042e0116f 100644 --- a/ext/session/tests/user_session_module/bug61728.phpt +++ b/ext/session/tests/user_session_module/bug61728.phpt @@ -40,5 +40,6 @@ class MySessionHandler implements SessionHandlerInterface { session_set_save_handler(new MySessionHandler()); session_start(); ?> ---EXPECT-- +--EXPECTF-- +Deprecated: ob_end_flush(): Returning a non-string result from user output handler output_html is deprecated in %s on line %d 8 diff --git a/main/output.c b/main/output.c index 827a94b0ab10..edf3952d856f 100644 --- a/main/output.c +++ b/main/output.c @@ -962,16 +962,16 @@ static inline php_output_handler_status_t php_output_handler_op(php_output_handl if (SUCCESS == zend_call_function(&handler->func.user->fci, &handler->func.user->fcc) && Z_TYPE(retval) != IS_UNDEF) { if (Z_TYPE(retval) != IS_STRING) { - // Make sure that we don't get lost in an output buffer - int old_flags = OG(flags); - OG(flags) = old_flags & (~PHP_OUTPUT_ACTIVATED); + // Make sure that we don't get lost in the current output buffer + // by disabling it + handler->flags |= PHP_OUTPUT_HANDLER_DISABLED; php_error_docref( NULL, E_DEPRECATED, "Returning a non-string result from user output handler %s is deprecated", ZSTR_VAL(handler->name) ); - OG(flags) = old_flags; + handler->flags &= (~PHP_OUTPUT_HANDLER_DISABLED); } if (Z_TYPE(retval) == IS_FALSE) { /* call failed, pass internal buffer along */ diff --git a/sapi/cli/tests/gh8827-002.phpt b/sapi/cli/tests/gh8827-002.phpt index 00fd5cfa78f7..72e2aeb077c3 100644 --- a/sapi/cli/tests/gh8827-002.phpt +++ b/sapi/cli/tests/gh8827-002.phpt @@ -32,10 +32,20 @@ print "STDOUT:\n"; fclose(STDOUT); var_dump(@fopen('php://stdout', 'a')); ?> ---EXPECT-- +--EXPECTF-- STDIN: + +Deprecated: main(): Returning a non-string result from user output handler Closure::__invoke is deprecated in %s on line %d bool(false) + +Deprecated: var_dump(): Returning a non-string result from user output handler Closure::__invoke is deprecated in %s on line %d STDERR: + +Deprecated: main(): Returning a non-string result from user output handler Closure::__invoke is deprecated in %s on line %d bool(false) + +Deprecated: var_dump(): Returning a non-string result from user output handler Closure::__invoke is deprecated in %s on line %d STDOUT: + +Deprecated: main(): Returning a non-string result from user output handler Closure::__invoke is deprecated in %s on line %d bool(false) diff --git a/tests/output/ob_start_basic_002.phpt b/tests/output/ob_start_basic_002.phpt index 7207c2a95914..e9af2b5e1904 100644 --- a/tests/output/ob_start_basic_002.phpt +++ b/tests/output/ob_start_basic_002.phpt @@ -40,18 +40,19 @@ foreach ($callbacks as $callback) { --> Use callback 'return_false': -My output. -Deprecated: ob_end_flush(): Returning a non-string result from user output handler return_false is deprecated in %s on line %d +Deprecated: ob_end_flush(): Returning a non-string result from user output handler return_false is deprecated in %s on line %d +My output. --> Use callback 'return_null': +Deprecated: ob_end_flush(): Returning a non-string result from user output handler return_null is deprecated in %s on line %d -Deprecated: ob_end_flush(): Returning a non-string result from user output handler return_null is deprecated in %s on line %d --> Use callback 'return_string': I stole your output. --> Use callback 'return_zero': -0 + Deprecated: ob_end_flush(): Returning a non-string result from user output handler return_zero is deprecated in %s on line %d +0 From 720923ec809bb31e74e769b63f6b9bf4a3d9b95c Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Tue, 24 Jun 2025 15:05:30 -0700 Subject: [PATCH 3/8] Tests --- sapi/cli/tests/gh8827-003.phpt | 1 + .../exception_handler.phpt | 91 +++++++++++++++++ .../exception_handler_nested.phpt | 97 +++++++++++++++++++ .../multiple_handlers.phpt | 89 +++++++++++++++++ 4 files changed, 278 insertions(+) create mode 100644 tests/output/ob_start_callback_bad_return/exception_handler.phpt create mode 100644 tests/output/ob_start_callback_bad_return/exception_handler_nested.phpt create mode 100644 tests/output/ob_start_callback_bad_return/multiple_handlers.phpt diff --git a/sapi/cli/tests/gh8827-003.phpt b/sapi/cli/tests/gh8827-003.phpt index 11f7880770ed..12b6798a57c2 100644 --- a/sapi/cli/tests/gh8827-003.phpt +++ b/sapi/cli/tests/gh8827-003.phpt @@ -34,6 +34,7 @@ file_put_contents('php://fd/2', "Goes to stderrFile\n"); ob_start(function ($buffer) use ($stdoutStream) { fwrite($stdoutStream, $buffer); + return ''; }, 1); print "stdoutFile:\n"; diff --git a/tests/output/ob_start_callback_bad_return/exception_handler.phpt b/tests/output/ob_start_callback_bad_return/exception_handler.phpt new file mode 100644 index 000000000000..ee176d8afa69 --- /dev/null +++ b/tests/output/ob_start_callback_bad_return/exception_handler.phpt @@ -0,0 +1,91 @@ +--TEST-- +ob_start(): Check behaviour with deprecation converted to exception +--FILE-- +>>"; + return null; +} + +function return_false($string) { + global $log; + $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; + return false; +} + +function return_true($string) { + global $log; + $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; + return true; +} + +function return_zero($string) { + global $log; + $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; + return 0; +} + +$cases = ['return_null', 'return_false', 'return_true', 'return_zero']; +foreach ($cases as $case) { + $log = []; + echo "\n\nTesting: $case\n"; + ob_start($case); + echo "Inside of $case"; + try { + ob_end_flush(); + } catch (\ErrorException $e) { + echo $e . "\n"; + } + echo "\nEnd of $case, log was:\n"; + echo implode("\n", $log); +} + +?> +--EXPECTF-- +Testing: return_null +ErrorException: ob_end_flush(): Returning a non-string result from user output handler return_null is deprecated in %s:%d +Stack trace: +#0 [internal function]: {closure:%s:%d}(8192, 'ob_end_flush():...', %s, %d) +#1 %s(%d): ob_end_flush() +#2 {main} + +End of return_null, log was: +return_null: <<>> + +Testing: return_false +Inside of return_falseErrorException: ob_end_flush(): Returning a non-string result from user output handler return_false is deprecated in %s:%d +Stack trace: +#0 [internal function]: {closure:%s:%d}(8192, 'ob_end_flush():...', %s, %d) +#1 %s(%d): ob_end_flush() +#2 {main} + +End of return_false, log was: +return_false: <<>> + +Testing: return_true +ErrorException: ob_end_flush(): Returning a non-string result from user output handler return_true is deprecated in %s:%d +Stack trace: +#0 [internal function]: {closure:%s:%d}(8192, 'ob_end_flush():...', %s, %d) +#1 %s(%d): ob_end_flush() +#2 {main} + +End of return_true, log was: +return_true: <<>> + +Testing: return_zero +0ErrorException: ob_end_flush(): Returning a non-string result from user output handler return_zero is deprecated in %s:%d +Stack trace: +#0 [internal function]: {closure:%s:%d}(8192, 'ob_end_flush():...', %s, %d) +#1 %s(%d): ob_end_flush() +#2 {main} + +End of return_zero, log was: +return_zero: <<>> diff --git a/tests/output/ob_start_callback_bad_return/exception_handler_nested.phpt b/tests/output/ob_start_callback_bad_return/exception_handler_nested.phpt new file mode 100644 index 000000000000..5983c1a626ae --- /dev/null +++ b/tests/output/ob_start_callback_bad_return/exception_handler_nested.phpt @@ -0,0 +1,97 @@ +--TEST-- +ob_start(): Check behaviour with deprecation converted to exception +--FILE-- +>>"; + return null; +} + +function return_false($string) { + global $log; + $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; + return false; +} + +function return_true($string) { + global $log; + $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; + return true; +} + +function return_zero($string) { + global $log; + $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; + return 0; +} + +ob_start('return_null'); +ob_start('return_false'); +ob_start('return_true'); +ob_start('return_zero'); + +echo "In all of them\n\n"; +try { + ob_end_flush(); +} catch (\ErrorException $e) { + echo $e->getMessage() . "\n"; +} +echo "Ended return_zero handler\n\n"; + +try { + ob_end_flush(); +} catch (\ErrorException $e) { + echo $e->getMessage() . "\n"; +} +echo "Ended return_true handler\n\n"; + +try { + ob_end_flush(); +} catch (\ErrorException $e) { + echo $e->getMessage() . "\n"; +} +echo "Ended return_false handler\n\n"; + +try { + ob_end_flush(); +} catch (\ErrorException $e) { + echo $e->getMessage() . "\n"; +} +echo "Ended return_null handler\n\n"; + +echo "All handlers are over\n\n"; +echo implode("\n", $log); + +?> +--EXPECT-- +ob_end_flush(): Returning a non-string result from user output handler return_null is deprecated +Ended return_null handler + +All handlers are over + +return_zero: <<>> +return_true: <<<0ob_end_flush(): Returning a non-string result from user output handler return_zero is deprecated +Ended return_zero handler + +>>> +return_false: <<>> +return_null: <<>> diff --git a/tests/output/ob_start_callback_bad_return/multiple_handlers.phpt b/tests/output/ob_start_callback_bad_return/multiple_handlers.phpt new file mode 100644 index 000000000000..33ae03f9e558 --- /dev/null +++ b/tests/output/ob_start_callback_bad_return/multiple_handlers.phpt @@ -0,0 +1,89 @@ +--TEST-- +ob_start(): Check behaviour with multiple nested handlers with had return values +--FILE-- +>>"; + return $string; +} + +function return_empty_string($string) { + global $log; + $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; + return ""; +} + +function return_false($string) { + global $log; + $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; + return false; +} + +function return_true($string) { + global $log; + $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; + return true; +} + +function return_null($string) { + global $log; + $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; + return null; +} + +function return_string($string) { + global $log; + $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; + return "I stole your output."; +} + +function return_zero($string) { + global $log; + $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; + return 0; +} + +ob_start('return_given_string'); +ob_start('return_empty_string'); +ob_start('return_false'); +ob_start('return_true'); +ob_start('return_null'); +ob_start('return_string'); +ob_start('return_zero'); + +echo "Testing..."; + +ob_end_flush(); +ob_end_flush(); +ob_end_flush(); +ob_end_flush(); +ob_end_flush(); +ob_end_flush(); +ob_end_flush(); + +echo "\n\nLog:\n"; +echo implode("\n", $log); +?> +--EXPECTF-- +Log: +return_zero: <<>> +return_string: <<< +Deprecated: ob_end_flush(): Returning a non-string result from user output handler return_zero is deprecated in %s on line %d +0>>> +return_null: <<>> +return_true: <<< +Deprecated: ob_end_flush(): Returning a non-string result from user output handler return_null is deprecated in %s on line %d +>>> +return_false: <<< +Deprecated: ob_end_flush(): Returning a non-string result from user output handler return_true is deprecated in %s on line %d +>>> +return_empty_string: <<< +Deprecated: ob_end_flush(): Returning a non-string result from user output handler return_false is deprecated in %s on line %d + +Deprecated: ob_end_flush(): Returning a non-string result from user output handler return_true is deprecated in %s on line %d +>>> +return_given_string: <<<>>> From cfe4d712a8e7027fd280b3f51008f4136ae7454b Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Tue, 24 Jun 2025 15:27:32 -0700 Subject: [PATCH 4/8] upgrading --- UPGRADING | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/UPGRADING b/UPGRADING index d1dadf1f9bda..8661d3317446 100644 --- a/UPGRADING +++ b/UPGRADING @@ -260,8 +260,9 @@ PHP 8.5 UPGRADE NOTES - Core: . Returning a non-string from a user output handler is deprecated. The - deprecation warning will bypass user output handlers to ensure it is - visible. + deprecation warning will bypass the handler with the bad return to ensure + it is visible; if there are nested output handlers the next one will still + be used. RFC: https://wiki.php.net/rfc/deprecations_php_8_4 - Hash: From 51892ba751d0b3d1949a042544512f8f0fda3b10 Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Thu, 26 Jun 2025 13:06:50 -0700 Subject: [PATCH 5/8] Handlers might be freed from OOM --- main/output.c | 61 +++++++++++++++---- .../exception_handler.phpt | 19 +++--- .../handler_removed.phpt | 21 +++++++ 3 files changed, 83 insertions(+), 18 deletions(-) create mode 100644 tests/output/ob_start_callback_bad_return/handler_removed.phpt diff --git a/main/output.c b/main/output.c index edf3952d856f..2d52bf521cd1 100644 --- a/main/output.c +++ b/main/output.c @@ -934,6 +934,7 @@ static inline php_output_handler_status_t php_output_handler_op(php_output_handl return PHP_OUTPUT_HANDLER_FAILURE; } + bool still_have_handler = true; /* storable? */ if (php_output_handler_append(handler, &context->in) && !context->op) { context->op = original_op; @@ -971,7 +972,22 @@ static inline php_output_handler_status_t php_output_handler_op(php_output_handl "Returning a non-string result from user output handler %s is deprecated", ZSTR_VAL(handler->name) ); - handler->flags &= (~PHP_OUTPUT_HANDLER_DISABLED); + // Check if the handler is still in the list of handlers to + // determine if the PHP_OUTPUT_HANDLER_DISABLED flag can + // be removed + still_have_handler = false; + int handler_count = php_output_get_level(); + if (handler_count) { + php_output_handler **handlers = (php_output_handler **) zend_stack_base(&OG(handlers)); + for (int iii = 0; iii < handler_count; ++iii) { + php_output_handler *curr_handler = handlers[iii]; + if (curr_handler == handler) { + handler->flags &= (~PHP_OUTPUT_HANDLER_DISABLED); + still_have_handler = true; + break; + } + } + } } if (Z_TYPE(retval) == IS_FALSE) { /* call failed, pass internal buffer along */ @@ -1013,14 +1029,18 @@ static inline php_output_handler_status_t php_output_handler_op(php_output_handl status = PHP_OUTPUT_HANDLER_FAILURE; } } - handler->flags |= PHP_OUTPUT_HANDLER_STARTED; + if (still_have_handler) { + handler->flags |= PHP_OUTPUT_HANDLER_STARTED; + } OG(running) = NULL; } switch (status) { case PHP_OUTPUT_HANDLER_FAILURE: - /* disable this handler */ - handler->flags |= PHP_OUTPUT_HANDLER_DISABLED; + if (still_have_handler) { + /* disable this handler */ + handler->flags |= PHP_OUTPUT_HANDLER_DISABLED; + } /* discard any output */ if (context->out.data && context->out.free) { efree(context->out.data); @@ -1029,18 +1049,22 @@ static inline php_output_handler_status_t php_output_handler_op(php_output_handl context->out.data = handler->buffer.data; context->out.used = handler->buffer.used; context->out.free = 1; - handler->buffer.data = NULL; - handler->buffer.used = 0; - handler->buffer.size = 0; + if (still_have_handler) { + handler->buffer.data = NULL; + handler->buffer.used = 0; + handler->buffer.size = 0; + } break; case PHP_OUTPUT_HANDLER_NO_DATA: /* handler ate all */ php_output_context_reset(context); ZEND_FALLTHROUGH; case PHP_OUTPUT_HANDLER_SUCCESS: - /* no more buffered data */ - handler->buffer.used = 0; - handler->flags |= PHP_OUTPUT_HANDLER_PROCESSED; + if (still_have_handler) { + /* no more buffered data */ + handler->buffer.used = 0; + handler->flags |= PHP_OUTPUT_HANDLER_PROCESSED; + } break; } @@ -1242,6 +1266,19 @@ static int php_output_stack_pop(int flags) } php_output_handler_op(orphan, &context); } + // If it isn't still in the stack, cannot free it + bool still_have_handler = false; + int handler_count = php_output_get_level(); + if (handler_count) { + php_output_handler **handlers = (php_output_handler **) zend_stack_base(&OG(handlers)); + for (int iii = 0; iii < handler_count; ++iii) { + php_output_handler *curr_handler = handlers[iii]; + if (curr_handler == orphan) { + still_have_handler = true; + break; + } + } + } /* pop it off the stack */ zend_stack_del_top(&OG(handlers)); @@ -1257,7 +1294,9 @@ static int php_output_stack_pop(int flags) } /* destroy the handler (after write!) */ - php_output_handler_free(&orphan); + if (still_have_handler) { + php_output_handler_free(&orphan); + } php_output_context_dtor(&context); return 1; diff --git a/tests/output/ob_start_callback_bad_return/exception_handler.phpt b/tests/output/ob_start_callback_bad_return/exception_handler.phpt index ee176d8afa69..413f855d40cc 100644 --- a/tests/output/ob_start_callback_bad_return/exception_handler.phpt +++ b/tests/output/ob_start_callback_bad_return/exception_handler.phpt @@ -38,9 +38,9 @@ foreach ($cases as $case) { $log = []; echo "\n\nTesting: $case\n"; ob_start($case); - echo "Inside of $case"; + echo "Inside of $case\n"; try { - ob_end_flush(); + ob_end_flush(); } catch (\ErrorException $e) { echo $e . "\n"; } @@ -58,17 +58,20 @@ Stack trace: #2 {main} End of return_null, log was: -return_null: <<>> +return_null: <<>> Testing: return_false -Inside of return_falseErrorException: ob_end_flush(): Returning a non-string result from user output handler return_false is deprecated in %s:%d +Inside of return_false +ErrorException: ob_end_flush(): Returning a non-string result from user output handler return_false is deprecated in %s:%d Stack trace: #0 [internal function]: {closure:%s:%d}(8192, 'ob_end_flush():...', %s, %d) #1 %s(%d): ob_end_flush() #2 {main} End of return_false, log was: -return_false: <<>> +return_false: <<>> Testing: return_true ErrorException: ob_end_flush(): Returning a non-string result from user output handler return_true is deprecated in %s:%d @@ -78,7 +81,8 @@ Stack trace: #2 {main} End of return_true, log was: -return_true: <<>> +return_true: <<>> Testing: return_zero 0ErrorException: ob_end_flush(): Returning a non-string result from user output handler return_zero is deprecated in %s:%d @@ -88,4 +92,5 @@ Stack trace: #2 {main} End of return_zero, log was: -return_zero: <<>> +return_zero: <<>> diff --git a/tests/output/ob_start_callback_bad_return/handler_removed.phpt b/tests/output/ob_start_callback_bad_return/handler_removed.phpt new file mode 100644 index 000000000000..f26dec04182d --- /dev/null +++ b/tests/output/ob_start_callback_bad_return/handler_removed.phpt @@ -0,0 +1,21 @@ +--TEST-- +ob_start(): Check behaviour with deprecation when OOM triggers handler removal +--FILE-- + +--EXPECTF-- +Deprecated: main(): Returning a non-string result from user output handler Closure::__invoke is deprecated in %s on line %d + +Fatal error: Allowed memory size of %d bytes exhausted at %s:%d (tried to allocate %d bytes) in %s on line %d From 6a2d6c10779df1e73dac634b9e72c50120d711e2 Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Thu, 26 Jun 2025 13:30:59 -0700 Subject: [PATCH 6/8] Location not always given --- tests/output/ob_start_callback_bad_return/handler_removed.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/output/ob_start_callback_bad_return/handler_removed.phpt b/tests/output/ob_start_callback_bad_return/handler_removed.phpt index f26dec04182d..c524ed04029f 100644 --- a/tests/output/ob_start_callback_bad_return/handler_removed.phpt +++ b/tests/output/ob_start_callback_bad_return/handler_removed.phpt @@ -18,4 +18,4 @@ while (1) { --EXPECTF-- Deprecated: main(): Returning a non-string result from user output handler Closure::__invoke is deprecated in %s on line %d -Fatal error: Allowed memory size of %d bytes exhausted at %s:%d (tried to allocate %d bytes) in %s on line %d +Fatal error: Allowed memory size of %d bytes exhausted%s(tried to allocate %d bytes) in %s on line %d From ab5acde9b9e7dba9625aa719bbffade1f54e0c75 Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Thu, 26 Jun 2025 13:36:13 -0700 Subject: [PATCH 7/8] context is also unavailable --- main/output.c | 27 +++++++++---------- ...emoved.phpt => handler_false_removed.phpt} | 2 +- .../handler_true_removed.phpt | 21 +++++++++++++++ .../handler_zero_removed.phpt | 21 +++++++++++++++ 4 files changed, 56 insertions(+), 15 deletions(-) rename tests/output/ob_start_callback_bad_return/{handler_removed.phpt => handler_false_removed.phpt} (92%) create mode 100644 tests/output/ob_start_callback_bad_return/handler_true_removed.phpt create mode 100644 tests/output/ob_start_callback_bad_return/handler_zero_removed.phpt diff --git a/main/output.c b/main/output.c index 2d52bf521cd1..1437eb37273b 100644 --- a/main/output.c +++ b/main/output.c @@ -1035,12 +1035,15 @@ static inline php_output_handler_status_t php_output_handler_op(php_output_handl OG(running) = NULL; } + if (!still_have_handler) { + // Handler and context will have both already been freed + return status; + } + switch (status) { case PHP_OUTPUT_HANDLER_FAILURE: - if (still_have_handler) { - /* disable this handler */ - handler->flags |= PHP_OUTPUT_HANDLER_DISABLED; - } + /* disable this handler */ + handler->flags |= PHP_OUTPUT_HANDLER_DISABLED; /* discard any output */ if (context->out.data && context->out.free) { efree(context->out.data); @@ -1049,22 +1052,18 @@ static inline php_output_handler_status_t php_output_handler_op(php_output_handl context->out.data = handler->buffer.data; context->out.used = handler->buffer.used; context->out.free = 1; - if (still_have_handler) { - handler->buffer.data = NULL; - handler->buffer.used = 0; - handler->buffer.size = 0; - } + handler->buffer.data = NULL; + handler->buffer.used = 0; + handler->buffer.size = 0; break; case PHP_OUTPUT_HANDLER_NO_DATA: /* handler ate all */ php_output_context_reset(context); ZEND_FALLTHROUGH; case PHP_OUTPUT_HANDLER_SUCCESS: - if (still_have_handler) { - /* no more buffered data */ - handler->buffer.used = 0; - handler->flags |= PHP_OUTPUT_HANDLER_PROCESSED; - } + /* no more buffered data */ + handler->buffer.used = 0; + handler->flags |= PHP_OUTPUT_HANDLER_PROCESSED; break; } diff --git a/tests/output/ob_start_callback_bad_return/handler_removed.phpt b/tests/output/ob_start_callback_bad_return/handler_false_removed.phpt similarity index 92% rename from tests/output/ob_start_callback_bad_return/handler_removed.phpt rename to tests/output/ob_start_callback_bad_return/handler_false_removed.phpt index c524ed04029f..1614e7f6e4d7 100644 --- a/tests/output/ob_start_callback_bad_return/handler_removed.phpt +++ b/tests/output/ob_start_callback_bad_return/handler_false_removed.phpt @@ -1,5 +1,5 @@ --TEST-- -ob_start(): Check behaviour with deprecation when OOM triggers handler removal +ob_start(): Check behaviour with deprecation when OOM triggers handler removal (handler returns false) --FILE-- +--EXPECTF-- +Deprecated: main(): Returning a non-string result from user output handler Closure::__invoke is deprecated in %s on line %d + +Fatal error: Allowed memory size of %d bytes exhausted%s(tried to allocate %d bytes) in %s on line %d diff --git a/tests/output/ob_start_callback_bad_return/handler_zero_removed.phpt b/tests/output/ob_start_callback_bad_return/handler_zero_removed.phpt new file mode 100644 index 000000000000..f9bbd7d37927 --- /dev/null +++ b/tests/output/ob_start_callback_bad_return/handler_zero_removed.phpt @@ -0,0 +1,21 @@ +--TEST-- +ob_start(): Check behaviour with deprecation when OOM triggers handler removal (handler returns zero) +--FILE-- + +--EXPECTF-- +Deprecated: main(): Returning a non-string result from user output handler Closure::__invoke is deprecated in %s on line %d + +Fatal error: Allowed memory size of %d bytes exhausted%s(tried to allocate %d bytes) in %s on line %d From 25c35b8bcee8b33ffdde28058d7af919cd1d234c Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Thu, 26 Jun 2025 13:37:59 -0700 Subject: [PATCH 8/8] No tabs --- .../exception_handler.phpt | 38 +++++++++---------- .../exception_handler_nested.phpt | 32 ++++++++-------- .../multiple_handlers.phpt | 30 +++++++-------- 3 files changed, 50 insertions(+), 50 deletions(-) diff --git a/tests/output/ob_start_callback_bad_return/exception_handler.phpt b/tests/output/ob_start_callback_bad_return/exception_handler.phpt index 413f855d40cc..efe9ffc4516c 100644 --- a/tests/output/ob_start_callback_bad_return/exception_handler.phpt +++ b/tests/output/ob_start_callback_bad_return/exception_handler.phpt @@ -10,42 +10,42 @@ set_error_handler(function (int $errno, string $errstr, string $errfile, int $er }); function return_null($string) { - global $log; - $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; + global $log; + $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; return null; } function return_false($string) { - global $log; - $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; + global $log; + $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; return false; } function return_true($string) { - global $log; - $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; + global $log; + $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; return true; } function return_zero($string) { - global $log; - $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; + global $log; + $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; return 0; } $cases = ['return_null', 'return_false', 'return_true', 'return_zero']; foreach ($cases as $case) { - $log = []; - echo "\n\nTesting: $case\n"; - ob_start($case); - echo "Inside of $case\n"; - try { - ob_end_flush(); - } catch (\ErrorException $e) { - echo $e . "\n"; - } - echo "\nEnd of $case, log was:\n"; - echo implode("\n", $log); + $log = []; + echo "\n\nTesting: $case\n"; + ob_start($case); + echo "Inside of $case\n"; + try { + ob_end_flush(); + } catch (\ErrorException $e) { + echo $e . "\n"; + } + echo "\nEnd of $case, log was:\n"; + echo implode("\n", $log); } ?> diff --git a/tests/output/ob_start_callback_bad_return/exception_handler_nested.phpt b/tests/output/ob_start_callback_bad_return/exception_handler_nested.phpt index 5983c1a626ae..29f17f99b649 100644 --- a/tests/output/ob_start_callback_bad_return/exception_handler_nested.phpt +++ b/tests/output/ob_start_callback_bad_return/exception_handler_nested.phpt @@ -10,26 +10,26 @@ set_error_handler(function (int $errno, string $errstr, string $errfile, int $er }); function return_null($string) { - global $log; - $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; + global $log; + $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; return null; } function return_false($string) { - global $log; - $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; + global $log; + $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; return false; } function return_true($string) { - global $log; - $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; + global $log; + $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; return true; } function return_zero($string) { - global $log; - $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; + global $log; + $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; return 0; } @@ -40,30 +40,30 @@ ob_start('return_zero'); echo "In all of them\n\n"; try { - ob_end_flush(); + ob_end_flush(); } catch (\ErrorException $e) { - echo $e->getMessage() . "\n"; + echo $e->getMessage() . "\n"; } echo "Ended return_zero handler\n\n"; try { - ob_end_flush(); + ob_end_flush(); } catch (\ErrorException $e) { - echo $e->getMessage() . "\n"; + echo $e->getMessage() . "\n"; } echo "Ended return_true handler\n\n"; try { - ob_end_flush(); + ob_end_flush(); } catch (\ErrorException $e) { - echo $e->getMessage() . "\n"; + echo $e->getMessage() . "\n"; } echo "Ended return_false handler\n\n"; try { - ob_end_flush(); + ob_end_flush(); } catch (\ErrorException $e) { - echo $e->getMessage() . "\n"; + echo $e->getMessage() . "\n"; } echo "Ended return_null handler\n\n"; diff --git a/tests/output/ob_start_callback_bad_return/multiple_handlers.phpt b/tests/output/ob_start_callback_bad_return/multiple_handlers.phpt index 33ae03f9e558..fef8daf8783a 100644 --- a/tests/output/ob_start_callback_bad_return/multiple_handlers.phpt +++ b/tests/output/ob_start_callback_bad_return/multiple_handlers.phpt @@ -6,44 +6,44 @@ ob_start(): Check behaviour with multiple nested handlers with had return values $log = []; function return_given_string($string) { - global $log; - $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; - return $string; + global $log; + $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; + return $string; } function return_empty_string($string) { - global $log; - $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; + global $log; + $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; return ""; } function return_false($string) { - global $log; - $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; + global $log; + $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; return false; } function return_true($string) { - global $log; - $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; + global $log; + $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; return true; } function return_null($string) { - global $log; - $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; + global $log; + $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; return null; } function return_string($string) { - global $log; - $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; + global $log; + $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; return "I stole your output."; } function return_zero($string) { - global $log; - $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; + global $log; + $log[] = __FUNCTION__ . ": <<<" . $string . ">>>"; return 0; }