Deprecate returning non-string values from a user output handler

https://wiki.php.net/rfc/deprecations_php_8_4

Author: DanielEScherzer

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

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 {

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
 

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