From efaeb8d222e4e1fea29735b2cbfbd5982880e1ab Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Wed, 17 Jun 2026 00:19:31 +0200 Subject: [PATCH] Fix timeout after supplemental read of non-blocking stream 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. --- .../stream_supplemental_read_timeout.phpt | 90 +++++++++++++++++++ ext/openssl/xp_ssl.c | 12 ++- 2 files changed, 98 insertions(+), 4 deletions(-) create mode 100644 ext/openssl/tests/stream_supplemental_read_timeout.phpt 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..55adaa56e920 --- /dev/null +++ b/ext/openssl/tests/stream_supplemental_read_timeout.phpt @@ -0,0 +1,90 @@ +--TEST-- +Supplemental read of blocking stream does not cause timeout +--EXTENSIONS-- +openssl +--SKIPIF-- + +--FILE-- + ['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-- + +--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; }