[PHP-CVS] [php-src] master: sapi/cli: check php_cli_server_client_send_through() return value

Author: Lazizbek Ergashev (lazerg)
Committer: David Carlier (devnexen)
Date: 2026-08-24T16:57:24+01:00

Commit: https://github.com/php/php-src/commit/6748db3ff57208ae37a9cd9736fd14b2b5040b34
Raw diff: https://github.com/php/php-src/commit/6748db3ff57208ae37a9cd9736fd14b2b5040b34.diff

sapi/cli: check php_cli_server_client_send_through() return value

sapi_cli_server_send_headers() sent the header buffer through
php_cli_server_client_send_through() but dropped the return value, so it
always reported SAPI_HEADER_SENT_SUCCESSFULLY, even when the send failed
and php_handle_aborted_connection() did not bail out (ignore_user_abort=1).
It now compares the send result against the buffer length and returns
SAPI_HEADER_SEND_FAILED when it comes up short.

php_cli_server_client_send_through() itself returned the number of bytes
left on failure, which equals str_len when nothing was sent: the same
value it returns on success. It now returns the number of bytes actually
sent on both paths.

Since sapi_send_headers() resets headers_sent on SAPI_HEADER_SEND_FAILED,
a headers_written flag on the client guards against rebuilding and
resending the whole header block on every subsequent output write.

Close GH-23428

Changed paths:
  A  sapi/cli/tests/gh23425.phpt
  M  NEWS
  M  sapi/cli/php_cli_server.c


Diff:

diff --git a/NEWS b/NEWS
index a2c65685b4ce..a4339ddf600f 100644
--- a/NEWS
+++ b/NEWS
@@ -8,6 +8,10 @@ PHP                                                                        NEWS
   . Fixed bug GH-23301 (Nested "yield from" yields a value twice when the
     middle generator delegates again). (Lazizbek Ergashev)
 
+- CLI:
+  . Fixed bug GH-23425 (sapi_cli_server_send_headers() does not check the
+    return value of php_cli_server_client_send_through()). (Lazizbek Ergashev)
+
 - DOM:
   . Fixed a use-after-free when cloning a DOMNameSpaceNode after
     DOMDocument::xinclude(). (iliaal)
diff --git a/sapi/cli/php_cli_server.c b/sapi/cli/php_cli_server.c
index f4d29ce56855..36187aaeb035 100644
--- a/sapi/cli/php_cli_server.c
+++ b/sapi/cli/php_cli_server.c
@@ -177,6 +177,7 @@ typedef struct php_cli_server_client {
 	zend_string *addr_str;
 	php_http_parser parser;
 	bool request_read;
+	bool headers_written;
 	zend_string *current_header_name;
 	zend_string *current_header_value;
 	enum { HEADER_NONE=0, HEADER_FIELD, HEADER_VALUE } last_header_element;
@@ -555,7 +556,7 @@ static int sapi_cli_server_send_headers(sapi_headers_struct *sapi_headers) /* {{
 	sapi_header_struct *h;
 	zend_llist_position pos;
 
-	if (client == NULL || SG(request_info).no_headers) {
+	if (client == NULL || SG(request_info).no_headers || client->headers_written) {
 		return SAPI_HEADER_SENT_SUCCESSFULLY;
 	}
 
@@ -578,10 +579,12 @@ static int sapi_cli_server_send_headers(sapi_headers_struct *sapi_headers) /* {{
 	}
 	smart_str_appendl(&buffer, "\r\n", 2);
 
-	php_cli_server_client_send_through(client, ZSTR_VAL(buffer.s), ZSTR_LEN(buffer.s));
+	size_t buffer_len = ZSTR_LEN(buffer.s);
+	bool sent = php_cli_server_client_send_through(client, ZSTR_VAL(buffer.s), buffer_len) == buffer_len;
 
+	client->headers_written = true;
 	smart_str_free(&buffer);
-	return SAPI_HEADER_SENT_SUCCESSFULLY;
+	return sent ? SAPI_HEADER_SENT_SUCCESSFULLY : SAPI_HEADER_SEND_FAILED;
 }
 /* }}} */
 
@@ -1920,11 +1923,11 @@ static size_t php_cli_server_client_send_through(php_cli_server_client *client,
 				} else {
 					/* error or timeout */
 					php_handle_aborted_connection();
-					return nbytes_left;
+					return str_len - nbytes_left;
 				}
 			} else {
 				php_handle_aborted_connection();
-				return nbytes_left;
+				return str_len - nbytes_left;
 			}
 		}
 		nbytes_left -= nbytes_sent;
@@ -1973,6 +1976,7 @@ static void php_cli_server_client_ctor(php_cli_server_client *client, php_cli_se
 
 	php_http_parser_init(&client->parser, PHP_HTTP_REQUEST);
 	client->request_read = false;
+	client->headers_written = false;
 
 	client->last_header_element = HEADER_NONE;
 	client->current_header_name = NULL;
diff --git a/sapi/cli/tests/gh23425.phpt b/sapi/cli/tests/gh23425.phpt
new file mode 100644
index 000000000000..98ed07859174
--- /dev/null
+++ b/sapi/cli/tests/gh23425.phpt
@@ -0,0 +1,40 @@
+--TEST--
+GH-23425 (sapi_cli_server_send_headers() does not check the return value of php_cli_server_client_send_through())
+--EXTENSIONS--
+sockets
+--SKIPIF--
+<?php
+include "skipif.inc";
+if (PHP_OS_FAMILY === "Windows") die("skip SO_LINGER reset behaviour differs on Windows");
+?>
+--FILE--
+<?php
+include "php_cli_server.inc";
+
+$info = php_cli_server_start(<<<'PHP'
+ignore_user_abort(true);
+usleep(300000);
+header('X-Test: 1');
+echo 'x';
+file_put_contents(__DIR__ . '/result.txt', headers_sent() ? 'sent' : 'not-sent');
+PHP);
+
+// Connect the usual way, then drop to the socket extension only to force a
+// hard reset (SO_LINGER=0) instead of a graceful close, so the server's
+// header write fails deterministically while the script is still running
+// (ignore_user_abort(true)).
+$stream = stream_socket_client("tcp://" . PHP_CLI_SERVER_ADDRESS);
+$sock = socket_import_stream($stream);
+socket_write($sock, "GET /index.php HTTP/1.1\r\nHost: " . PHP_CLI_SERVER_HOSTNAME . "\r\nConnection: close\r\n\r\n");
+socket_set_option($sock, SOL_SOCKET, SO_LINGER, ['l_onoff' => 1, 'l_linger' => 0]);
+socket_close($sock);
+
+$result_file = $info->docRoot . '/result.txt';
+for ($i = 0; $i < 40 && !file_exists($result_file); $i++) {
+    usleep(50000);
+}
+
+echo file_get_contents($result_file), "\n";
+?>
+--EXPECT--
+not-sent