From e1c712eb7c4cab6ca11607d45f8eec4d27e664d5 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Mon, 30 Jan 2023 22:21:11 +0100 Subject: [PATCH] Implement GH-9673: $length argument for fpassthru This implements an optional $length argument to fpassthru, SplFileObject::fpassthru and gzpassthru for partially copying the file to the output. The behaviour of this new argument has the same behaviour as the $length argument of stream_copy_to_stream. Internally, a new macro and function is introduced: _php_stream_passthru_with_length. _php_stream_passthru now calls the new function to perform its tasks without introducing a BC break internally. This unfortunately has one BC break: classes overriding SplFileObject must update the method signature of fpassthru to add the new argument. --- UPGRADING | 8 ++ UPGRADING.INTERNALS | 5 + ext/spl/spl_directory.c | 14 ++- ext/spl/spl_directory.stub.php | 2 +- ext/spl/spl_directory_arginfo.h | 6 +- .../SplFileObject_fpassthru_with_length.phpt | 33 ++++++ ext/spl/tests/gh8318.phpt | 2 +- ext/standard/basic_functions.stub.php | 2 +- ext/standard/basic_functions_arginfo.h | 3 +- ext/standard/file.c | 12 +- .../tests/file/fpassthru_with_length.phpt | 109 ++++++++++++++++++ ext/zlib/tests/gzpassthru_basic.phpt | 17 +++ ext/zlib/zlib.stub.php | 2 +- ext/zlib/zlib_arginfo.h | 3 +- main/php_streams.h | 4 +- main/streams/streams.c | 15 ++- 16 files changed, 220 insertions(+), 17 deletions(-) create mode 100644 ext/spl/tests/SplFileObject/SplFileObject_fpassthru_with_length.phpt create mode 100644 ext/standard/tests/file/fpassthru_with_length.phpt diff --git a/UPGRADING b/UPGRADING index 589692aa1e446..9b521062c41fe 100644 --- a/UPGRADING +++ b/UPGRADING @@ -64,6 +64,11 @@ PHP 8.3 UPGRADE NOTES casing only applies to MB_CASE_LOWER and MB_CASE_TITLE modes, not to MB_CASE_LOWER_SIMPLE and MB_CASE_TITLE_SIMPLE. (Alex Dowad) +- Spl: + . SplFileObject::fpassthru now has an optional $length parameter which allows the user + to specify how many bytes at most may be copied to the output. The behaviour of this + parameter is identical to how the $length parameter of stream_copy_to_stream behaves. + - Standard: . E_NOTICEs emitted by unserialized() have been promoted to E_WARNING. RFC: https://wiki.php.net/rfc/improve_unserialize_error_handling @@ -73,6 +78,9 @@ PHP 8.3 UPGRADE NOTES . strtok() raises a warning in the case token is not provided when starting tokenization. . password_hash() will now chain the underlying Random\RandomException as the ValueError’s $previous Exception when salt generation fails. + . fpassthru and gzpassthru now has an optional $length parameter which allows the user + to specify how many bytes at most may be copied to the output. The behaviour of this + parameter is identical to how the $length parameter of stream_copy_to_stream behaves. ======================================== 6. New Functions diff --git a/UPGRADING.INTERNALS b/UPGRADING.INTERNALS index d55c3cfe05816..65435118270c1 100644 --- a/UPGRADING.INTERNALS +++ b/UPGRADING.INTERNALS @@ -30,6 +30,11 @@ PHP 8.3 INTERNALS UPGRADE NOTES for C99 features have been removed and therefore macro definitions from php_config.h have disappeared. Do not use those feature macros. +* A new _php_stream_passthru_with_length() function has been added that takes + a stream and a length parameter. This length parameter indicates how many + bytes at most may be written to the output. _php_stream_passthru() is now + a wrapper around _php_stream_passthru_with_length() and still behaves the + same as in the old implementation. ======================== 2. Build system changes diff --git a/ext/spl/spl_directory.c b/ext/spl/spl_directory.c index d1dc2e2027495..5152870a16338 100644 --- a/ext/spl/spl_directory.c +++ b/ext/spl/spl_directory.c @@ -2552,15 +2552,23 @@ PHP_METHOD(SplFileObject, fgetc) /* {{{ Output all remaining data from a file pointer */ PHP_METHOD(SplFileObject, fpassthru) { + zend_long length; + bool length_is_null = true; + spl_filesystem_object *intern = spl_filesystem_from_obj(Z_OBJ_P(ZEND_THIS)); - if (zend_parse_parameters_none() == FAILURE) { - RETURN_THROWS(); + ZEND_PARSE_PARAMETERS_START(0, 1) + Z_PARAM_OPTIONAL + Z_PARAM_LONG_OR_NULL(length, length_is_null) + ZEND_PARSE_PARAMETERS_END(); + + if (length_is_null) { + length = PHP_STREAM_COPY_ALL; } CHECK_SPL_FILE_OBJECT_IS_INITIALIZED(intern); - RETURN_LONG(php_stream_passthru(intern->u.file.stream)); + RETURN_LONG(php_stream_passthru_with_length(intern->u.file.stream, length)); } /* }}} */ /* {{{ Implements a mostly ANSI compatible fscanf() */ diff --git a/ext/spl/spl_directory.stub.php b/ext/spl/spl_directory.stub.php index d11c9b50daf9d..5d0f344e7c99c 100644 --- a/ext/spl/spl_directory.stub.php +++ b/ext/spl/spl_directory.stub.php @@ -328,7 +328,7 @@ public function fseek(int $offset, int $whence = SEEK_SET): int {} public function fgetc(): string|false {} /** @tentative-return-type */ - public function fpassthru(): int {} + public function fpassthru(?int $length = null): int {} /** @tentative-return-type */ public function fscanf(string $format, mixed &...$vars): array|int|null {} diff --git a/ext/spl/spl_directory_arginfo.h b/ext/spl/spl_directory_arginfo.h index 99a6e0b854421..2e5d114a2809d 100644 --- a/ext/spl/spl_directory_arginfo.h +++ b/ext/spl/spl_directory_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: 7e67d07b6079c39a091e91dfddfbe7170067955e */ + * Stub hash: 5d814a046c92e9a4f104e15f17a5203d0c814828 */ ZEND_BEGIN_ARG_INFO_EX(arginfo_class_SplFileInfo___construct, 0, 0, 1) ZEND_ARG_TYPE_INFO(0, filename, IS_STRING, 0) @@ -219,7 +219,9 @@ ZEND_END_ARG_INFO() #define arginfo_class_SplFileObject_fgetc arginfo_class_SplFileInfo_getType -#define arginfo_class_SplFileObject_fpassthru arginfo_class_FilesystemIterator_getFlags +ZEND_BEGIN_ARG_WITH_TENTATIVE_RETURN_TYPE_INFO_EX(arginfo_class_SplFileObject_fpassthru, 0, 0, IS_LONG, 0) + ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, length, IS_LONG, 1, "null") +ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_WITH_TENTATIVE_RETURN_TYPE_MASK_EX(arginfo_class_SplFileObject_fscanf, 0, 1, MAY_BE_ARRAY|MAY_BE_LONG|MAY_BE_NULL) ZEND_ARG_TYPE_INFO(0, format, IS_STRING, 0) diff --git a/ext/spl/tests/SplFileObject/SplFileObject_fpassthru_with_length.phpt b/ext/spl/tests/SplFileObject/SplFileObject_fpassthru_with_length.phpt new file mode 100644 index 0000000000000..e2e0d1e096e5b --- /dev/null +++ b/ext/spl/tests/SplFileObject/SplFileObject_fpassthru_with_length.phpt @@ -0,0 +1,33 @@ +--TEST-- +SplFileObject::fpassthru function - functionality test with length +--FILE-- +fpassthru(1); +echo "\n"; +$read[] = $obj->fpassthru(10); +echo "\n"; +$read[] = $obj->fpassthru(0); +echo "\n"; +$read[] = $obj->fpassthru(-10000); +echo "\n"; +print_r($read); +?> +--EXPECT-- +f +irst,secon + +d,third +1,2,3 +4,5,6 +7,8,9 +0,0,0 + +Array +( + [0] => 1 + [1] => 10 + [2] => 0 + [3] => 32 +) diff --git a/ext/spl/tests/gh8318.phpt b/ext/spl/tests/gh8318.phpt index 6c5f6483d34dd..d81093b49e49c 100644 --- a/ext/spl/tests/gh8318.phpt +++ b/ext/spl/tests/gh8318.phpt @@ -8,7 +8,7 @@ class bug8318 extends \SplFileObject { } - public function fpassthru(): int + public function fpassthru(?int $length = null): int { return 0; } diff --git a/ext/standard/basic_functions.stub.php b/ext/standard/basic_functions.stub.php index ab56a8c0e8fbb..6c843b201aeb8 100755 --- a/ext/standard/basic_functions.stub.php +++ b/ext/standard/basic_functions.stub.php @@ -2650,7 +2650,7 @@ function fopen(string $filename, string $mode, bool $use_include_path = false, $ function fscanf($stream, string $format, mixed &...$vars): array|int|false|null {} /** @param resource $stream */ -function fpassthru($stream): int {} +function fpassthru($stream, ?int $length = null): int {} /** @param resource $stream */ function ftruncate($stream, int $size): bool {} diff --git a/ext/standard/basic_functions_arginfo.h b/ext/standard/basic_functions_arginfo.h index 077f9876df7c2..cc58f34acd24e 100644 --- a/ext/standard/basic_functions_arginfo.h +++ b/ext/standard/basic_functions_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: 39d455982dfdea9d0b9b646bc207b05f7108d1b2 */ + * Stub hash: 4cf5f26c059f2684eb32deb6b9387887274f62ef */ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_set_time_limit, 0, 1, _IS_BOOL, 0) ZEND_ARG_TYPE_INFO(0, seconds, IS_LONG, 0) @@ -1252,6 +1252,7 @@ ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_fpassthru, 0, 1, IS_LONG, 0) ZEND_ARG_INFO(0, stream) + ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, length, IS_LONG, 1, "null") ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_ftruncate, 0, 2, _IS_BOOL, 0) diff --git a/ext/standard/file.c b/ext/standard/file.c index 345536624d8fc..f7680d04a7b3c 100644 --- a/ext/standard/file.c +++ b/ext/standard/file.c @@ -1253,15 +1253,23 @@ PHPAPI PHP_FUNCTION(fpassthru) { zval *res; size_t size; + zend_long length; + bool length_is_null = true; php_stream *stream; - ZEND_PARSE_PARAMETERS_START(1, 1) + ZEND_PARSE_PARAMETERS_START(1, 2) Z_PARAM_RESOURCE(res) + Z_PARAM_OPTIONAL + Z_PARAM_LONG_OR_NULL(length, length_is_null) ZEND_PARSE_PARAMETERS_END(); + if (length_is_null) { + length = PHP_STREAM_COPY_ALL; + } + PHP_STREAM_FROM_ZVAL(stream, res); - size = php_stream_passthru(stream); + size = php_stream_passthru_with_length(stream, length); RETURN_LONG(size); } /* }}} */ diff --git a/ext/standard/tests/file/fpassthru_with_length.phpt b/ext/standard/tests/file/fpassthru_with_length.phpt new file mode 100644 index 0000000000000..c01409c7af68b --- /dev/null +++ b/ext/standard/tests/file/fpassthru_with_length.phpt @@ -0,0 +1,109 @@ +--TEST-- +Test fpassthru() function with length +--FILE-- + +--EXPECT-- +Test consecutive reads +1 +234 + +5 +67890abcdefghijklmnopqrstuvwxyz +Array +( + [0] => 1 + [1] => 3 + [2] => 0 + [3] => 1 + [4] => 31 +) +Read full file after rewind +1234567890abcdefghijklmnopqrstuvwxyz +Test reading at file offsets +abcde +v +xyz + +Array +( + [0] => 5 + [1] => 1 + [2] => 3 + [3] => 0 +) +Test reading negative lengths +1234567890abcdefghijklmnopqrstuvwxyz +1234567890abcdefghijklmnopqrstuvwxyz +1234567890abcdefghijklmnopqrstuvwxyz +1234567890abcdefghijklmnopqrstuvwxyz +Array +( + [0] => 36 + [1] => 36 + [2] => 36 + [3] => 36 +) + +--CLEAN-- + \ No newline at end of file diff --git a/ext/zlib/tests/gzpassthru_basic.phpt b/ext/zlib/tests/gzpassthru_basic.phpt index 11a57301c0a3f..4c5f18e13cdb2 100644 --- a/ext/zlib/tests/gzpassthru_basic.phpt +++ b/ext/zlib/tests/gzpassthru_basic.phpt @@ -12,6 +12,13 @@ $f = __DIR__."/004.txt.gz"; $h = gzopen($f, 'r'); var_dump(gzpassthru($h)); var_dump(gzpassthru($h)); +rewind($h); +$result = gzpassthru($h, 10); +echo "\n"; +var_dump($result); +$result = gzpassthru($h); +echo "\n"; +var_dump($result); gzclose($h); ?> @@ -24,3 +31,13 @@ as it turns around and I know that it descends down on me int(176) int(0) +When you'r +int(10) +e taught through feelings +Destiny flying high above +all I know is that you can realize it +Destiny who cares +as it turns around +and I know that it descends down on me + +int(166) diff --git a/ext/zlib/zlib.stub.php b/ext/zlib/zlib.stub.php index 1083564f76505..0a6e970d62431 100644 --- a/ext/zlib/zlib.stub.php +++ b/ext/zlib/zlib.stub.php @@ -244,7 +244,7 @@ function gzgetc($stream): string|false {} * @param resource $stream * @alias fpassthru */ -function gzpassthru($stream): int {} +function gzpassthru($stream, ?int $length = null): int {} /** * @param resource $stream diff --git a/ext/zlib/zlib_arginfo.h b/ext/zlib/zlib_arginfo.h index 743662fd036bd..f0ea76b6cb137 100644 --- a/ext/zlib/zlib_arginfo.h +++ b/ext/zlib/zlib_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: 3660ad3239f93c84b6909c36ddfcc92dd0773c70 */ + * Stub hash: 73dba8ccef5a5d331f3fa0be1180c2fd6079ca9d */ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_ob_gzhandler, 0, 2, MAY_BE_STRING|MAY_BE_FALSE) ZEND_ARG_TYPE_INFO(0, data, IS_STRING, 0) @@ -82,6 +82,7 @@ ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_gzpassthru, 0, 1, IS_LONG, 0) ZEND_ARG_INFO(0, stream) + ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, length, IS_LONG, 1, "null") ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_gzseek, 0, 2, IS_LONG, 0) diff --git a/main/php_streams.h b/main/php_streams.h index 0661491db98fe..9f22dacc94e16 100644 --- a/main/php_streams.h +++ b/main/php_streams.h @@ -480,8 +480,10 @@ PHPAPI zend_string *_php_stream_copy_to_mem(php_stream *src, size_t maxlen, int #define php_stream_copy_to_mem(src, maxlen, persistent) _php_stream_copy_to_mem((src), (maxlen), (persistent) STREAMS_CC) /* output all data from a stream */ -PHPAPI ssize_t _php_stream_passthru(php_stream * src STREAMS_DC); +PHPAPI ssize_t _php_stream_passthru(php_stream *src STREAMS_DC); +PHPAPI ssize_t _php_stream_passthru_with_length(php_stream *src, size_t length STREAMS_DC); #define php_stream_passthru(stream) _php_stream_passthru((stream) STREAMS_CC) +#define php_stream_passthru_with_length(stream, length) _php_stream_passthru_with_length((stream), (length) STREAMS_CC) END_EXTERN_C() #include "streams/php_stream_transport.h" diff --git a/main/streams/streams.c b/main/streams/streams.c index 1231dbebd63ae..8a2b09101ccad 100644 --- a/main/streams/streams.c +++ b/main/streams/streams.c @@ -1419,17 +1419,22 @@ PHPAPI int _php_stream_truncate_set_size(php_stream *stream, size_t newsize) return php_stream_set_option(stream, PHP_STREAM_OPTION_TRUNCATE_API, PHP_STREAM_TRUNCATE_SET_SIZE, &newsize); } -PHPAPI ssize_t _php_stream_passthru(php_stream * stream STREAMS_DC) +PHPAPI ssize_t _php_stream_passthru_with_length(php_stream *stream, size_t length STREAMS_DC) { size_t bcount = 0; char buf[8192]; ssize_t b; + if (UNEXPECTED(length == 0)) { + /* Prevent misinterpretation issue with PHP_STREAM_MMAP_ALL */ + return 0; + } + if (php_stream_mmap_possible(stream)) { char *p; size_t mapped; - p = php_stream_mmap_range(stream, php_stream_tell(stream), PHP_STREAM_MMAP_ALL, PHP_STREAM_MAP_MODE_SHARED_READONLY, &mapped); + p = php_stream_mmap_range(stream, php_stream_tell(stream), length == PHP_STREAM_COPY_ALL ? PHP_STREAM_MMAP_ALL : length, PHP_STREAM_MAP_MODE_SHARED_READONLY, &mapped); if (p) { do { @@ -1445,7 +1450,7 @@ PHPAPI ssize_t _php_stream_passthru(php_stream * stream STREAMS_DC) } } - while ((b = php_stream_read(stream, buf, sizeof(buf))) > 0) { + while ((b = php_stream_read(stream, buf, MIN(length - bcount, sizeof(buf)))) > 0) { PHPWRITE(buf, b); bcount += b; } @@ -1457,6 +1462,10 @@ PHPAPI ssize_t _php_stream_passthru(php_stream * stream STREAMS_DC) return bcount; } +PHPAPI ssize_t _php_stream_passthru(php_stream * stream STREAMS_DC) +{ + return _php_stream_passthru_with_length(stream, PHP_STREAM_COPY_ALL STREAMS_CC); +} PHPAPI zend_string *_php_stream_copy_to_mem(php_stream *src, size_t maxlen, int persistent STREAMS_DC) {