Fix #81659: stream_get_contents() may unnecessarily overallocate

Since we're going to read from the current stream position anyway, the
`max_len` should be the size of the file minus the current position
(still catering to potentially filtered streams).  We must, however,
make sure to cater to the file position being beyond the actual file
size.

While we're at, we also fix the step size in the comment, which is 8K.

A further optimization could be done for unfiltered streams, thus
saving that step size, but 8K might not be worth it.

Closes GH-7693.
This commit is contained in:
Christoph M. Becker 2021-11-26 13:44:36 +01:00
parent c2d6d837ca
commit 31749aac62
No known key found for this signature in database
GPG Key ID: D66C9593118BCCB6
3 changed files with 28 additions and 2 deletions

2
NEWS
View File

@ -19,6 +19,8 @@ PHP NEWS
- Standard:
. Fixed bug #81618 (dns_get_record fails on FreeBSD for missing type).
(fsbruva)
. Fixed bug #81659 (stream_get_contents() may unnecessarily overallocate).
(cmb)
18 Nov 2021, PHP 7.4.26

View File

@ -0,0 +1,24 @@
--TEST--
Bug #81659 (stream_get_contents() may unnecessarily overallocate)
--FILE--
<?php
$stream = fopen(__DIR__ . "/81659.txt", "w+");
for ($i = 0; $i < 1024; $i++) {
fwrite($stream, str_repeat("*", 1024));
}
fseek($stream, 1023 * 1024);
$m0 = memory_get_peak_usage();
var_dump(strlen(stream_get_contents($stream)));
$m1 = memory_get_peak_usage();
var_dump($m1 < $m0 + 512 * 1024);
?>
--CLEAN--
<?php
@unlink(__DIR__ . "/81659.txt");
?>
--EXPECT--
int(1024)
bool(true)

View File

@ -1502,9 +1502,9 @@ PHPAPI zend_string *_php_stream_copy_to_mem(php_stream *src, size_t maxlen, int
* result may be inaccurate, as the filter may inflate or deflate the
* number of bytes that we can read. In order to avoid an upsize followed
* by a downsize of the buffer, overestimate by the step size (which is
* 2K). */
* 8K). */
if (php_stream_stat(src, &ssbuf) == 0 && ssbuf.sb.st_size > 0) {
max_len = ssbuf.sb.st_size + step;
max_len = MAX(ssbuf.sb.st_size - src->position, 0) + step;
} else {
max_len = step;
}