Skip to content

Commit b33fbbf

Browse files
committed
Fix GH-10031: [Stream] STREAM_NOTIFY_PROGRESS over HTTP emitted irregularly for last chunk of data
It's possible that the server already sent in more data than just the headers. Since the stream only accepts progress increments after the headers are processed, the already read data is never added to the process. We account for this by adjusting the progress counter by the difference of already read header data and the body. For the test: Co-authored-by: aetonsi <18366087+aetonsi@users.noreply.github.com> Closes GH-10492.
1 parent 05bd142 commit b33fbbf

File tree

3 files changed

+61
-0
lines changed

3 files changed

+61
-0
lines changed

NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ PHP NEWS
2424
source file). (ilutov)
2525

2626
- Streams:
27+
. Fixed bug GH-10031 ([Stream] STREAM_NOTIFY_PROGRESS over HTTP emitted
28+
irregularly for last chunk of data). (nielsdos)
2729
. Fixed bug GH-11175 (Stream Socket Timeout). (nielsdos)
2830
. Fixed bug GH-11177 (ASAN UndefinedBehaviorSanitizer when timeout = -1
2931
passed to stream_socket_accept/stream_socket_client). (nielsdos)

ext/standard/http_fopen_wrapper.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -955,6 +955,13 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
955955
if (transfer_encoding) {
956956
php_stream_filter_append(&stream->readfilters, transfer_encoding);
957957
}
958+
959+
/* It's possible that the server already sent in more data than just the headers.
960+
* We account for this by adjusting the progress counter by the difference of
961+
* already read header data and the body. */
962+
if (stream->writepos > stream->readpos) {
963+
php_stream_notify_progress_increment(context, stream->writepos - stream->readpos, 0);
964+
}
958965
}
959966

960967
return stream;
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
--TEST--
2+
GH-10031 ([Stream] STREAM_NOTIFY_PROGRESS over HTTP emitted irregularly for last chunk of data)
3+
--SKIPIF--
4+
<?php
5+
if (getenv("SKIP_SLOW_TESTS")) die("skip slow test");
6+
?>
7+
--INI--
8+
allow_url_fopen=1
9+
--CONFLICTS--
10+
server
11+
--FILE--
12+
<?php
13+
14+
$serverCode = <<<'CODE'
15+
$fsize = 1000;
16+
$chunksize = 99;
17+
$chunks = floor($fsize / $chunksize); // 10 chunks * 99 bytes
18+
$lastchunksize = $fsize - $chunksize * $chunks; // 1 chunk * 10 bytes
19+
20+
header("Content-Length: " . $fsize);
21+
flush();
22+
for ($chunk = 1; $chunk <= $chunks; $chunk++) {
23+
echo str_repeat('x', $chunksize);
24+
@ob_flush();
25+
usleep(50 * 1000);
26+
}
27+
28+
echo str_repeat('x', $lastchunksize);
29+
CODE;
30+
31+
include __DIR__."/../../../../sapi/cli/tests/php_cli_server.inc";
32+
php_cli_server_start($serverCode, null, []);
33+
34+
$context = stream_context_create(['http' => ['ignore_errors' => true,]]);
35+
$lastBytesTransferred = 0;
36+
stream_context_set_params($context, ['notification' => function ($code, $s, $m, $mc, $bytes_transferred, $bytes_max)
37+
use (&$lastBytesTransferred) {
38+
if ($code === STREAM_NOTIFY_FILE_SIZE_IS) echo "expected filesize=$bytes_max".PHP_EOL;
39+
$lastBytesTransferred = $bytes_transferred;
40+
@ob_flush();
41+
}]);
42+
43+
$get = file_get_contents("http://".PHP_CLI_SERVER_ADDRESS, false, $context);
44+
45+
echo "got filesize=" . strlen($get) . PHP_EOL;
46+
var_dump($lastBytesTransferred);
47+
48+
?>
49+
--EXPECT--
50+
expected filesize=1000
51+
got filesize=1000
52+
int(1000)

0 commit comments

Comments
 (0)