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.
php · Jan 30, 2023 · 7df8ef8 · 7df8ef8
1 parent ff84598
commit 7df8ef8
Show file tree

Hide file tree

Showing 16 changed files with 220 additions and 17 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
diff --git a/ext/spl/tests/SplFileObject/SplFileObject_fpassthru_with_length.phpt b/ext/spl/tests/SplFileObject/SplFileObject_fpassthru_with_length.phpt
@@ -0,0 +1,33 @@
+--TEST--
+SplFileObject::fpassthru function - functionality test with length
+--FILE--
+<?php
+$obj = New SplFileObject(__DIR__.'/SplFileObject_testinput.csv');
+$read = [];
+$read[] = $obj->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
@@ -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
@@ -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
diff --git 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
@@ -0,0 +1,109 @@
+--TEST--
+Test fpassthru() function with length
+--FILE--
+<?php
+$file_name = __DIR__."/passthru_with_length.tmp";
+$file_write = fopen($file_name, "w");
+fwrite($file_write, "1234567890abcdefghijklmnopqrstuvwxyz");
+fclose($file_write);
+
+$file_read = fopen($file_name, "r");
+
+echo "Test consecutive reads\n";
+$read = [];
+$read[] = fpassthru($file_read, 1);
+echo "\n";
+$read[] = fpassthru($file_read, 3);
+echo "\n";
+$read[] = fpassthru($file_read, 0);
+echo "\n";
+$read[] = fpassthru($file_read, 1);
+echo "\n";
+$read[] = fpassthru($file_read);
+echo "\n";
+print_r($read);
+
+echo "Read full file after rewind\n";
+rewind($file_read);
+fpassthru($file_read);
+echo "\n";
+
+echo "Test reading at file offsets\n";
+$read = [];
+fseek($file_read, 10);
+$read[] = fpassthru($file_read, 5);
+echo "\n";
+fseek($file_read, -5, SEEK_END);
+$read[] = fpassthru($file_read, 1);
+echo "\n";
+fseek($file_read, -3, SEEK_END);
+$read[] = fpassthru($file_read, 1000);
+echo "\n";
+$read[] = fpassthru($file_read);
+echo "\n";
+print_r($read);
+
+echo "Test reading negative lengths\n";
+$read = [];
+rewind($file_read);
+$read[] = fpassthru($file_read, -1);
+echo "\n";
+rewind($file_read);
+$read[] = fpassthru($file_read, -2);
+echo "\n";
+rewind($file_read);
+$read[] = fpassthru($file_read, -100);
+echo "\n";
+rewind($file_read);
+$read[] = fpassthru($file_read, PHP_INT_MIN);
+echo "\n";
+print_r($read);
+
+fclose($file_read);
+?>
+--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--
+<?php
+@unlink(__DIR__."/passthru_with_length.tmp");
+?>
diff --git 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
@@ -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
diff --git 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"