From 7b1a0441312124784b9d6efab72c8c496f8f7c69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 24 Feb 2022 12:03:36 +0100 Subject: [PATCH 1/4] Improve typing in OC\Archive classes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/private/Archive/Archive.php | 7 ++++--- lib/private/Archive/TAR.php | 31 +++++++++++++++++++++++++------ lib/private/Archive/ZIP.php | 30 +++++++++++++++++++++--------- 3 files changed, 50 insertions(+), 18 deletions(-) diff --git a/lib/private/Archive/Archive.php b/lib/private/Archive/Archive.php index 141db11f419..4f8100530ac 100644 --- a/lib/private/Archive/Archive.php +++ b/lib/private/Archive/Archive.php @@ -31,7 +31,7 @@ namespace OC\Archive; abstract class Archive { /** - * @param $source + * @param string $source */ abstract public function __construct($source); /** @@ -80,14 +80,14 @@ abstract class Archive { /** * get the content of a file * @param string $path - * @return string + * @return string|false */ abstract public function getFile($path); /** * extract a single file from the archive * @param string $path * @param string $dest - * @return bool + * @return bool success */ abstract public function extractFile($path, $dest); /** @@ -119,6 +119,7 @@ abstract class Archive { * add a folder and all its content * @param string $path * @param string $source + * @return void */ public function addRecursive($path, $source) { $dh = opendir($source); diff --git a/lib/private/Archive/TAR.php b/lib/private/Archive/TAR.php index a024ce84b2f..1b8f72fd6ce 100644 --- a/lib/private/Archive/TAR.php +++ b/lib/private/Archive/TAR.php @@ -39,13 +39,23 @@ class TAR extends Archive { public const GZIP = 1; public const BZIP = 2; - private $fileList; - private $cachedHeaders; + /** + * @var string[]|false + */ + private $fileList = false; + /** + * @var array|false + */ + private $cachedHeaders = false; /** - * @var \Archive_Tar tar + * @var \Archive_Tar */ private $tar = null; + + /** + * @var string + */ private $path; /** @@ -154,6 +164,7 @@ class TAR extends Archive { /** * @param string $file + * @return array|null */ private function getHeader($file) { if (!$this->cachedHeaders) { @@ -244,10 +255,15 @@ class TAR extends Archive { * get the content of a file * * @param string $path - * @return string + * @return string|false */ public function getFile($path) { - return $this->tar->extractInString($path); + $string = $this->tar->extractInString($path); + if (is_string($string)) { + return $string; + } else { + return false; + } } /** @@ -364,6 +380,9 @@ class TAR extends Archive { /** * write back temporary files + * @param string $tmpFile + * @param string $path + * @return void */ public function writeBack($tmpFile, $path) { $this->addFile($path, $tmpFile); @@ -373,7 +392,7 @@ class TAR extends Archive { /** * reopen the archive to ensure everything is written */ - private function reopen() { + private function reopen(): void { if ($this->tar) { $this->tar->_close(); $this->tar = null; diff --git a/lib/private/Archive/ZIP.php b/lib/private/Archive/ZIP.php index e058d476021..8c1b162206d 100644 --- a/lib/private/Archive/ZIP.php +++ b/lib/private/Archive/ZIP.php @@ -33,12 +33,17 @@ namespace OC\Archive; use Icewind\Streams\CallbackWrapper; use OCP\ILogger; +use Psr\Log\LoggerInterface; class ZIP extends Archive { /** * @var \ZipArchive zip */ - private $zip = null; + private $zip; + + /** + * @var string + */ private $path; /** @@ -49,7 +54,7 @@ class ZIP extends Archive { $this->zip = new \ZipArchive(); if ($this->zip->open($source, \ZipArchive::CREATE)) { } else { - \OCP\Util::writeLog('files_archive', 'Error while opening archive '.$source, ILogger::WARN); + \OC::$server->get(LoggerInterface::class)->warning('Error while opening archive '.$source, ['app' => 'files_archive']); } } /** @@ -82,12 +87,12 @@ class ZIP extends Archive { * rename a file or folder in the archive * @param string $source * @param string $dest - * @return boolean|null + * @return bool */ public function rename($source, $dest) { $source = $this->stripPath($source); $dest = $this->stripPath($dest); - $this->zip->renameName($source, $dest); + return $this->zip->renameName($source, $dest); } /** * get the uncompressed size of a file in the archive @@ -139,7 +144,7 @@ class ZIP extends Archive { /** * get the content of a file * @param string $path - * @return string + * @return string|false */ public function getFile($path) { return $this->zip->getFromName($path); @@ -148,11 +153,14 @@ class ZIP extends Archive { * extract a single file from the archive * @param string $path * @param string $dest - * @return boolean|null + * @return bool */ public function extractFile($path, $dest) { $fp = $this->zip->getStream($path); - file_put_contents($dest, $fp); + if ($fp === false) { + return false; + } + return file_put_contents($dest, $fp) !== false; } /** * extract the archive @@ -195,8 +203,9 @@ class ZIP extends Archive { //since we can't directly get a writable stream, //make a temp copy of the file and put it back //in the archive when the stream is closed - if (strrpos($path, '.') !== false) { - $ext = substr($path, strrpos($path, '.')); + $lastPoint = strrpos($path, '.'); + if ($lastPoint !== false) { + $ext = substr($path, $lastPoint); } else { $ext = ''; } @@ -213,6 +222,9 @@ class ZIP extends Archive { /** * write back temporary files + * @param string $tmpFile + * @param string $path + * @return void */ public function writeBack($tmpFile, $path) { $this->addFile($path, $tmpFile); From c9100e3d442a136b2e4888a5a1fcf62fdc04189b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 24 Feb 2022 12:25:04 +0100 Subject: [PATCH 2/4] Fix more typing in OC\Archive classes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/private/Archive/Archive.php | 4 ++-- lib/private/Archive/TAR.php | 8 ++++---- lib/private/Archive/ZIP.php | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/private/Archive/Archive.php b/lib/private/Archive/Archive.php index 4f8100530ac..31db51d59a1 100644 --- a/lib/private/Archive/Archive.php +++ b/lib/private/Archive/Archive.php @@ -57,13 +57,13 @@ abstract class Archive { /** * get the uncompressed size of a file in the archive * @param string $path - * @return int + * @return int|false */ abstract public function filesize($path); /** * get the last modified time of a file in the archive * @param string $path - * @return int + * @return int|false */ abstract public function mtime($path); /** diff --git a/lib/private/Archive/TAR.php b/lib/private/Archive/TAR.php index 1b8f72fd6ce..a3c2abb41bb 100644 --- a/lib/private/Archive/TAR.php +++ b/lib/private/Archive/TAR.php @@ -186,22 +186,22 @@ class TAR extends Archive { * get the uncompressed size of a file in the archive * * @param string $path - * @return int + * @return int|false */ public function filesize($path) { $stat = $this->getHeader($path); - return $stat['size']; + return $stat['size'] ?? false; } /** * get the last modified time of a file in the archive * * @param string $path - * @return int + * @return int|false */ public function mtime($path) { $stat = $this->getHeader($path); - return $stat['mtime']; + return $stat['mtime'] ?? false; } /** diff --git a/lib/private/Archive/ZIP.php b/lib/private/Archive/ZIP.php index 8c1b162206d..5dad747edf0 100644 --- a/lib/private/Archive/ZIP.php +++ b/lib/private/Archive/ZIP.php @@ -97,16 +97,16 @@ class ZIP extends Archive { /** * get the uncompressed size of a file in the archive * @param string $path - * @return int + * @return int|false */ public function filesize($path) { $stat = $this->zip->statName($path); - return $stat['size']; + return $stat['size'] ?? false; } /** * get the last modified time of a file in the archive * @param string $path - * @return int + * @return int|false */ public function mtime($path) { return filemtime($this->path); From 153eca8ef0ed347083f1f8ef274eb675a2e740be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 24 Feb 2022 12:34:32 +0100 Subject: [PATCH 3/4] Fix unused import in OC\Archive\ZIP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/private/Archive/ZIP.php | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/private/Archive/ZIP.php b/lib/private/Archive/ZIP.php index 5dad747edf0..40dfa3b41b6 100644 --- a/lib/private/Archive/ZIP.php +++ b/lib/private/Archive/ZIP.php @@ -32,7 +32,6 @@ namespace OC\Archive; use Icewind\Streams\CallbackWrapper; -use OCP\ILogger; use Psr\Log\LoggerInterface; class ZIP extends Archive { From 77644cb67444c81a873f67f86d314d688db8a462 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 24 Feb 2022 17:04:09 +0100 Subject: [PATCH 4/4] Move to strong typing in OC\Archive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/private/Archive/Archive.php | 72 ++++++++++++---------------- lib/private/Archive/TAR.php | 84 ++++++++------------------------- lib/private/Archive/ZIP.php | 77 ++++++++++++------------------ 3 files changed, 78 insertions(+), 155 deletions(-) diff --git a/lib/private/Archive/Archive.php b/lib/private/Archive/Archive.php index 31db51d59a1..cef306230fd 100644 --- a/lib/private/Archive/Archive.php +++ b/lib/private/Archive/Archive.php @@ -30,98 +30,84 @@ namespace OC\Archive; abstract class Archive { - /** - * @param string $source - */ - abstract public function __construct($source); + abstract public function __construct(string $source); + /** * add an empty folder to the archive - * @param string $path - * @return bool */ - abstract public function addFolder($path); + abstract public function addFolder(string $path): bool; + /** * add a file to the archive - * @param string $path * @param string $source either a local file or string data - * @return bool */ - abstract public function addFile($path, $source = ''); + abstract public function addFile(string $path, string $source = ''): bool; + /** * rename a file or folder in the archive - * @param string $source - * @param string $dest - * @return bool */ - abstract public function rename($source, $dest); + abstract public function rename(string $source, string $dest): bool; + /** * get the uncompressed size of a file in the archive - * @param string $path * @return int|false */ - abstract public function filesize($path); + abstract public function filesize(string $path); + /** * get the last modified time of a file in the archive - * @param string $path * @return int|false */ - abstract public function mtime($path); + abstract public function mtime(string $path); + /** * get the files in a folder * @param string $path * @return array */ - abstract public function getFolder($path); + abstract public function getFolder(string $path): array; + /** * get all files in the archive - * @return array */ - abstract public function getFiles(); + abstract public function getFiles(): array; + /** * get the content of a file - * @param string $path * @return string|false */ - abstract public function getFile($path); + abstract public function getFile(string $path); + /** * extract a single file from the archive - * @param string $path - * @param string $dest - * @return bool success */ - abstract public function extractFile($path, $dest); + abstract public function extractFile(string $path, string $dest): bool; + /** * extract the archive - * @param string $dest - * @return bool */ - abstract public function extract($dest); + abstract public function extract(string $dest): bool; + /** * check if a file or folder exists in the archive - * @param string $path - * @return bool */ - abstract public function fileExists($path); + abstract public function fileExists(string $path): bool; + /** * remove a file or folder from the archive - * @param string $path - * @return bool */ - abstract public function remove($path); + abstract public function remove(string $path): bool; + /** * get a file handler - * @param string $path - * @param string $mode * @return bool|resource */ - abstract public function getStream($path, $mode); + abstract public function getStream(string $path, string $mode); + /** * add a folder and all its content - * @param string $path - * @param string $source - * @return void */ - public function addRecursive($path, $source) { + public function addRecursive(string $path, string $source): void { $dh = opendir($source); if (is_resource($dh)) { $this->addFolder($path); diff --git a/lib/private/Archive/TAR.php b/lib/private/Archive/TAR.php index a3c2abb41bb..79c09cbe9e2 100644 --- a/lib/private/Archive/TAR.php +++ b/lib/private/Archive/TAR.php @@ -58,10 +58,7 @@ class TAR extends Archive { */ private $path; - /** - * @param string $source - */ - public function __construct($source) { + public function __construct(string $source) { $types = [null, 'gz', 'bz2']; $this->path = $source; $this->tar = new \Archive_Tar($source, $types[self::getTarType($source)]); @@ -69,11 +66,8 @@ class TAR extends Archive { /** * try to detect the type of tar compression - * - * @param string $file - * @return integer */ - public static function getTarType($file) { + public static function getTarType(string $file): int { if (strpos($file, '.')) { $extension = substr($file, strrpos($file, '.')); switch ($extension) { @@ -95,11 +89,8 @@ class TAR extends Archive { /** * add an empty folder to the archive - * - * @param string $path - * @return bool */ - public function addFolder($path) { + public function addFolder(string $path): bool { $tmpBase = \OC::$server->getTempManager()->getTemporaryFolder(); $path = rtrim($path, '/') . '/'; if ($this->fileExists($path)) { @@ -123,11 +114,9 @@ class TAR extends Archive { /** * add a file to the archive * - * @param string $path * @param string $source either a local file or string data - * @return bool */ - public function addFile($path, $source = '') { + public function addFile(string $path, string $source = ''): bool { if ($this->fileExists($path)) { $this->remove($path); } @@ -142,12 +131,8 @@ class TAR extends Archive { /** * rename a file or folder in the archive - * - * @param string $source - * @param string $dest - * @return bool */ - public function rename($source, $dest) { + public function rename(string $source, string $dest): bool { //no proper way to delete, rename entire archive, rename file and remake archive $tmp = \OC::$server->getTempManager()->getTemporaryFolder(); $this->tar->extract($tmp); @@ -162,11 +147,7 @@ class TAR extends Archive { return true; } - /** - * @param string $file - * @return array|null - */ - private function getHeader($file) { + private function getHeader(string $file): ?array { if (!$this->cachedHeaders) { $this->cachedHeaders = $this->tar->listContent(); } @@ -185,10 +166,9 @@ class TAR extends Archive { /** * get the uncompressed size of a file in the archive * - * @param string $path * @return int|false */ - public function filesize($path) { + public function filesize(string $path) { $stat = $this->getHeader($path); return $stat['size'] ?? false; } @@ -196,21 +176,17 @@ class TAR extends Archive { /** * get the last modified time of a file in the archive * - * @param string $path * @return int|false */ - public function mtime($path) { + public function mtime(string $path) { $stat = $this->getHeader($path); return $stat['mtime'] ?? false; } /** * get the files in a folder - * - * @param string $path - * @return array */ - public function getFolder($path) { + public function getFolder(string $path): array { $files = $this->getFiles(); $folderContent = []; $pathLength = strlen($path); @@ -233,10 +209,8 @@ class TAR extends Archive { /** * get all files in the archive - * - * @return array */ - public function getFiles() { + public function getFiles(): array { if ($this->fileList) { return $this->fileList; } @@ -254,10 +228,9 @@ class TAR extends Archive { /** * get the content of a file * - * @param string $path * @return string|false */ - public function getFile($path) { + public function getFile(string $path) { $string = $this->tar->extractInString($path); if (is_string($string)) { return $string; @@ -268,12 +241,8 @@ class TAR extends Archive { /** * extract a single file from the archive - * - * @param string $path - * @param string $dest - * @return bool */ - public function extractFile($path, $dest) { + public function extractFile(string $path, string $dest): bool { $tmp = \OC::$server->getTempManager()->getTemporaryFolder(); if (!$this->fileExists($path)) { return false; @@ -292,21 +261,15 @@ class TAR extends Archive { /** * extract the archive - * - * @param string $dest - * @return bool */ - public function extract($dest) { + public function extract(string $dest): bool { return $this->tar->extract($dest); } /** * check if a file or folder exists in the archive - * - * @param string $path - * @return bool */ - public function fileExists($path) { + public function fileExists(string $path): bool { $files = $this->getFiles(); if ((array_search($path, $files) !== false) or (array_search($path . '/', $files) !== false)) { return true; @@ -328,11 +291,8 @@ class TAR extends Archive { /** * remove a file or folder from the archive - * - * @param string $path - * @return bool */ - public function remove($path) { + public function remove(string $path): bool { if (!$this->fileExists($path)) { return false; } @@ -352,13 +312,12 @@ class TAR extends Archive { /** * get a file handler * - * @param string $path - * @param string $mode * @return bool|resource */ - public function getStream($path, $mode) { - if (strrpos($path, '.') !== false) { - $ext = substr($path, strrpos($path, '.')); + public function getStream(string $path, string $mode) { + $lastPoint = strrpos($path, '.'); + if ($lastPoint !== false) { + $ext = substr($path, $lastPoint); } else { $ext = ''; } @@ -380,11 +339,8 @@ class TAR extends Archive { /** * write back temporary files - * @param string $tmpFile - * @param string $path - * @return void */ - public function writeBack($tmpFile, $path) { + public function writeBack(string $tmpFile, string $path): void { $this->addFile($path, $tmpFile); unlink($tmpFile); } diff --git a/lib/private/Archive/ZIP.php b/lib/private/Archive/ZIP.php index 40dfa3b41b6..ca9a046ab83 100644 --- a/lib/private/Archive/ZIP.php +++ b/lib/private/Archive/ZIP.php @@ -45,10 +45,7 @@ class ZIP extends Archive { */ private $path; - /** - * @param string $source - */ - public function __construct($source) { + public function __construct(string $source) { $this->path = $source; $this->zip = new \ZipArchive(); if ($this->zip->open($source, \ZipArchive::CREATE)) { @@ -56,21 +53,21 @@ class ZIP extends Archive { \OC::$server->get(LoggerInterface::class)->warning('Error while opening archive '.$source, ['app' => 'files_archive']); } } + /** * add an empty folder to the archive * @param string $path * @return bool */ - public function addFolder($path) { + public function addFolder(string $path): bool { return $this->zip->addEmptyDir($path); } + /** * add a file to the archive - * @param string $path * @param string $source either a local file or string data - * @return bool */ - public function addFile($path, $source = '') { + public function addFile(string $path, string $source = ''): bool { if ($source and $source[0] == '/' and file_exists($source)) { $result = $this->zip->addFile($source, $path); } else { @@ -82,40 +79,37 @@ class ZIP extends Archive { } return $result; } + /** * rename a file or folder in the archive - * @param string $source - * @param string $dest - * @return bool */ - public function rename($source, $dest) { + public function rename(string $source, string $dest): bool { $source = $this->stripPath($source); $dest = $this->stripPath($dest); return $this->zip->renameName($source, $dest); } + /** * get the uncompressed size of a file in the archive - * @param string $path * @return int|false */ - public function filesize($path) { + public function filesize(string $path) { $stat = $this->zip->statName($path); return $stat['size'] ?? false; } + /** * get the last modified time of a file in the archive - * @param string $path * @return int|false */ - public function mtime($path) { + public function mtime(string $path) { return filemtime($this->path); } + /** * get the files in a folder - * @param string $path - * @return array */ - public function getFolder($path) { + public function getFolder(string $path): array { $files = $this->getFiles(); $folderContent = []; $pathLength = strlen($path); @@ -128,11 +122,11 @@ class ZIP extends Archive { } return $folderContent; } + /** * get all files in the archive - * @return array */ - public function getFiles() { + public function getFiles(): array { $fileCount = $this->zip->numFiles; $files = []; for ($i = 0;$i < $fileCount;$i++) { @@ -140,62 +134,56 @@ class ZIP extends Archive { } return $files; } + /** * get the content of a file - * @param string $path * @return string|false */ - public function getFile($path) { + public function getFile(string $path) { return $this->zip->getFromName($path); } + /** * extract a single file from the archive - * @param string $path - * @param string $dest - * @return bool */ - public function extractFile($path, $dest) { + public function extractFile(string $path, string $dest): bool { $fp = $this->zip->getStream($path); if ($fp === false) { return false; } return file_put_contents($dest, $fp) !== false; } + /** * extract the archive - * @param string $dest - * @return bool */ - public function extract($dest) { + public function extract(string $dest): bool { return $this->zip->extractTo($dest); } + /** * check if a file or folder exists in the archive - * @param string $path - * @return bool */ - public function fileExists($path) { + public function fileExists(string $path): bool { return ($this->zip->locateName($path) !== false) or ($this->zip->locateName($path.'/') !== false); } + /** * remove a file or folder from the archive - * @param string $path - * @return bool */ - public function remove($path) { + public function remove(string $path): bool { if ($this->fileExists($path.'/')) { return $this->zip->deleteName($path.'/'); } else { return $this->zip->deleteName($path); } } + /** * get a file handler - * @param string $path - * @param string $mode * @return bool|resource */ - public function getStream($path, $mode) { + public function getStream(string $path, string $mode) { if ($mode == 'r' or $mode == 'rb') { return $this->zip->getStream($path); } else { @@ -221,20 +209,13 @@ class ZIP extends Archive { /** * write back temporary files - * @param string $tmpFile - * @param string $path - * @return void */ - public function writeBack($tmpFile, $path) { + public function writeBack(string $tmpFile, string $path): void { $this->addFile($path, $tmpFile); unlink($tmpFile); } - /** - * @param string $path - * @return string - */ - private function stripPath($path) { + private function stripPath(string $path): string { if (!$path || $path[0] == '/') { return substr($path, 1); } else {