Skip to content

Commit 49fbbea

Browse files
committed
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
1 parent d22d0e2 commit 49fbbea

File tree

5 files changed

+53
-11
lines changed

5 files changed

+53
-11
lines changed

NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ PHP NEWS
1515
- Streams:
1616
. Implement GH-8641 (STREAM_NOTIFY_COMPLETED over HTTP never emitted).
1717
(nielsdos, Jakub Zelenka)
18+
. Fix bug GH-10406 (fgets on a redis socket connection fails on PHP 8.3).
19+
(Jakub Zelenka)
1820

1921
08 Jun 2023, PHP 8.3.0alpha1
2022

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
--TEST--
2+
GH-11418: fgets on a redis socket connection fails on PHP 8.3
3+
--FILE--
4+
<?php
5+
6+
$serverCode = <<<'CODE'
7+
$server = stream_socket_server('tcp://127.0.0.1:64324');
8+
phpt_notify();
9+
10+
$conn = stream_socket_accept($server);
11+
12+
fwrite($conn, "Hi Hello"); // 8 bytes
13+
usleep(50000);
14+
fwrite($conn, " World\n"); // 8 bytes
15+
16+
fclose($conn);
17+
fclose($server);
18+
CODE;
19+
20+
$clientCode = <<<'CODE'
21+
22+
phpt_wait();
23+
24+
$fp = fsockopen("tcp://127.0.0.1:64324");
25+
26+
echo fread($fp, 3);
27+
echo fgets($fp);
28+
29+
CODE;
30+
31+
include sprintf("%s/../../../openssl/tests/ServerClientTestCase.inc", __DIR__);
32+
ServerClientTestCase::getInstance()->run($clientCode, $serverCode);
33+
34+
?>
35+
--EXPECT--
36+
Hi Hello World

main/php_streams.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,9 @@ struct _php_stream {
211211
* PHP_STREAM_FCLOSE_XXX as appropriate */
212212
uint8_t fclose_stdiocast:2;
213213

214+
/* flag to mark whether the stream has buffered data */
215+
uint8_t has_buffered_data:1;
216+
214217
char mode[16]; /* "rwb" etc. ala stdio */
215218

216219
uint32_t flags; /* PHP_STREAM_FLAG_XXX */
@@ -227,7 +230,6 @@ struct _php_stream {
227230
size_t readbuflen;
228231
zend_off_t readpos;
229232
zend_off_t writepos;
230-
ssize_t didread;
231233

232234
/* how much data to read when filling buffer */
233235
size_t chunk_size;

main/streams/streams.c

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -694,8 +694,7 @@ PHPAPI zend_result _php_stream_fill_read_buffer(php_stream *stream, size_t size)
694694

695695
PHPAPI ssize_t _php_stream_read(php_stream *stream, char *buf, size_t size)
696696
{
697-
ssize_t toread = 0;
698-
stream->didread = 0;
697+
ssize_t toread = 0, didread = 0;
699698

700699
while (size > 0) {
701700

@@ -714,7 +713,8 @@ PHPAPI ssize_t _php_stream_read(php_stream *stream, char *buf, size_t size)
714713
stream->readpos += toread;
715714
size -= toread;
716715
buf += toread;
717-
stream->didread += toread;
716+
didread += toread;
717+
stream->has_buffered_data = 1;
718718
}
719719

720720
/* 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)
727727
if (toread < 0) {
728728
/* Report an error if the read failed and we did not read any data
729729
* before that. Otherwise return the data we did read. */
730-
if (stream->didread == 0) {
730+
if (didread == 0) {
731731
return toread;
732732
}
733733
break;
734734
}
735735
} else {
736736
if (php_stream_fill_read_buffer(stream, size) != SUCCESS) {
737-
if (stream->didread == 0) {
737+
if (didread == 0) {
738738
return -1;
739739
}
740740
break;
@@ -751,9 +751,10 @@ PHPAPI ssize_t _php_stream_read(php_stream *stream, char *buf, size_t size)
751751
}
752752
}
753753
if (toread > 0) {
754-
stream->didread += toread;
754+
didread += toread;
755755
buf += toread;
756756
size -= toread;
757+
stream->has_buffered_data = 1;
757758
} else {
758759
/* EOF, or temporary end of data (for non-blocking mode). */
759760
break;
@@ -767,11 +768,12 @@ PHPAPI ssize_t _php_stream_read(php_stream *stream, char *buf, size_t size)
767768
}
768769
}
769770

770-
if (stream->didread > 0) {
771-
stream->position += stream->didread;
771+
if (didread > 0) {
772+
stream->position += didread;
773+
stream->has_buffered_data = 0;
772774
}
773775

774-
return stream->didread;
776+
return didread;
775777
}
776778

777779
/* Like php_stream_read(), but reading into a zend_string buffer. This has some similarity

main/streams/xp_socket.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ static ssize_t php_sockop_read(php_stream *stream, char *buf, size_t count)
168168
/* Special handling for blocking read. */
169169
if (sock->is_blocked) {
170170
/* Find out if there is any data buffered from the previous read. */
171-
bool has_buffered_data = stream->didread > 0;
171+
bool has_buffered_data = stream->has_buffered_data;
172172
/* No need to wait if there is any data buffered or no timeout. */
173173
bool dont_wait = has_buffered_data ||
174174
(sock->timeout.tv_sec == 0 && sock->timeout.tv_usec == 0);

0 commit comments

Comments
 (0)