Fix bug #51056: fread() on blocking stream will block even if data is available

This is applied only on socket connection which already returns
immediately if there is no data in the buffer.

Author: bukka

NEWS

@@ -35,6 +35,10 @@ PHP                                                                        NEWS
     over socket binding for a cpu core. (David Carlier)
   . Added SKF_AD_QUEUE for cbpf filters. (David Carlier)
 
+- Streams:
+  . Fixed bug #51056: blocking fread() will block even if data is available.
+    (Jakub Zelenka)
+
 - Reflection:
   . Fix GH-9470 (ReflectionMethod constructor should not find private parent
     method). (ilutov)

UPGRADING

@@ -102,6 +102,10 @@ PHP 8.3 UPGRADE NOTES
     other SAPIs, this directive is required when running as root because
     preloading is done before the SAPI switches to an unprivileged user.
 
+- Streams:
+  . Blocking fread() on socket connection returns immediately if there are
+    any buffered data instead of waiting for more data.
+
 ========================================
 14. Performance Improvements
 ========================================

ext/standard/tests/streams/bug51056.phpt

@@ -0,0 +1,43 @@
+--TEST--
+Bug #51056 (fread() on blocking stream will block even if data is available)
+--FILE--
+<?php
+
+$serverCode = <<<'CODE'
+$server = stream_socket_server('tcp://127.0.0.1:64324');
+phpt_notify();
+
+$conn = stream_socket_accept($server);
+
+fwrite($conn, "Hello 1\n"); // 8 bytes
+usleep(50000);
+fwrite($conn, str_repeat('a', 300)."\n"); // 301 bytes
+usleep(50000);
+fwrite($conn, "Hello 1\n"); // 8 bytes
+
+fclose($conn);
+fclose($server);
+CODE;
+
+$clientCode = <<<'CODE'
+
+phpt_wait();
+
+$fp = fsockopen("tcp://127.0.0.1:64324");
+
+while(!feof($fp)) {
+  $data = fread($fp, 256);
+  printf("fread read %d bytes\n", strlen($data));
+}
+CODE;
+
+include sprintf("%s/../../../openssl/tests/ServerClientTestCase.inc", __DIR__);
+ServerClientTestCase::getInstance()->run($clientCode, $serverCode);
+
+?>
+--EXPECT--
+fread read 8 bytes
+fread read 256 bytes
+fread read 45 bytes
+fread read 8 bytes
+fread read 0 bytes

main/php_streams.h

@@ -225,6 +225,7 @@ struct _php_stream  {
 	size_t readbuflen;
 	zend_off_t readpos;
 	zend_off_t writepos;
+	ssize_t didread;
 
 	/* how much data to read when filling buffer */
 	size_t chunk_size;

main/streams/streams.c

@@ -680,7 +680,8 @@ PHPAPI zend_result _php_stream_fill_read_buffer(php_stream *stream, size_t size)
 
 PHPAPI ssize_t _php_stream_read(php_stream *stream, char *buf, size_t size)
 {
-	ssize_t toread = 0, didread = 0;
+	ssize_t toread = 0;
+	stream->didread = 0;
 
 	while (size > 0) {
 
@@ -699,7 +700,7 @@ PHPAPI ssize_t _php_stream_read(php_stream *stream, char *buf, size_t size)
 			stream->readpos += toread;
 			size -= toread;
 			buf += toread;
-			didread += toread;
+			stream->didread += toread;
 		}
 
 		/* ignore eof here; the underlying state might have changed */
@@ -712,14 +713,14 @@ PHPAPI ssize_t _php_stream_read(php_stream *stream, char *buf, size_t size)
 			if (toread < 0) {
 				/* Report an error if the read failed and we did not read any data
 				 * before that. Otherwise return the data we did read. */
-				if (didread == 0) {
+				if (stream->didread == 0) {
 					return toread;
 				}
 				break;
 			}
 		} else {
 			if (php_stream_fill_read_buffer(stream, size) != SUCCESS) {
-				if (didread == 0) {
+				if (stream->didread == 0) {
 					return -1;
 				}
 				break;
@@ -736,7 +737,7 @@ PHPAPI ssize_t _php_stream_read(php_stream *stream, char *buf, size_t size)
 			}
 		}
 		if (toread > 0) {
-			didread += toread;
+			stream->didread += toread;
 			buf += toread;
 			size -= toread;
 		} else {
@@ -752,11 +753,11 @@ PHPAPI ssize_t _php_stream_read(php_stream *stream, char *buf, size_t size)
 		}
 	}
 
-	if (didread > 0) {
-		stream->position += didread;
+	if (stream->didread > 0) {
+		stream->position += stream->didread;
 	}
 
-	return didread;
+	return stream->didread;
 }
 
 /* Like php_stream_read(), but reading into a zend_string buffer. This has some similarity

main/streams/xp_socket.c

@@ -120,21 +120,27 @@ static ssize_t php_sockop_write(php_stream *stream, const char *buf, size_t coun
 	return didwrite;
 }
 
-static void php_sock_stream_wait_for_data(php_stream *stream, php_netstream_data_t *sock)
+static void php_sock_stream_wait_for_data(php_stream *stream, php_netstream_data_t *sock, bool has_buffered_data)
 {
 	int retval;
-	struct timeval *ptimeout;
+	struct timeval *ptimeout, zero_timeout;
 
 	if (!sock || sock->socket == -1) {
 		return;
 	}
 
 	sock->timeout_event = 0;
 
-	if (sock->timeout.tv_sec == -1)
+	if (has_buffered_data) {
+		/* If there is already buffered data, use no timeout. */
+		zero_timeout.tv_sec = 0;
+		zero_timeout.tv_usec = 0;
+		ptimeout = &zero_timeout;
+	} else if (sock->timeout.tv_sec == -1) {
 		ptimeout = NULL;
-	else
+	} else {
 		ptimeout = &sock->timeout;
+	}
 
 	while(1) {
 		retval = php_pollfd_for(sock->socket, PHP_POLLREADABLE, ptimeout);
@@ -153,21 +159,37 @@ static void php_sock_stream_wait_for_data(php_stream *stream, php_netstream_data
 static ssize_t php_sockop_read(php_stream *stream, char *buf, size_t count)
 {
 	php_netstream_data_t *sock = (php_netstream_data_t*)stream->abstract;
-	ssize_t nr_bytes = 0;
-	int err;
 
 	if (!sock || sock->socket == -1) {
 		return -1;
 	}
 
+	int recv_flags = 0;
+	/* Special handling for blocking read. */
 	if (sock->is_blocked) {
-		php_sock_stream_wait_for_data(stream, sock);
-		if (sock->timeout_event)
-			return -1;
+		/* Find out if there is any data buffered from the previous read. */
+		bool has_buffered_data = stream->didread > 0;
+		/* No need to wait if there is any data buffered or no timeout. */
+		bool dont_wait = has_buffered_data ||
+				(sock->timeout.tv_sec == 0 && sock->timeout.tv_usec == 0);
+		/* Set MSG_DONTWAIT if no wait is needed or there is unlimited timeout which was
+		 * added by fix for #41984 commited in 9343c5404. */
+		if (dont_wait || sock->timeout.tv_sec != -1) {
+			recv_flags = MSG_DONTWAIT;
+		}
+		/* If the wait is needed or it is a platform without MSG_DONTWAIT support (e.g. Windows),
+		 * then poll for data. */
+		if (!dont_wait || MSG_DONTWAIT == 0) {
+			php_sock_stream_wait_for_data(stream, sock, has_buffered_data);
+			if (sock->timeout_event) {
+				/* It is ok to timeout if there is any data buffered so return 0, otherwise -1. */
+				return has_buffered_data ? 0 : -1;
+			}
+		}
 	}
 
-	nr_bytes = recv(sock->socket, buf, XP_SOCK_BUF_SIZE(count), (sock->is_blocked && sock->timeout.tv_sec != -1) ? MSG_DONTWAIT : 0);
-	err = php_socket_errno();
+	ssize_t nr_bytes = recv(sock->socket, buf, XP_SOCK_BUF_SIZE(count), recv_flags);
+	int err = php_socket_errno();
 
 	if (nr_bytes < 0) {
 		if (PHP_IS_TRANSIENT_ERROR(err)) {