From 49fbbea2ea5fe1f6bab0719c95a46a119ea3d91f Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Sat, 10 Jun 2023 18:22:26 +0100 Subject: [PATCH] Fix GH-10406: fgets on a redis socket connection fails on PHP 8.3 This is an alternative implementation for GH-10406 that resets the has_buffered_data flag after finishing stream read so it does not impact other ops->read use like for example php_stream_get_line. Closes GH-11421 --- NEWS | 2 ++ ext/standard/tests/streams/gh11418.phpt | 36 +++++++++++++++++++++++++ main/php_streams.h | 4 ++- main/streams/streams.c | 20 +++++++------- main/streams/xp_socket.c | 2 +- 5 files changed, 53 insertions(+), 11 deletions(-) create mode 100644 ext/standard/tests/streams/gh11418.phpt diff --git a/NEWS b/NEWS index c77f990d4a5..b2415262102 100644 --- a/NEWS +++ b/NEWS @@ -15,6 +15,8 @@ PHP NEWS - Streams: . Implement GH-8641 (STREAM_NOTIFY_COMPLETED over HTTP never emitted). (nielsdos, Jakub Zelenka) + . Fix bug GH-10406 (fgets on a redis socket connection fails on PHP 8.3). + (Jakub Zelenka) 08 Jun 2023, PHP 8.3.0alpha1 diff --git a/ext/standard/tests/streams/gh11418.phpt b/ext/standard/tests/streams/gh11418.phpt new file mode 100644 index 00000000000..99f70ff4c6b --- /dev/null +++ b/ext/standard/tests/streams/gh11418.phpt @@ -0,0 +1,36 @@ +--TEST-- +GH-11418: fgets on a redis socket connection fails on PHP 8.3 +--FILE-- +run($clientCode, $serverCode); + +?> +--EXPECT-- +Hi Hello World diff --git a/main/php_streams.h b/main/php_streams.h index 13f8fe827b0..5acb94f3042 100644 --- a/main/php_streams.h +++ b/main/php_streams.h @@ -211,6 +211,9 @@ struct _php_stream { * PHP_STREAM_FCLOSE_XXX as appropriate */ uint8_t fclose_stdiocast:2; + /* flag to mark whether the stream has buffered data */ + uint8_t has_buffered_data:1; + char mode[16]; /* "rwb" etc. ala stdio */ uint32_t flags; /* PHP_STREAM_FLAG_XXX */ @@ -227,7 +230,6 @@ 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 14b534c998a..2a5178e2942 100644 --- a/main/streams/streams.c +++ b/main/streams/streams.c @@ -694,8 +694,7 @@ out_is_eof: PHPAPI ssize_t _php_stream_read(php_stream *stream, char *buf, size_t size) { - ssize_t toread = 0; - stream->didread = 0; + ssize_t toread = 0, didread = 0; while (size > 0) { @@ -714,7 +713,8 @@ PHPAPI ssize_t _php_stream_read(php_stream *stream, char *buf, size_t size) stream->readpos += toread; size -= toread; buf += toread; - stream->didread += toread; + didread += toread; + stream->has_buffered_data = 1; } /* ignore eof here; the underlying state might have changed */ @@ -727,14 +727,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 (stream->didread == 0) { + if (didread == 0) { return toread; } break; } } else { if (php_stream_fill_read_buffer(stream, size) != SUCCESS) { - if (stream->didread == 0) { + if (didread == 0) { return -1; } break; @@ -751,9 +751,10 @@ PHPAPI ssize_t _php_stream_read(php_stream *stream, char *buf, size_t size) } } if (toread > 0) { - stream->didread += toread; + didread += toread; buf += toread; size -= toread; + stream->has_buffered_data = 1; } else { /* EOF, or temporary end of data (for non-blocking mode). */ break; @@ -767,11 +768,12 @@ PHPAPI ssize_t _php_stream_read(php_stream *stream, char *buf, size_t size) } } - if (stream->didread > 0) { - stream->position += stream->didread; + if (didread > 0) { + stream->position += didread; + stream->has_buffered_data = 0; } - return stream->didread; + return 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 8f0a87b9980..6c770d77aed 100644 --- a/main/streams/xp_socket.c +++ b/main/streams/xp_socket.c @@ -168,7 +168,7 @@ static ssize_t php_sockop_read(php_stream *stream, char *buf, size_t count) /* Special handling for blocking read. */ if (sock->is_blocked) { /* Find out if there is any data buffered from the previous read. */ - bool has_buffered_data = stream->didread > 0; + bool has_buffered_data = stream->has_buffered_data; /* 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);