diff --git a/NEWS b/NEWS index 8e569075eb2..65e7c30c23d 100644 --- a/NEWS +++ b/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) diff --git a/UPGRADING b/UPGRADING index ffcfdd32213..e50897c3d97 100644 --- a/UPGRADING +++ b/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 ======================================== diff --git a/ext/standard/tests/streams/bug51056.phpt b/ext/standard/tests/streams/bug51056.phpt new file mode 100644 index 00000000000..2196376b58b --- /dev/null +++ b/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-- +run($clientCode, $serverCode); + +?> +--EXPECT-- +fread read 8 bytes +fread read 256 bytes +fread read 45 bytes +fread read 8 bytes +fread read 0 bytes diff --git a/main/php_streams.h b/main/php_streams.h index 71ef26c9701..fd16901f7c7 100644 --- a/main/php_streams.h +++ b/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; diff --git a/main/streams/streams.c b/main/streams/streams.c index fd65896586e..ef2a084545a 100644 --- a/main/streams/streams.c +++ b/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 diff --git a/main/streams/xp_socket.c b/main/streams/xp_socket.c index be41d3dde78..3a4beca9f07 100644 --- a/main/streams/xp_socket.c +++ b/main/streams/xp_socket.c @@ -120,10 +120,10 @@ retry: 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; @@ -131,10 +131,16 @@ static void php_sock_stream_wait_for_data(php_stream *stream, php_netstream_data 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)) {