Mitigate Trashbin bug that puts '//' in node paths.

sara-nl · Nov 19, 2024 · 1652b65 · 1652b65
1 parent 25cef18
commit 1652b65
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 20 deletions.
diff --git a/Makefile b/Makefile
@@ -190,7 +190,12 @@ buildapp:
 	--exclude="$(app_name)/build" \
 	--exclude="$(app_name)/release" \
 	--exclude="$(app_name)/tests" \
-	--exclude="$(app_name)/Makefile" \
+	--exclude="$(app_name)/src" \
+	--exclude="$(app_name)/js" \
+	--exclude="$(app_name)/css" \
+	--exclude="$(app_name)/img" \
+	--exclude="$(app_name)/tests" \
+	--exclude="$(app_name)/vite.config.js" \
 	--exclude="$(app_name)/*.log" \
 	--exclude="$(app_name)/phpunit*xml" \
 	--exclude="$(app_name)/composer.*" \

diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php
@@ -20,8 +20,8 @@
 use OCP\AppFramework\Bootstrap\IRegistrationContext;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Files\Events\Node\NodeDeletedEvent;
+use OCP\IUserSession;
 use OCP\Util;
-use Psr\Log\LoggerInterface;
 
 class Application extends App implements IBootstrap
 {
@@ -47,7 +47,7 @@ public function __construct()
             $this->getContainer()->get(TrashbinService::class),
             $this->getContainer()->get(FileCacheMapper::class),
             $this->getContainer()->get(TrashbinMapper::class),
-            $this->getContainer()->get(LoggerInterface::class)
+            $this->getContainer()->get(IUserSession::class)
         );
         Util::connectHook('\OCP\Trashbin', 'delete', $trashbinHook, 'permanentDelete');
     }

diff --git a/lib/Hooks/TrashbinHook.php b/lib/Hooks/TrashbinHook.php
@@ -12,6 +12,7 @@
 use OCA\SURFTrashbin\Db\FileCacheMapper;
 use OCA\SURFTrashbin\Db\TrashbinMapper;
 use OCA\SURFTrashbin\Service\TrashbinService;
+use OCP\IUserSession;
 
 class TrashbinHook
 {
@@ -24,14 +25,19 @@ class TrashbinHook
     /** @var TrashbinMapper */
     private TrashbinMapper $trashbinMapper;
 
+    /** @var IUserSession */
+    private $userSession;
+
     public function __construct(
         TrashbinService $trashbinService,
         FileCacheMapper $fileCacheMapper,
         TrashbinMapper $trashbinMapper,
+        IUserSession $userSession,
     ) {
         $this->trashbinService = $trashbinService;
         $this->fileCacheMapper = $fileCacheMapper;
         $this->trashbinMapper = $trashbinMapper;
+        $this->userSession = $userSession;
     }
 
     /**
@@ -44,14 +50,19 @@ public function permanentDelete(array $params): void
     {
         // get the filecache items and find out if we are dealing with an f_account item
         $path = $params['path'];
-        $cleanPath = trim($path, '/');
+        /**
+         * Mitigate Trashbin bug:
+         * Trashbin app leaves '//' in the path which is not present in the filecache path thus preventing proper cleanup. 
+         * We replace these with a single '/'.
+         */
+        $cleanPath = str_replace('//', '/', ltrim($path, '/'));
         $fragments = explode('/', $cleanPath);
         $name = array_pop($fragments);
         $fileCacheItems = $this->fileCacheMapper->getItems($cleanPath, $name);
         $fAccountFileCacheItem = null;
         $fAccountUID = null;
         $ownerOrUserFileCacheItem = null;
-        $ownerOrUserUserUID = null;
+        $ownerOrUserUID = null;
         // Note that if the session user IS the owner then that filecache item is already deleted,
         // so only the f_account item will be returned
         foreach ($fileCacheItems as $item) {
@@ -61,7 +72,7 @@ public function permanentDelete(array $params): void
                 $fAccountUID = $accountUID;
             } else if (TrashbinService::ACCOUNT_TYPE_USER === $accountType) {
                 $ownerOrUserFileCacheItem = $item;
-                $ownerOrUserUserUID = $accountUID;
+                $ownerOrUserUID = $accountUID;
             } else {
                 // this should not happen
                 throw new Exception('Unable to handle permanent delete. Found unexpected account type: ' . print_r($accountType, true));
@@ -76,25 +87,19 @@ public function permanentDelete(array $params): void
         $fAccountView->unlink($fAccountFileCacheItem[FileCacheMapper::TABLE_COLUMN_PATH]);
 
         if (isset($ownerOrUserFileCacheItem)) {
-            $ownerOrUserView = new View("/$ownerOrUserUserUID");
+            $ownerOrUserView = new View("/$ownerOrUserUID");
             $ownerOrUserView->unlink($ownerOrUserFileCacheItem[FileCacheMapper::TABLE_COLUMN_PATH]);
         }
 
-        // retrieve the trashbin items so we can delete them from the table
+        // Retrieve the trashbin items so we can delete them from the table
         [$fileOrFoldername, $timestamp] = $this->trashbinService->getNameAndTimestamp($name);
         $trashbinItems = $this->trashbinMapper->getItems($fileOrFoldername, $timestamp);
         foreach ($trashbinItems as $item) {
-            // to be sure; check that we are dealing with the right items
-            if (
-                $fAccountUID === $item[TrashbinMapper::TABLE_COLUMN_USER]
-                || $ownerOrUserUserUID === $item[TrashbinMapper::TABLE_COLUMN_USER]
-            ) {
-                $this->trashbinMapper->deleteItems(
-                    $item[TrashbinMapper::TABLE_COLUMN_ID],
-                    $item[TrashbinMapper::TABLE_COLUMN_TIMESTAMP],
-                    $item[TrashbinMapper::TABLE_COLUMN_USER]
-                );
-            }
+            $this->trashbinMapper->deleteItems(
+                $item[TrashbinMapper::TABLE_COLUMN_ID],
+                $item[TrashbinMapper::TABLE_COLUMN_TIMESTAMP],
+                $item[TrashbinMapper::TABLE_COLUMN_USER]
+            );
         }
     }
 }
diff --git a/lib/Service/TrashbinService.php b/lib/Service/TrashbinService.php
@@ -128,7 +128,6 @@ public function handleDeleteNode(Node $nodeDeleted): void
 			$view = new View('/' . $ownerUID);
 			$userView = new View('/' . $sessionUID);
 			$nodePath = 'files_trashbin/files/' . $userAccountFilecacheItem[FileCacheMapper::TABLE_COLUMN_NAME];
-			$this->logger->debug(" creating node $nodePath");
 			if (!$userView->file_exists($nodePath)) {
 				throw new Exception("Node '$nodePath' does not exist for user '$sessionUID'.");
 			}