Deprecate producing output in a user output handler

UPGRADING

@@ -265,6 +265,12 @@ PHP 8.5 UPGRADE NOTES
     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
+  . Trying to produce output (e.g. with `echo`) within a user output handler
+    is deprecated. The deprecation warning will bypass the handler producing the
+    output to ensure it is visible; if there are nested output handlers the next
+    one will still be used. If a user output handler returns a non-string and
+    produces output, the warning about producing an output is emitted first.
+    RFC: https://wiki.php.net/rfc/deprecations_php_8_4
 
 - Hash:
   . The MHASH_* constants have been deprecated.  These have been overlooked

ext/mbstring/mbstring.c

@@ -1584,10 +1584,22 @@ PHP_FUNCTION(mb_output_handler)
 		if (SG(sapi_headers).send_default_content_type || free_mimetype) {
 			const char *charset = encoding->mime_name;
 			if (charset) {
-				char *p;
-				size_t len = spprintf(&p, 0, "Content-Type: %s; charset=%s",  mimetype, charset);
-				if (sapi_add_header(p, len, 0) != FAILURE) {
-					SG(sapi_headers).send_default_content_type = 0;
+				/* Don't try to add a header if we are in an output handler;
+				 * we aren't supposed to directly access the output globals
+				 * from outside of main/output.c, so just try to get the flags
+				 * for the currently running handler, will only succeed if
+				 * there is a handler running. */
+				int unused;
+				bool in_handler = php_output_handler_hook(
+					PHP_OUTPUT_HANDLER_HOOK_GET_FLAGS,
+					&unused
+				) == SUCCESS;
+				if (!in_handler) {
+					char *p;
+					size_t len = spprintf(&p, 0, "Content-Type: %s; charset=%s",  mimetype, charset);
+					if (sapi_add_header(p, len, 0) != FAILURE) {
+						SG(sapi_headers).send_default_content_type = 0;
+					}
 				}
 			}
 

main/output.c

@@ -934,6 +934,13 @@ static inline php_output_handler_status_t php_output_handler_op(php_output_handl
 		return PHP_OUTPUT_HANDLER_FAILURE;
 	}
 
+	/* php_output_lock_error() doesn't fail for PHP_OUTPUT_HANDLER_WRITE but
+	 * anything that gets written will silently be discarded, remember that we
+	 * tried to write so a deprecation warning can be emitted at the end. */
+	if (context->op == PHP_OUTPUT_HANDLER_WRITE && OG(active) && OG(running)) {
+		handler->flags |= PHP_OUTPUT_HANDLER_PRODUCED_OUTPUT;
+	}
+
 	bool still_have_handler = true;
 	/* storable? */
 	if (php_output_handler_append(handler, &context->in) && !context->op) {
@@ -962,16 +969,37 @@ static inline php_output_handler_status_t php_output_handler_op(php_output_handl
 			handler->func.user->fci.retval = &retval;
 
 			if (SUCCESS == zend_call_function(&handler->func.user->fci, &handler->func.user->fcc) && Z_TYPE(retval) != IS_UNDEF) {
-				if (Z_TYPE(retval) != IS_STRING) {
+				if (Z_TYPE(retval) != IS_STRING || handler->flags & PHP_OUTPUT_HANDLER_PRODUCED_OUTPUT) {
 					// 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)
-					);
+					// Make sure we keep a reference to the handler name in
+					// case
+					// * The handler produced output *and* returned a non-string
+					// * The first deprecation message causes the handler to
+					// be removed
+					zend_string *handler_name = handler->name;
+					zend_string_addref(handler_name);
+					if (handler->flags & PHP_OUTPUT_HANDLER_PRODUCED_OUTPUT) {
+						// The handler might not always produce output
+						handler->flags &= ~PHP_OUTPUT_HANDLER_PRODUCED_OUTPUT;
+						php_error_docref(
+							NULL,
+							E_DEPRECATED,
+							"Producing output from user output handler %s is deprecated",
+							ZSTR_VAL(handler_name)
+						);
+					}
+					if (Z_TYPE(retval) != IS_STRING) {
+						php_error_docref(
+							NULL,
+							E_DEPRECATED,
+							"Returning a non-string result from user output handler %s is deprecated",
+							ZSTR_VAL(handler_name)
+						);
+					}
+					zend_string_release(handler_name);
+
 					// Check if the handler is still in the list of handlers to
 					// determine if the PHP_OUTPUT_HANDLER_DISABLED flag can
 					// be removed

main/php_output.h

@@ -42,6 +42,7 @@
 #define PHP_OUTPUT_HANDLER_STARTED		0x1000
 #define PHP_OUTPUT_HANDLER_DISABLED		0x2000
 #define PHP_OUTPUT_HANDLER_PROCESSED	0x4000
+#define PHP_OUTPUT_HANDLER_PRODUCED_OUTPUT 0x8000
 
 #define PHP_OUTPUT_HANDLER_ABILITY_FLAGS(bitmask) ((bitmask) & ~0xf00f)
 

tests/output/ob_start_callback_bad_return/exception_handler.phpt

@@ -1,5 +1,5 @@
 --TEST--
-ob_start(): Check behaviour with deprecation converted to exception
+ob_start(): Check behaviour with deprecation converted to exception [bad return]
 --FILE--
 <?php
 

tests/output/ob_start_callback_bad_return/exception_handler_nested.phpt

@@ -1,5 +1,5 @@
 --TEST--
-ob_start(): Check behaviour with deprecation converted to exception
+ob_start(): Check behaviour with nested deprecation converted to exception [bad return]
 --FILE--
 <?php
 

tests/output/ob_start_callback_bad_return/multiple_handlers.phpt

@@ -1,5 +1,5 @@
 --TEST--
-ob_start(): Check behaviour with multiple nested handlers with had return values
+ob_start(): Check behaviour with multiple nested handlers with bad return values
 --FILE--
 <?php
 
@@ -69,21 +69,38 @@ echo "\n\nLog:\n";
 echo implode("\n", $log);
 ?>
 --EXPECTF--
+Deprecated: ob_end_flush(): Producing output from user output handler return_given_string is deprecated in %s on line %d3
+
+Deprecated: ob_end_flush(): Producing output from user output handler return_empty_string is deprecated in %s on line %d2
+
+
 Log:
 return_zero: <<<Testing...>>>
 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: <<<I stole your output.>>>
+return_null: <<<
+Deprecated: ob_end_flush(): Producing output from user output handler return_string is deprecated in %s on line %d
+I stole your output.>>>
 return_true: <<<
+Deprecated: ob_end_flush(): Producing output 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
 >>>
 return_false: <<<
+Deprecated: ob_end_flush(): Producing output from user output handler return_true 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_empty_string: <<<
+Deprecated: ob_end_flush(): Producing output 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
 
+Deprecated: ob_end_flush(): Producing output from user output handler return_true 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: <<<>>>
+return_given_string: <<<
+Deprecated: ob_end_flush(): Producing output from user output handler return_empty_string is deprecated in %s on line %d2
+>>>

tests/output/ob_start_callback_output/exception_handler.phpt

@@ -0,0 +1,88 @@
+--TEST--
+ob_start(): Check behaviour with deprecation converted to exception [produce output]
+--FILE--
+<?php
+
+$log = [];
+
+set_error_handler(function (int $errno, string $errstr, string $errfile, int $errline) {
+    throw new \ErrorException($errstr, 0, $errno, $errfile, $errline);
+});
+
+function first_handler($string) {
+    global $log;
+    $log[] = __FUNCTION__ . ": <<<" . $string . ">>>";
+    echo __FUNCTION__;
+    return "FIRST\n";
+}
+
+function second_handler($string) {
+    global $log;
+    $log[] = __FUNCTION__ . ": <<<" . $string . ">>>";
+    echo __FUNCTION__;
+    return "SECOND\n";
+}
+
+function third_handler($string) {
+    global $log;
+    $log[] = __FUNCTION__ . ": <<<" . $string . ">>>";
+    echo __FUNCTION__;
+    return "THIRD\n";
+}
+
+$cases = [
+    'first_handler',
+    'second_handler',
+    'third_handler',
+];
+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);
+}
+
+?>
+--EXPECTF--
+Testing: first_handler
+FIRST
+ErrorException: ob_end_flush(): Producing output from user output handler first_handler is deprecated in %s:%d
+Stack trace:
+#0 [internal function]: {closure:%s:%d}(8192, 'ob_end_flush():...', %s, 41)
+#1 %s(%d): ob_end_flush()
+#2 {main}
+
+End of first_handler, log was:
+first_handler: <<<Inside of first_handler
+>>>
+
+Testing: second_handler
+SECOND
+ErrorException: ob_end_flush(): Producing output from user output handler second_handler is deprecated in %s:%d
+Stack trace:
+#0 [internal function]: {closure:%s:%d}(8192, 'ob_end_flush():...', %s, 41)
+#1 %s(%d): ob_end_flush()
+#2 {main}
+
+End of second_handler, log was:
+second_handler: <<<Inside of second_handler
+>>>
+
+Testing: third_handler
+THIRD
+ErrorException: ob_end_flush(): Producing output from user output handler third_handler is deprecated in %s:%d
+Stack trace:
+#0 [internal function]: {closure:%s:%d}(8192, 'ob_end_flush():...', %s, 41)
+#1 %s(%d): ob_end_flush()
+#2 {main}
+
+End of third_handler, log was:
+third_handler: <<<Inside of third_handler
+>>>

tests/output/ob_start_callback_output/exception_handler_nested.phpt

@@ -0,0 +1,82 @@
+--TEST--
+ob_start(): Check behaviour with nested deprecation converted to exception [produce output]
+--FILE--
+<?php
+
+$log = [];
+
+set_error_handler(function (int $errno, string $errstr, string $errfile, int $errline) {
+    throw new \ErrorException($errstr, 0, $errno, $errfile, $errline);
+});
+
+function first_handler($string) {
+    global $log;
+    $log[] = __FUNCTION__ . ": <<<" . $string . ">>>";
+    echo __FUNCTION__;
+    return "FIRST\n";
+}
+
+function second_handler($string) {
+    global $log;
+    $log[] = __FUNCTION__ . ": <<<" . $string . ">>>";
+    echo __FUNCTION__;
+    return "SECOND\n";
+}
+
+function third_handler($string) {
+    global $log;
+    $log[] = __FUNCTION__ . ": <<<" . $string . ">>>";
+    echo __FUNCTION__;
+    return "THIRD\n";
+}
+
+ob_start('first_handler');
+ob_start('second_handler');
+ob_start('third_handler');
+
+echo "In all of them\n\n";
+try {
+    ob_end_flush();
+} catch (\ErrorException $e) {
+    echo $e->getMessage() . "\n";
+}
+echo "Ended third_handler\n\n";
+
+try {
+    ob_end_flush();
+} catch (\ErrorException $e) {
+    echo $e->getMessage() . "\n";
+}
+echo "Ended second_handler\n\n";
+
+try {
+    ob_end_flush();
+} catch (\ErrorException $e) {
+    echo $e->getMessage() . "\n";
+}
+echo "Ended first_handler handler\n\n";
+
+echo "All handlers are over\n\n";
+echo implode("\n", $log);
+
+?>
+--EXPECT--
+FIRST
+ob_end_flush(): Producing output from user output handler first_handler is deprecated
+Ended first_handler handler
+
+All handlers are over
+
+third_handler: <<<In all of them
+
+>>>
+second_handler: <<<THIRD
+ob_end_flush(): Producing output from user output handler third_handler is deprecated
+Ended third_handler
+
+>>>
+first_handler: <<<SECOND
+ob_end_flush(): Producing output from user output handler second_handler is deprecated
+Ended second_handler
+
+>>>