From 0db4617e694eba8e96ce4bb11655724ee4dcac75 Mon Sep 17 00:00:00 2001 From: 19leunam93 Date: Sun, 17 Dec 2023 11:23:12 +0100 Subject: [PATCH 1/2] fix issue by checking if is_file() Before calling File::delete(), the file path gets checked for existence with the PHP build in function is_file(). Only if the provided path is an existing file, it gets deleted using File::delete(). --- .../src/Installer/Adapter/FileAdapter.php | 11 ++++++++-- .../src/Installer/Adapter/LibraryAdapter.php | 6 +++++- .../src/Installer/Adapter/PackageAdapter.php | 6 +++++- libraries/src/Installer/Installer.php | 20 +++++++++++++++---- libraries/src/Installer/InstallerScript.php | 2 +- 5 files changed, 36 insertions(+), 9 deletions(-) diff --git a/libraries/src/Installer/Adapter/FileAdapter.php b/libraries/src/Installer/Adapter/FileAdapter.php index 52da239f4bfa7..5d4fd95974529 100644 --- a/libraries/src/Installer/Adapter/FileAdapter.php +++ b/libraries/src/Installer/Adapter/FileAdapter.php @@ -172,7 +172,11 @@ protected function finaliseInstall() */ protected function finaliseUninstall(): bool { - File::delete(JPATH_MANIFESTS . '/files/' . $this->extension->element . '.xml'); + $manifest = JPATH_MANIFESTS . '/files/' . $this->extension->element . '.xml'; + + if (\is_file($manifest)) { + File::delete($manifest); + } $extensionId = $this->extension->extension_id; @@ -283,7 +287,10 @@ protected function removeExtensionFiles() $folderList[] = $targetFolder . '/' . $eFileName; } else { $fileName = $targetFolder . '/' . $eFileName; - File::delete($fileName); + + if (is_file($fileName)) { + File::delete($fileName); + } } } } diff --git a/libraries/src/Installer/Adapter/LibraryAdapter.php b/libraries/src/Installer/Adapter/LibraryAdapter.php index 18d991b1908b8..1ad62dd8b3f7a 100644 --- a/libraries/src/Installer/Adapter/LibraryAdapter.php +++ b/libraries/src/Installer/Adapter/LibraryAdapter.php @@ -274,7 +274,11 @@ public function prepareDiscoverInstall() protected function removeExtensionFiles() { $this->parent->removeFiles($this->getManifest()->files, -1); - File::delete(JPATH_MANIFESTS . '/libraries/' . $this->extension->element . '.xml'); + $manifest = JPATH_MANIFESTS . '/libraries/' . $this->extension->element . '.xml'; + + if (is_file($manifest)) { + File::delete($manifest); + } // @todo: Change this so it walked up the path backwards so we clobber multiple empties // If the folder is empty, let's delete it diff --git a/libraries/src/Installer/Adapter/PackageAdapter.php b/libraries/src/Installer/Adapter/PackageAdapter.php index d2f2d24af9083..7b67c31cf8357 100644 --- a/libraries/src/Installer/Adapter/PackageAdapter.php +++ b/libraries/src/Installer/Adapter/PackageAdapter.php @@ -323,7 +323,11 @@ protected function finaliseUninstall(): bool $update->delete($uid); } - File::delete(JPATH_MANIFESTS . '/packages/' . $this->extension->element . '.xml'); + $file = JPATH_MANIFESTS . '/packages/' . $this->extension->element . '.xml'; + + if (is_file($file)) { + File::delete($file); + } $folder = $this->parent->getPath('extension_root'); diff --git a/libraries/src/Installer/Installer.php b/libraries/src/Installer/Installer.php index 2e69af40b1168..5a3a1e62e5a5c 100644 --- a/libraries/src/Installer/Installer.php +++ b/libraries/src/Installer/Installer.php @@ -525,12 +525,16 @@ public function abort($msg = null, $type = null) switch ($step['type']) { case 'file': // Remove the file - $stepval = File::delete($step['path']); + if (is_file($step['path']) && !($stepval = File::delete($step['path']))) { + Log::add(Text::sprintf('JLIB_INSTALLER_ERROR_FILE_FOLDER', $step['path']), Log::WARNING, 'jerror'); + } break; case 'folder': // Remove the folder - $stepval = Folder::delete($step['path']); + if (Folder::exists($step['path']) && !($stepval = Folder::delete($step['path']))) { + Log::add(Text::sprintf('JLIB_INSTALLER_ERROR_FILE_FOLDER', $step['path']), Log::WARNING, 'jerror'); + } break; case 'query': @@ -1429,11 +1433,19 @@ public function parseFiles(\SimpleXMLElement $element, $cid = 0, $oldFiles = nul $deletions = $this->findDeletedFiles($oldEntries, $element->children()); foreach ($deletions['folders'] as $deleted_folder) { - Folder::delete($destination . '/' . $deleted_folder); + $folder = $destination . '/' . $deleted_folder; + + if (Folder::exists($folder) && !Folder::delete($folder)) { + Log::add(Text::sprintf('JLIB_INSTALLER_ERROR_FILE_FOLDER', $folder), Log::WARNING, 'jerror'); + } } foreach ($deletions['files'] as $deleted_file) { - File::delete($destination . '/' . $deleted_file); + $file = $destination . '/' . $deleted_file; + + if (is_file($file) && !File::delete($file)) { + Log::add(Text::sprintf('JLIB_INSTALLER_ERROR_FILE_FOLDER', $file), Log::WARNING, 'jerror'); + } } } } diff --git a/libraries/src/Installer/InstallerScript.php b/libraries/src/Installer/InstallerScript.php index 23bb71499f6c7..28da7035a4f52 100644 --- a/libraries/src/Installer/InstallerScript.php +++ b/libraries/src/Installer/InstallerScript.php @@ -317,7 +317,7 @@ public function removeFiles() { if (!empty($this->deleteFiles)) { foreach ($this->deleteFiles as $file) { - if (file_exists(JPATH_ROOT . $file) && !File::delete(JPATH_ROOT . $file)) { + if (is_file(JPATH_ROOT . $file) && !File::delete(JPATH_ROOT . $file)) { echo Text::sprintf('JLIB_INSTALLER_ERROR_FILE_FOLDER', $file) . '
'; } } From ccb0e5991f40d3d696221531d58434e91eb02ed1 Mon Sep 17 00:00:00 2001 From: 19leunam93 Date: Sun, 17 Dec 2023 17:23:30 +0100 Subject: [PATCH 2/2] remove \ for consistency --- libraries/src/Installer/Adapter/FileAdapter.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/src/Installer/Adapter/FileAdapter.php b/libraries/src/Installer/Adapter/FileAdapter.php index 5d4fd95974529..85f1eb98b2e8e 100644 --- a/libraries/src/Installer/Adapter/FileAdapter.php +++ b/libraries/src/Installer/Adapter/FileAdapter.php @@ -174,7 +174,7 @@ protected function finaliseUninstall(): bool { $manifest = JPATH_MANIFESTS . '/files/' . $this->extension->element . '.xml'; - if (\is_file($manifest)) { + if (is_file($manifest)) { File::delete($manifest); }