diff --git a/apps/dav/lib/Connector/Sabre/File.php b/apps/dav/lib/Connector/Sabre/File.php index 64e8faa5c36..109440eea4c 100644 --- a/apps/dav/lib/Connector/Sabre/File.php +++ b/apps/dav/lib/Connector/Sabre/File.php @@ -236,7 +236,13 @@ class File extends Node implements IFile { // because we have no clue about the cause we can only throw back a 500/Internal Server Error throw new Exception($this->l10n->t('Could not write file contents')); } - [$count, $result] = Files::streamCopy($data, $target, true); + $count = stream_copy_to_stream($data, $target); + if ($count === false) { + $result = false; + $count = 0; + } else { + $result = true; + } fclose($target); } if ($result === false && $expected !== null) { diff --git a/apps/files_versions/lib/Storage.php b/apps/files_versions/lib/Storage.php index ca3a130d9b3..fe9f91534fa 100644 --- a/apps/files_versions/lib/Storage.php +++ b/apps/files_versions/lib/Storage.php @@ -433,7 +433,10 @@ class Storage { $target = $storage2->fopen($internalPath2, 'w'); $result = $target !== false; if ($result) { - [, $result] = Files::streamCopy($source, $target, true); + $result = stream_copy_to_stream($source, $target); + if ($result !== false) { + $result = true; + } } // explicit check as S3 library closes streams already if (is_resource($target)) { diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index 4faa60b4a1b..634b83f8731 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -557,12 +557,6 @@ - - - - - - @@ -1982,14 +1976,12 @@ - - diff --git a/lib/private/Files/Storage/Common.php b/lib/private/Files/Storage/Common.php index e19f8812e9a..d58042b042c 100644 --- a/lib/private/Files/Storage/Common.php +++ b/lib/private/Files/Storage/Common.php @@ -212,7 +212,10 @@ abstract class Common implements Storage, ILockingStorage, IWriteStreamStorage, } else { $sourceStream = $this->fopen($source, 'r'); $targetStream = $this->fopen($target, 'w'); - [, $result] = Files::streamCopy($sourceStream, $targetStream, true); + $result = stream_copy_to_stream($sourceStream, $targetStream); + if ($result !== false) { + $result = true; + } if (!$result) { Server::get(LoggerInterface::class)->warning("Failed to write data while copying $source to $target"); } @@ -743,8 +746,8 @@ abstract class Common implements Storage, ILockingStorage, IWriteStreamStorage, throw new GenericFileException("Failed to open $path for writing"); } try { - [$count, $result] = Files::streamCopy($stream, $target, true); - if (!$result) { + $count = stream_copy_to_stream($stream, $target); + if ($count === false) { throw new GenericFileException('Failed to copy stream'); } } finally { diff --git a/lib/private/Files/Storage/LocalTempFileTrait.php b/lib/private/Files/Storage/LocalTempFileTrait.php index 5824189e3d8..805a67084cc 100644 --- a/lib/private/Files/Storage/LocalTempFileTrait.php +++ b/lib/private/Files/Storage/LocalTempFileTrait.php @@ -7,7 +7,6 @@ */ namespace OC\Files\Storage; -use OCP\Files; use OCP\ITempManager; use OCP\Server; @@ -49,8 +48,12 @@ trait LocalTempFileTrait { } $tmpFile = Server::get(ITempManager::class)->getTemporaryFile($extension); $target = fopen($tmpFile, 'w'); - Files::streamCopy($source, $target); + $result = stream_copy_to_stream($source, $target); fclose($target); + if ($result === false) { + return false; + } + return $tmpFile; } } diff --git a/lib/private/Files/Storage/Wrapper/Encryption.php b/lib/private/Files/Storage/Wrapper/Encryption.php index 9673ba9575a..0af619cc232 100644 --- a/lib/private/Files/Storage/Wrapper/Encryption.php +++ b/lib/private/Files/Storage/Wrapper/Encryption.php @@ -693,7 +693,10 @@ class Encryption extends Wrapper { if ($source === false || $target === false) { $result = false; } else { - [, $result] = Files::streamCopy($source, $target, true); + $result = stream_copy_to_stream($source, $target); + if ($result !== false) { + $result = true; + } } } finally { if ($source !== false) { @@ -715,7 +718,7 @@ class Encryption extends Wrapper { $this->getCache()->remove($targetInternalPath); } } - return (bool)$result; + return $result; } public function getLocalFile(string $path): string|false { @@ -915,7 +918,13 @@ class Encryption extends Wrapper { if ($target === false) { throw new GenericFileException("Failed to open $path for writing"); } - [$count, $result] = Files::streamCopy($stream, $target, true); + $count = stream_copy_to_stream($stream, $target); + if ($count === false) { + $result = false; + $count = 0; + } else { + $result = true; + } fclose($stream); fclose($target); diff --git a/lib/private/Files/Storage/Wrapper/Jail.php b/lib/private/Files/Storage/Wrapper/Jail.php index 125f5469f92..ebde93328d9 100644 --- a/lib/private/Files/Storage/Wrapper/Jail.php +++ b/lib/private/Files/Storage/Wrapper/Jail.php @@ -11,10 +11,10 @@ use OC\Files\Cache\Wrapper\CacheJail; use OC\Files\Cache\Wrapper\JailPropagator; use OC\Files\Cache\Wrapper\JailWatcher; use OC\Files\Filesystem; -use OCP\Files; use OCP\Files\Cache\ICache; use OCP\Files\Cache\IPropagator; use OCP\Files\Cache\IWatcher; +use OCP\Files\GenericFileException; use OCP\Files\Storage\IStorage; use OCP\Files\Storage\IWriteStreamStorage; use OCP\IDBConnection; @@ -258,9 +258,13 @@ class Jail extends Wrapper { return $storage->writeStream($this->getUnjailedPath($path), $stream, $size); } else { $target = $this->fopen($path, 'w'); - $count = Files::streamCopy($stream, $target); + $count = stream_copy_to_stream($stream, $target); fclose($stream); fclose($target); + if ($count === false) { + throw new GenericFileException('Failed to copy stream.'); + } + return $count; } } diff --git a/lib/private/Files/Storage/Wrapper/Wrapper.php b/lib/private/Files/Storage/Wrapper/Wrapper.php index 19fe9a6621e..3a95c754747 100644 --- a/lib/private/Files/Storage/Wrapper/Wrapper.php +++ b/lib/private/Files/Storage/Wrapper/Wrapper.php @@ -9,12 +9,12 @@ namespace OC\Files\Storage\Wrapper; use OC\Files\Storage\FailedStorage; use OC\Files\Storage\Storage; -use OCP\Files; use OCP\Files\Cache\ICache; use OCP\Files\Cache\IPropagator; use OCP\Files\Cache\IScanner; use OCP\Files\Cache\IUpdater; use OCP\Files\Cache\IWatcher; +use OCP\Files\GenericFileException; use OCP\Files\Storage\ILockingStorage; use OCP\Files\Storage\IStorage; use OCP\Files\Storage\IWriteStreamStorage; @@ -330,9 +330,13 @@ class Wrapper implements Storage, ILockingStorage, IWriteStreamStorage { return $storage->writeStream($path, $stream, $size); } else { $target = $this->fopen($path, 'w'); - $count = Files::streamCopy($stream, $target); + $count = stream_copy_to_stream($stream, $target); fclose($stream); fclose($target); + if ($count === false) { + throw new GenericFileException('Failed to copy stream.'); + } + return $count; } } diff --git a/lib/private/Files/View.php b/lib/private/Files/View.php index e7bf6d7aea9..0d20ab75067 100644 --- a/lib/private/Files/View.php +++ b/lib/private/Files/View.php @@ -639,7 +639,10 @@ class View { [$storage, $internalPath] = $this->resolvePath($path); $target = $storage->fopen($internalPath, 'w'); if ($target) { - [, $result] = Files::streamCopy($data, $target, true); + $result = stream_copy_to_stream($data, $target); + if ($result !== false) { + $result = true; + } fclose($target); fclose($data); diff --git a/lib/public/Files.php b/lib/public/Files.php index 77657c41a91..c7918c4cee6 100644 --- a/lib/public/Files.php +++ b/lib/public/Files.php @@ -83,47 +83,4 @@ class Files { public static function searchByMime($mimetype) { return \OC\Files\Filesystem::searchByMime($mimetype); } - - /** - * Copy the contents of one stream to another - * - * @template T of null|true - * @param resource $source - * @param resource $target - * @param T $includeResult - * @return int|array - * @psalm-return (T is true ? array{0: int, 1: bool} : int) - * @since 5.0.0 - * @since 32.0.0 added $includeResult parameter - * @deprecated 14.0.0 - */ - public static function streamCopy($source, $target, ?bool $includeResult = null) { - if (!$source || !$target) { - return $includeResult ? [0, false] : 0; - } - - $bufSize = 8192; - $count = 0; - $result = true; - while (!feof($source)) { - $buf = fread($source, $bufSize); - if ($buf === false) { - $result = false; - break; - } - - $bytesWritten = fwrite($target, $buf); - if ($bytesWritten !== false) { - $count += $bytesWritten; - } - - if ($bytesWritten === false - || ($bytesWritten < $bufSize && $bytesWritten < strlen($buf)) - ) { - $result = false; - break; - } - } - return $includeResult ? [$count, $result] : $count; - } } diff --git a/tests/lib/Archive/TestBase.php b/tests/lib/Archive/TestBase.php index f3728dacbad..24b518104d5 100644 --- a/tests/lib/Archive/TestBase.php +++ b/tests/lib/Archive/TestBase.php @@ -96,7 +96,8 @@ abstract class TestBase extends \Test\TestCase { $this->instance = $this->getNew(); $fh = $this->instance->getStream('lorem.txt', 'w'); $source = fopen($dir . '/lorem.txt', 'r'); - Files::streamCopy($source, $fh); + $result = stream_copy_to_stream($source, $fh); + $this->assertNotFalse($result); fclose($source); fclose($fh); $this->assertTrue($this->instance->fileExists('lorem.txt')); diff --git a/tests/lib/Files/Storage/Wrapper/QuotaTest.php b/tests/lib/Files/Storage/Wrapper/QuotaTest.php index 3d313666a93..ce43aa2736e 100644 --- a/tests/lib/Files/Storage/Wrapper/QuotaTest.php +++ b/tests/lib/Files/Storage/Wrapper/QuotaTest.php @@ -115,9 +115,8 @@ class QuotaTest extends \Test\Files\Storage\Storage { $instance = $this->getLimitedStorage(16); $inputStream = fopen('data://text/plain,foobarqwerty', 'r'); $outputStream = $instance->fopen('files/foo', 'w+'); - [$count, $result] = Files::streamCopy($inputStream, $outputStream, true); + $count = stream_copy_to_stream($inputStream, $outputStream); $this->assertEquals(12, $count); - $this->assertTrue($result); fclose($inputStream); fclose($outputStream); } @@ -126,9 +125,8 @@ class QuotaTest extends \Test\Files\Storage\Storage { $instance = $this->getLimitedStorage(9); $inputStream = fopen('data://text/plain,foobarqwerty', 'r'); $outputStream = $instance->fopen('files/foo', 'w+'); - [$count, $result] = Files::streamCopy($inputStream, $outputStream, true); - $this->assertEquals(9, $count); - $this->assertFalse($result); + $count = stream_copy_to_stream($inputStream, $outputStream); + $this->assertFalse($count); fclose($inputStream); fclose($outputStream); } diff --git a/tests/lib/FilesTest.php b/tests/lib/FilesTest.php index 5b1bc59d2d9..2ba6ce2666b 100644 --- a/tests/lib/FilesTest.php +++ b/tests/lib/FilesTest.php @@ -39,36 +39,4 @@ class FilesTest extends TestCase { Files::rmdirr($baseDir); $this->assertFalse(file_exists($baseDir)); } - - #[\PHPUnit\Framework\Attributes\DataProvider('streamCopyDataProvider')] - public function testStreamCopy($expectedCount, $expectedResult, $source, $target): void { - if (is_string($source)) { - $source = fopen($source, 'r'); - } - if (is_string($target)) { - $target = fopen($target, 'w'); - } - - [$count, $result] = Files::streamCopy($source, $target, true); - - if (is_resource($source)) { - fclose($source); - } - if (is_resource($target)) { - fclose($target); - } - - $this->assertSame($expectedCount, $count); - $this->assertSame($expectedResult, $result); - } - - - public static function streamCopyDataProvider(): array { - return [ - [0, false, false, false], - [0, false, \OC::$SERVERROOT . '/tests/data/lorem.txt', false], - [filesize(\OC::$SERVERROOT . '/tests/data/lorem.txt'), true, \OC::$SERVERROOT . '/tests/data/lorem.txt', \OC::$SERVERROOT . '/tests/data/lorem-copy.txt'], - [3670, true, \OC::$SERVERROOT . '/tests/data/testimage.png', \OC::$SERVERROOT . '/tests/data/testimage-copy.png'], - ]; - } }