Skip to content

Commit d22d0e2

Browse files
committed
Implement GH-8641: STREAM_NOTIFY_COMPLETED over HTTP never emitted
This adds support for the completed event. Since the read handler could be entered twice towards the end of the stream we remember what the eof flag was before reading so we can emit the completed event when the flag changes to true. Closes GH-10505.
1 parent 30c5ae4 commit d22d0e2

File tree

5 files changed

+78
-7
lines changed

5 files changed

+78
-7
lines changed

NEWS

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@ PHP NEWS
1212
. Fix #77894 (DOMNode::C14N() very slow on generated DOMDocuments even after
1313
normalisation). (nielsdos)
1414

15+
- Streams:
16+
. Implement GH-8641 (STREAM_NOTIFY_COMPLETED over HTTP never emitted).
17+
(nielsdos, Jakub Zelenka)
18+
1519
08 Jun 2023, PHP 8.3.0alpha1
1620

1721
- CLI:

UPGRADING

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,10 @@ PHP 8.3 UPGRADE NOTES
7272
. posix_ttyname() now raises type warnings for integers following the usual ZPP semantics
7373
and value warnings for invalid file descriptor integers.
7474

75+
- Streams
76+
. Streams can now emit the STREAM_NOTIFY_COMPLETED notification. This was previously
77+
not implemented.
78+
7579
========================================
7680
3. Changes in SAPI modules
7781
========================================

ext/standard/tests/http/gh8641.phpt

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
--TEST--
2+
GH-8641 ([Stream] STREAM_NOTIFY_COMPLETED over HTTP never emitted)
3+
--SKIPIF--
4+
<?php require 'server.inc'; http_server_skipif(); ?>
5+
--INI--
6+
allow_url_fopen=1
7+
--FILE--
8+
<?php
9+
require 'server.inc';
10+
11+
function stream_notification_callback($notification_code, $severity, $message, $message_code, $bytes_transferred, $bytes_max)
12+
{
13+
if ($notification_code === STREAM_NOTIFY_COMPLETED) {
14+
echo $notification_code, ' ', $bytes_transferred, ' ', $bytes_max, PHP_EOL;
15+
}
16+
}
17+
18+
$ctx = stream_context_create();
19+
stream_context_set_params($ctx, array("notification" => "stream_notification_callback"));
20+
21+
$responses = array(
22+
"data://text/plain,HTTP/1.0 200 Ok\r\nContent-Length: 11\r\n\r\nHello world",
23+
);
24+
25+
['pid' => $pid, 'uri' => $uri] = http_server($responses, $output);
26+
27+
$f = file_get_contents($uri, 0, $ctx);
28+
29+
http_server_kill($pid);
30+
var_dump($f);
31+
?>
32+
--EXPECTF--
33+
8 11 11
34+
string(11) "Hello world"

main/streams/php_stream_context.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,10 @@ END_EXTERN_C()
9494
php_stream_notification_notify((context), PHP_STREAM_NOTIFY_PROGRESS, PHP_STREAM_NOTIFY_SEVERITY_INFO, \
9595
NULL, 0, (bsofar), (bmax), NULL); } } while(0)
9696

97+
#define php_stream_notify_completed(context) do { if ((context) && (context)->notifier) { \
98+
php_stream_notification_notify((context), PHP_STREAM_NOTIFY_COMPLETED, PHP_STREAM_NOTIFY_SEVERITY_INFO, \
99+
NULL, 0, (context)->notifier->progress, (context)->notifier->progress_max, NULL); } } while(0)
100+
97101
#define php_stream_notify_progress_init(context, sofar, bmax) do { if ((context) && (context)->notifier) { \
98102
(context)->notifier->progress = (sofar); \
99103
(context)->notifier->progress_max = (bmax); \

main/streams/streams.c

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -542,6 +542,9 @@ PHPAPI zend_result _php_stream_fill_read_buffer(php_stream *stream, size_t size)
542542
{
543543
/* allocate/fill the buffer */
544544

545+
zend_result retval;
546+
bool old_eof = stream->eof;
547+
545548
if (stream->readfilters.head) {
546549
size_t to_read_now = MIN(size, stream->chunk_size);
547550
char *chunk_buf;
@@ -562,7 +565,8 @@ PHPAPI zend_result _php_stream_fill_read_buffer(php_stream *stream, size_t size)
562565
justread = stream->ops->read(stream, chunk_buf, stream->chunk_size);
563566
if (justread < 0 && stream->writepos == stream->readpos) {
564567
efree(chunk_buf);
565-
return FAILURE;
568+
retval = FAILURE;
569+
goto out_check_eof;
566570
} else if (justread > 0) {
567571
bucket = php_stream_bucket_new(stream, chunk_buf, justread, 0, 0);
568572

@@ -633,7 +637,8 @@ PHPAPI zend_result _php_stream_fill_read_buffer(php_stream *stream, size_t size)
633637
* further reads should fail. */
634638
stream->eof = 1;
635639
efree(chunk_buf);
636-
return FAILURE;
640+
retval = FAILURE;
641+
goto out_is_eof;
637642
}
638643

639644
if (justread <= 0) {
@@ -643,7 +648,6 @@ PHPAPI zend_result _php_stream_fill_read_buffer(php_stream *stream, size_t size)
643648

644649
efree(chunk_buf);
645650
return SUCCESS;
646-
647651
} else {
648652
/* is there enough data in the buffer ? */
649653
if (stream->writepos - stream->readpos < (zend_off_t)size) {
@@ -670,12 +674,22 @@ PHPAPI zend_result _php_stream_fill_read_buffer(php_stream *stream, size_t size)
670674
stream->readbuflen - stream->writepos
671675
);
672676
if (justread < 0) {
673-
return FAILURE;
677+
retval = FAILURE;
678+
goto out_check_eof;
674679
}
675680
stream->writepos += justread;
681+
retval = SUCCESS;
682+
goto out_check_eof;
676683
}
677684
return SUCCESS;
678685
}
686+
687+
out_check_eof:
688+
if (old_eof != stream->eof) {
689+
out_is_eof:
690+
php_stream_notify_completed(PHP_STREAM_CONTEXT(stream));
691+
}
692+
return retval;
679693
}
680694

681695
PHPAPI ssize_t _php_stream_read(php_stream *stream, char *buf, size_t size)
@@ -1124,6 +1138,7 @@ PHPAPI zend_string *php_stream_get_record(php_stream *stream, size_t maxlen, con
11241138
static ssize_t _php_stream_write_buffer(php_stream *stream, const char *buf, size_t count)
11251139
{
11261140
ssize_t didwrite = 0;
1141+
ssize_t retval;
11271142

11281143
/* if we have a seekable stream we need to ensure that data is written at the
11291144
* current stream->position. This means invalidating the read buffer and then
@@ -1134,15 +1149,19 @@ static ssize_t _php_stream_write_buffer(php_stream *stream, const char *buf, siz
11341149
stream->ops->seek(stream, stream->position, SEEK_SET, &stream->position);
11351150
}
11361151

1152+
bool old_eof = stream->eof;
1153+
11371154
while (count > 0) {
11381155
ssize_t justwrote = stream->ops->write(stream, buf, count);
11391156
if (justwrote <= 0) {
11401157
/* If we already successfully wrote some bytes and a write error occurred
11411158
* later, report the successfully written bytes. */
11421159
if (didwrite == 0) {
1143-
return justwrote;
1160+
retval = justwrote;
1161+
goto out;
11441162
}
1145-
return didwrite;
1163+
retval = didwrite;
1164+
goto out;
11461165
}
11471166

11481167
buf += justwrote;
@@ -1151,7 +1170,13 @@ static ssize_t _php_stream_write_buffer(php_stream *stream, const char *buf, siz
11511170
stream->position += justwrote;
11521171
}
11531172

1154-
return didwrite;
1173+
retval = didwrite;
1174+
1175+
out:
1176+
if (old_eof != stream->eof) {
1177+
php_stream_notify_completed(PHP_STREAM_CONTEXT(stream));
1178+
}
1179+
return retval;
11551180
}
11561181

11571182
/* push some data through the write filter chain.

0 commit comments

Comments
 (0)