From 4722551a1698f0cc5534d784fbcddbd66054365d Mon Sep 17 00:00:00 2001 From: odain-cbd <56586767+odain-cbd@users.noreply.github.com> Date: Wed, 19 Aug 2026 15:14:32 +0200 Subject: [PATCH] =?UTF-8?q?=20N=C2=B09875=20-=20Error=20deprecated=20on=20?= =?UTF-8?q?component=20version=20object=20(#1010)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * N°9875 - Error deprecated on component version object - propose boilerplate methods to handle ZipArchive * N°9875 - Error deprecated on component version object - use ZipArchive boilerplate methods * N°9875 - code style * N°9875 - review feedbacks * N°9875 - simplify return type of ZipArchiveCreateWithTempNam * N°9875 - add @since --- application/utils.inc.php | 44 +++++++++ .../src/Service/CoreUpdater.php | 3 +- datamodels/2.x/itop-hub-connector/land.php | 6 +- .../TwigBase/Controller/Controller.php | 6 +- .../unitary-tests/application/utilsTest.php | 99 +++++++++++++++++++ 5 files changed, 150 insertions(+), 8 deletions(-) diff --git a/application/utils.inc.php b/application/utils.inc.php index 4f0ded9679..821bfaf6fe 100644 --- a/application/utils.inc.php +++ b/application/utils.inc.php @@ -3249,4 +3249,48 @@ TXT return (int)$sLimit; } } + + /** + * @since 3.3.0 N°9875 + * Open archive and raise appropriate exception. + * Warning: do not forget to close archive afterwhile + * @param string $sArchiveFilePath + * @param int|null $flags + * @return ZipArchive + * @throws \Exception + */ + public static function ZipArchiveOpen(string $sArchiveFilePath, int|null $flags = null): ZipArchive + { + $oZip = new ZipArchive(); + if (is_null($flags)) { + $code = $oZip->open($sArchiveFilePath); + } else { + $code = $oZip->open($sArchiveFilePath, $flags); + } + if (true !== $code) { + //ZipArchive::ZIP_ER_NOZIP : 19 + if ($code === 19) { + throw new \Exception(sprintf('Cannot to open zip file due to inconsistent or empty content')); + } + + throw new \Exception(sprintf('Cannot to open zip file due to error code %s', $code)); + } + return $oZip; + } + + /** + * @since 3.3.0 N°9875 + * @param string $sDirectory + * @param string $sPrefix + * @return ZipArchive + * @throws \Exception + */ + public static function ZipArchiveCreateWithTempNam(string $sDirectory, string $sPrefix): ZipArchive + { + $sTempnam = tempnam($sDirectory, $sPrefix); + unlink($sTempnam); + $sArchiveName = $sTempnam.'.zip'; + + return self::ZipArchiveOpen($sArchiveName, ZipArchive::CREATE); + } } diff --git a/datamodels/2.x/itop-core-update/src/Service/CoreUpdater.php b/datamodels/2.x/itop-core-update/src/Service/CoreUpdater.php index 116da96a56..59f56de931 100644 --- a/datamodels/2.x/itop-core-update/src/Service/CoreUpdater.php +++ b/datamodels/2.x/itop-core-update/src/Service/CoreUpdater.php @@ -217,8 +217,7 @@ final class CoreUpdater throw new Exception(Dict::S('iTopUpdate:Error:BadFileFormat')); } - $oArchive = new ZipArchive(); - $oArchive->open($sArchiveFile); + $oArchive = utils::ZipArchiveOpen($sArchiveFile); self::RRmdir(self::UPDATE_DIR); SetupUtils::builddir(self::UPDATE_DIR); diff --git a/datamodels/2.x/itop-hub-connector/land.php b/datamodels/2.x/itop-hub-connector/land.php index 67e4707457..8d82ab187a 100644 --- a/datamodels/2.x/itop-hub-connector/land.php +++ b/datamodels/2.x/itop-hub-connector/land.php @@ -113,8 +113,10 @@ function DoLanding(WebPage $oPage) file_put_contents($sZipArchiveFile, $sArchive); // Expand the content of extension-x.zip into utils::GetDataPath().'downloaded-extensions/' // where the installation will load the extension automatically - $oZip = new ZipArchive(); - if (!$oZip->open($sZipArchiveFile)) { + + try { + $oZip = utils::ZipArchiveOpen($sZipArchiveFile); + } catch (\Exception $e) { throw new Exception('Unable to open "'.$sZipArchiveFile.'" for extraction. Make sure that the directory "'.'data/downloaded-extensions/'.'" is writable for the web server.'); } for ($idx = 0; $idx < $oZip->numFiles; $idx++) { diff --git a/sources/Application/TwigBase/Controller/Controller.php b/sources/Application/TwigBase/Controller/Controller.php index b3f2bb564d..5235cd22b6 100644 --- a/sources/Application/TwigBase/Controller/Controller.php +++ b/sources/Application/TwigBase/Controller/Controller.php @@ -646,9 +646,7 @@ abstract class Controller extends AbstractController */ final protected function ZipDownloadRemoveFile(array $aFiles, string $sDownloadArchiveName, bool $bUnlinkFiles = false): void { - $sArchiveFileFullPath = tempnam(SetupUtils::GetTmpDir(), 'itop_download-').'.zip'; - $oArchive = new ZipArchive(); - $oArchive->open($sArchiveFileFullPath, ZipArchive::CREATE); + $oArchive = utils::ZipArchiveCreateWithTempNam(SetupUtils::GetTmpDir(), 'itop_download-'); foreach ($aFiles as $sFile) { $oArchive->addFile($sFile, basename($sFile)); } @@ -660,7 +658,7 @@ abstract class Controller extends AbstractController } } - $this->SendFileContent($sArchiveFileFullPath, $sDownloadArchiveName.'.zip', true, true); + $this->SendFileContent($oArchive->filename, $sDownloadArchiveName.'.zip', true, true); } final protected function SendFileContent($sFilePath, $sDownloadArchiveName = null, $bFileTransfer = true, $bRemoveFile = false, $aHeaders = []): void diff --git a/tests/php-unit-tests/unitary-tests/application/utilsTest.php b/tests/php-unit-tests/unitary-tests/application/utilsTest.php index 6084e3453b..5e4e1940fa 100644 --- a/tests/php-unit-tests/unitary-tests/application/utilsTest.php +++ b/tests/php-unit-tests/unitary-tests/application/utilsTest.php @@ -1009,4 +1009,103 @@ INI; utils::Unserialize($sData); } + + public static function ZipArchiveOpen_ValidZipFileProvider() + { + return [ + "RDONLY" => [\ZipArchive::RDONLY], + "null" => [null], + ]; + } + + /** + * @dataProvider ZipArchiveOpen_ValidZipFileProvider + */ + public function testZipArchiveOpen_ValidZipFile($flags) + { + $sArchiveName = tempnam(sys_get_temp_dir(), "testZipArchiveOpen_ValidZipFile_"); + unlink($sArchiveName); + $oZip = new \ZipArchive(); + $oZip->open($sArchiveName, \ZipArchive::CREATE); + $oZip->addFile(__FILE__); + $oZip->close(); + + $this->aFileToClean [] = $sArchiveName; + + $oZip = utils::ZipArchiveOpen($sArchiveName, $flags); + self::assertNotNull($oZip); + $oZip->close(); + } + + public static function ZipArchiveOpen_EmptyExistingFileProvider() + { + return [ + "RDONLY" => [\ZipArchive::RDONLY, 'Cannot to open zip file due to inconsistent or empty content'], + "OVERWRITE" => [\ZipArchive::OVERWRITE], + ]; + } + + /** + * @dataProvider ZipArchiveOpen_EmptyExistingFileProvider + */ + public function testZipArchiveOpen_EmptyExistingFile($flags, $sExpectedMessage = null) + { + $sFolderPath = tempnam(sys_get_temp_dir(), "testZipArchiveOpen_ZipFile_"); + $this->aFileToClean [] = $sFolderPath; + + if (! is_null($sExpectedMessage)) { + $this->expectExceptionMessage($sExpectedMessage); + } + $oZip = utils::ZipArchiveOpen($sFolderPath, $flags); + if (is_null($sExpectedMessage)) { + self::assertNotNull($oZip); + $oZip->close(); + touch($sFolderPath); + } + } + + public static function ZipArchiveOpen_NotyExistingFileProvider() + { + return [ + "CREATE" => [\ZipArchive::CREATE], + "null" => [null, 'Cannot to open zip file due to error code 9'], + ]; + } + + /** + * @dataProvider ZipArchiveOpen_NotyExistingFileProvider + */ + public function testZipArchiveOpen_NotyExistingFile($flags, $sExpectedMessage = null) + { + $sFolderPath = tempnam(sys_get_temp_dir(), "testZipArchiveOpen_ZipFile_"); + @unlink($sFolderPath); + + if (! is_null($sExpectedMessage)) { + $this->expectExceptionMessage($sExpectedMessage); + } + $oZip = utils::ZipArchiveOpen($sFolderPath, $flags); + if (is_null($sExpectedMessage)) { + self::assertNotNull($oZip); + $oZip->close(); + touch($sFolderPath); + $this->aFileToClean[] = $sFolderPath; + } + } + + public function testZipArchiveCreateWithTempNam() + { + $oZip = utils::ZipArchiveCreateWithTempNam(sys_get_temp_dir(), "testZipArchiveOpenWithTempFile_"); + self::assertNotNull($oZip); + $sFilePath = $oZip->filename; + + self::assertFalse(is_file($sFilePath), $sFilePath); + + $oZip->addEmptyDir('toto'); + $oZip->addFile(__FILE__); + $oZip->close(); + + self::assertTrue(in_array($sFilePath, glob(sys_get_temp_dir().'/**'))); + self::assertTrue(is_file($sFilePath), $sFilePath); + unlink($sFilePath); + } }