[php-src] PHP-8.5: Fixed timeout for supplemental read at end of a blocking stream in SSL stream wrapper

Author: Ilija Tovilo (iluuu1994)
Date: 2026-07-01T12:03:02+02:00

Commit: https://github.com/php/php-src/commit/1c196d0ee5670b0e9873fad49dda5464b8294707
Raw diff: https://github.com/php/php-src/commit/1c196d0ee5670b0e9873fad49dda5464b8294707.diff

Fixed timeout for supplemental read at end of a blocking stream in SSL stream wrapper

This issue is related to GH-22332:

> Let's assume we have a stream with 5 queued bytes. When calling fread($s, 1),
> the underlying buffered stream will request a chunk of up to 8192 bytes
> immediately, but only return the requested 1 byte back to the reader. If the
> reader then requests fread($s, 10), _php_stream_read() will first return
> anything that was buffered but not yet read.
>
> If the requested length exceeds the number of buffered bytes (as is the case
> above), another read call is issued. This call will return nothing, because
> the stream only provides 4 more readable bytes, all of which are buffered.
> php_openssl_handle_ssl_error() (called by php_openssl_sockop_io()) will then
> incorrectly set last_status to WANT_READ, even though we've already read the
> remaining data.
>
> Furthermore, stream_select() can cause the same issue via
> php_openssl_sockop_cast(castas: PHP_STREAM_AS_FD_FOR_SELECT), which pre-fills
> the read buffer on SSL_pending() > 0. The subsequent fread() will lead to the
> same condition as above.
>
> There's a second issue here. If the stream is blocking, the supplement read
> will block for the duration of the timeout. This will be addressed in a second
> PR.

This addresses the last paragraph. Avoid a blocking read when we already have
buffered data, as we may have reached the end of the stream and will wait in
vain.

Closes GH-22346

Changed paths:
  A  ext/openssl/tests/stream_supplemental_read_timeout.phpt
  M  NEWS
  M  ext/openssl/xp_ssl.c


Diff:

diff --git a/NEWS b/NEWS
index d03a77b01d91..feb9221c2e0b 100644
--- a/NEWS
+++ b/NEWS
@@ -14,6 +14,10 @@ PHP                                                                        NEWS
   . Fixed IntlChar methods leaving stale global error state after successful
     calls. (Xuyang Zhang)
 
+- OpenSSL:
+  . Fixed timeout for supplemental read at end of a blocking stream in SSL
+    stream wrapper. (ilutov)
+
 - PDO_ODBC:
   . Fixed bug GH-20726 (Crash with ODBC connection pooling when the DSN
     carries no credentials). (iliaal)
diff --git a/ext/openssl/tests/stream_supplemental_read_timeout.phpt b/ext/openssl/tests/stream_supplemental_read_timeout.phpt
new file mode 100644
index 000000000000..0e36f146bf9e
--- /dev/null
+++ b/ext/openssl/tests/stream_supplemental_read_timeout.phpt
@@ -0,0 +1,90 @@
+--TEST--
+Timeout for supplemental read at end of a blocking stream in SSL stream wrapper
+--EXTENSIONS--
+openssl
+--SKIPIF--
+<?php
+if (!function_exists("proc_open")) die("skip no proc_open");
+?>
+--FILE--
+<?php
+$certFile = __DIR__ . DIRECTORY_SEPARATOR . 'crypto_supplemental_read_timeout.pem.tmp';
+$peerName = 'crypto-supplemental-read-timeout';
+
+$serverCode = <<<'CODE'
+    $ctx = stream_context_create(['ssl' => ['local_cert' => '%s']]);
+    $flags = STREAM_SERVER_BIND|STREAM_SERVER_LISTEN;
+    $server = stream_socket_server("tls://127.0.0.1:0", $errno, $errstr, $flags, $ctx);
+    phpt_notify_server_start($server);
+
+    $conn = stream_socket_accept($server, 30);
+
+    fwrite($conn, "hello\n");
+
+    phpt_wait();
+    fclose($conn);
+CODE;
+$serverCode = sprintf($serverCode, $certFile);
+
+$clientCode = <<<'CODE'
+    $ctx = stream_context_create(['ssl' => [
+        'verify_peer' => false,
+        'verify_peer_name' => false,
+        'peer_name' => '%s',
+    ]]);
+
+    $client = stream_socket_client("tls://{{ ADDR }}", $errno, $errstr, 30, STREAM_CLIENT_CONNECT, $ctx);
+    stream_set_blocking($client, true);
+    stream_set_timeout($client, 5);
+    $start = hrtime(true);
+
+    $buf = '';
+    $read = [$client];
+    $write = $except = null;
+    while (true) {
+        if (!stream_select($read, $write, $except, 5)) {
+            break;
+        }
+
+        // Initially, read only the first char, then request more than is stored
+        // in the buffer, triggering a supplemental read.
+        $chunk = fread($client, strlen($buf) === 0 ? 1 : 10);
+        if ($chunk === '' || $chunk === false) {
+            /* A non-application record (e.g. a TLS 1.3 session ticket) may arrive first. */
+            if (feof($client)) {
+                break;
+            }
+        } else {
+            $buf .= $chunk;
+            if (strlen($buf) >= 6) {
+                break;
+            }
+        }
+        $read = [$client];
+        $write = $except = null;
+    }
+
+    echo trim($buf), "\n";
+
+    $diff = (hrtime(true) - $start) / 1e9;
+    var_dump($diff < 4.0);
+
+    phpt_notify();
+    fclose($client);
+CODE;
+$clientCode = sprintf($clientCode, $peerName);
+
+include 'CertificateGenerator.inc';
+$certificateGenerator = new CertificateGenerator();
+$certificateGenerator->saveNewCertAsFileWithKey($peerName, $certFile);
+
+include 'ServerClientTestCase.inc';
+ServerClientTestCase::getInstance()->run($clientCode, $serverCode);
+?>
+--CLEAN--
+<?php
+@unlink(__DIR__ . DIRECTORY_SEPARATOR . 'crypto_supplemental_read_timeout.pem.tmp');
+?>
+--EXPECT--
+hello
+bool(true)
diff --git a/ext/openssl/xp_ssl.c b/ext/openssl/xp_ssl.c
index ffacd8a107b7..77c98f65b518 100644
--- a/ext/openssl/xp_ssl.c
+++ b/ext/openssl/xp_ssl.c
@@ -2069,7 +2069,11 @@ static ssize_t php_openssl_sockop_io(int read, php_stream *stream, char *buf, si
 
 	/* Only do this if SSL is active. */
 	if (sslsock->ssl_active) {
-		int retry = 1;
+		/* We have already returned some buffered data. Don't retry and don't
+		 * block. We're just trying to fill the buffer more, but the stream might
+		 * be empty, so we don't want to wait in vain. */
+		bool supplemental = stream->has_buffered_data;
+		int retry = !supplemental;
 		struct timeval start_time;
 		struct timeval *timeout = NULL;
 		int began_blocked = sslsock->s.is_blocked;
@@ -2082,11 +2086,11 @@ static ssize_t php_openssl_sockop_io(int read, php_stream *stream, char *buf, si
 		}
 
 		/* never use a timeout with non-blocking sockets */
-		if (began_blocked) {
+		if (began_blocked && !supplemental) {
 			timeout = &sslsock->s.timeout;
 		}
 
-		if (timeout) {
+		if (timeout || supplemental) {
 			php_openssl_set_blocking(sslsock, 0);
 		}
 
@@ -2160,7 +2164,7 @@ static ssize_t php_openssl_sockop_io(int read, php_stream *stream, char *buf, si
 				}
 
 				/* Don't loop indefinitely in non-blocking mode if no data is available */
-				if (began_blocked == 0) {
+				if (began_blocked == 0 || supplemental) {
 					break;
 				}