From ad1052ddc951386d5b53958865d399de28594434 Mon Sep 17 00:00:00 2001 From: Christopher Ng Date: Wed, 31 May 2023 19:06:42 -0700 Subject: [PATCH] fix(trashbin): Truncate long filenames Signed-off-by: Christopher Ng --- .../lib/Command/RestoreAllFiles.php | 2 +- apps/files_trashbin/lib/Sabre/TrashFile.php | 6 ++- apps/files_trashbin/lib/Sabre/TrashFolder.php | 4 +- .../lib/Trash/LegacyTrashBackend.php | 3 +- apps/files_trashbin/lib/Trashbin.php | 47 +++++++++++++------ apps/files_trashbin/tests/StorageTest.php | 45 ++++++++++++++++++ 6 files changed, 88 insertions(+), 19 deletions(-) diff --git a/apps/files_trashbin/lib/Command/RestoreAllFiles.php b/apps/files_trashbin/lib/Command/RestoreAllFiles.php index 43e9363327bdb..23bce17742cbf 100644 --- a/apps/files_trashbin/lib/Command/RestoreAllFiles.php +++ b/apps/files_trashbin/lib/Command/RestoreAllFiles.php @@ -142,7 +142,7 @@ protected function restoreDeletedFiles(string $uid, OutputInterface $output): vo $timestamp = $trashFile->getMtime(); $humanTime = $this->l10n->l('datetime', $timestamp); $output->write("File $filename originally deleted at $humanTime "); - $file = $filename . '.d' . $timestamp; + $file = Trashbin::getTrashFilename($filename, $timestamp); $location = Trashbin::getLocation($uid, $filename, (string) $timestamp); if ($location === '.') { $location = ''; diff --git a/apps/files_trashbin/lib/Sabre/TrashFile.php b/apps/files_trashbin/lib/Sabre/TrashFile.php index 13df15ff08448..d9dc102ac831c 100644 --- a/apps/files_trashbin/lib/Sabre/TrashFile.php +++ b/apps/files_trashbin/lib/Sabre/TrashFile.php @@ -26,12 +26,14 @@ */ namespace OCA\Files_Trashbin\Sabre; +use OCA\Files_Trashbin\Trashbin; + class TrashFile extends AbstractTrashFile { public function get() { - return $this->data->getStorage()->fopen($this->data->getInternalPath() . '.d' . $this->getLastModified(), 'rb'); + return $this->data->getStorage()->fopen(Trashbin::getTrashFilename($this->data->getInternalPath(), $this->getLastModified()), 'rb'); } public function getName(): string { - return $this->data->getName() . '.d' . $this->getLastModified(); + return Trashbin::getTrashFilename($this->data->getName(), $this->getLastModified()); } } diff --git a/apps/files_trashbin/lib/Sabre/TrashFolder.php b/apps/files_trashbin/lib/Sabre/TrashFolder.php index 094f80dad98be..4623913b18a78 100644 --- a/apps/files_trashbin/lib/Sabre/TrashFolder.php +++ b/apps/files_trashbin/lib/Sabre/TrashFolder.php @@ -26,8 +26,10 @@ */ namespace OCA\Files_Trashbin\Sabre; +use OCA\Files_Trashbin\Trashbin; + class TrashFolder extends AbstractTrashFolder { public function getName(): string { - return $this->data->getName() . '.d' . $this->getLastModified(); + return Trashbin::getTrashFilename($this->data->getName(), $this->getLastModified()); } } diff --git a/apps/files_trashbin/lib/Trash/LegacyTrashBackend.php b/apps/files_trashbin/lib/Trash/LegacyTrashBackend.php index 22f12c9a062d6..3d1f3c42bc41c 100644 --- a/apps/files_trashbin/lib/Trash/LegacyTrashBackend.php +++ b/apps/files_trashbin/lib/Trash/LegacyTrashBackend.php @@ -58,11 +58,12 @@ private function mapTrashItems(array $items, IUser $user, ITrashItem $parent = n if (!$originalLocation) { $originalLocation = $file->getName(); } + $trashFilename = Trashbin::getTrashFilename($file->getName(), $file->getMtime()); return new TrashItem( $this, $originalLocation, $file->getMTime(), - $parentTrashPath . '/' . $file->getName() . ($isRoot ? '.d' . $file->getMtime() : ''), + $parentTrashPath . '/' . ($isRoot ? $trashFilename : $file->getName()), $file, $user ); diff --git a/apps/files_trashbin/lib/Trashbin.php b/apps/files_trashbin/lib/Trashbin.php index 32488cb9103c4..a86a45e47b0c3 100644 --- a/apps/files_trashbin/lib/Trashbin.php +++ b/apps/files_trashbin/lib/Trashbin.php @@ -202,8 +202,8 @@ private static function copyFilesToUser($sourcePath, $owner, $targetPath, $user, $view = new View('/'); - $target = $user . '/files_trashbin/files/' . $targetFilename . '.d' . $timestamp; - $source = $owner . '/files_trashbin/files/' . $sourceFilename . '.d' . $timestamp; + $target = $user . '/files_trashbin/files/' . static::getTrashFilename($targetFilename, $timestamp); + $source = $owner . '/files_trashbin/files/' . static::getTrashFilename($sourceFilename, $timestamp); $free = $view->free_space($target); $isUnknownOrUnlimitedFreeSpace = $free < 0; $isEnoughFreeSpaceLeft = $view->filesize($source) < $free; @@ -270,7 +270,7 @@ public static function move2trash($file_path, $ownerOnly = false) { $lockingProvider = \OC::$server->getLockingProvider(); // disable proxy to prevent recursive calls - $trashPath = '/files_trashbin/files/' . $filename . '.d' . $timestamp; + $trashPath = '/files_trashbin/files/' . static::getTrashFilename($filename, $timestamp); $gotLock = false; while (!$gotLock) { @@ -286,7 +286,7 @@ public static function move2trash($file_path, $ownerOnly = false) { $timestamp = $timestamp + 1; - $trashPath = '/files_trashbin/files/' . $filename . '.d' . $timestamp; + $trashPath = '/files_trashbin/files/' . static::getTrashFilename($filename, $timestamp); } } @@ -351,7 +351,7 @@ public static function move2trash($file_path, $ownerOnly = false) { \OC::$server->get(LoggerInterface::class)->error('trash bin database couldn\'t be updated', ['app' => 'files_trashbin']); } \OCP\Util::emitHook('\OCA\Files_Trashbin\Trashbin', 'post_moveToTrash', ['filePath' => Filesystem::normalizePath($file_path), - 'trashPath' => Filesystem::normalizePath($filename . '.d' . $timestamp)]); + 'trashPath' => Filesystem::normalizePath(static::getTrashFilename($filename, $timestamp))]); self::retainVersions($filename, $owner, $ownerPath, $timestamp); @@ -388,15 +388,15 @@ private static function retainVersions($filename, $owner, $ownerPath, $timestamp if ($rootView->is_dir($owner . '/files_versions/' . $ownerPath)) { if ($owner !== $user) { - self::copy_recursive($owner . '/files_versions/' . $ownerPath, $owner . '/files_trashbin/versions/' . basename($ownerPath) . '.d' . $timestamp, $rootView); + self::copy_recursive($owner . '/files_versions/' . $ownerPath, $owner . '/files_trashbin/versions/' . static::getTrashFilename(basename($ownerPath), $timestamp), $rootView); } - self::move($rootView, $owner . '/files_versions/' . $ownerPath, $user . '/files_trashbin/versions/' . $filename . '.d' . $timestamp); + self::move($rootView, $owner . '/files_versions/' . $ownerPath, $user . '/files_trashbin/versions/' . static::getTrashFilename($filename, $timestamp)); } elseif ($versions = \OCA\Files_Versions\Storage::getVersions($owner, $ownerPath)) { foreach ($versions as $v) { if ($owner !== $user) { - self::copy($rootView, $owner . '/files_versions' . $v['path'] . '.v' . $v['version'], $owner . '/files_trashbin/versions/' . $v['name'] . '.v' . $v['version'] . '.d' . $timestamp); + self::copy($rootView, $owner . '/files_versions' . $v['path'] . '.v' . $v['version'], $owner . '/files_trashbin/versions/' . static::getTrashFilename($v['name'] . '.v' . $v['version'], $timestamp)); } - self::move($rootView, $owner . '/files_versions' . $v['path'] . '.v' . $v['version'], $user . '/files_trashbin/versions/' . $filename . '.v' . $v['version'] . '.d' . $timestamp); + self::move($rootView, $owner . '/files_versions' . $v['path'] . '.v' . $v['version'], $user . '/files_trashbin/versions/' . static::getTrashFilename($filename . '.v' . $v['version'], $timestamp)); } } } @@ -554,7 +554,7 @@ private static function restoreVersions(View $view, $file, $filename, $uniqueFil } elseif ($versions = self::getVersionsFromTrash($versionedFile, $timestamp, $user)) { foreach ($versions as $v) { if ($timestamp) { - $rootView->rename($user . '/files_trashbin/versions/' . $versionedFile . '.v' . $v . '.d' . $timestamp, $owner . '/files_versions/' . $ownerPath . '.v' . $v); + $rootView->rename($user . '/files_trashbin/versions/' . static::getTrashFilename($versionedFile . '.v' . $v, $timestamp), $owner . '/files_versions/' . $ownerPath . '.v' . $v); } else { $rootView->rename($user . '/files_trashbin/versions/' . $versionedFile . '.v' . $v, $owner . '/files_versions/' . $ownerPath . '.v' . $v); } @@ -655,7 +655,7 @@ public static function delete($filename, $user, $timestamp = null) { ->andWhere($query->expr()->eq('timestamp', $query->createNamedParameter($timestamp))); $query->executeStatement(); - $file = $filename . '.d' . $timestamp; + $file = static::getTrashFilename($filename, $timestamp); } else { $file = $filename; } @@ -698,8 +698,8 @@ private static function deleteVersions(View $view, $file, $filename, $timestamp, } elseif ($versions = self::getVersionsFromTrash($filename, $timestamp, $user)) { foreach ($versions as $v) { if ($timestamp) { - $size += $view->filesize('/files_trashbin/versions/' . $filename . '.v' . $v . '.d' . $timestamp); - $view->unlink('/files_trashbin/versions/' . $filename . '.v' . $v . '.d' . $timestamp); + $size += $view->filesize('/files_trashbin/versions/' . static::getTrashFilename($filename . '.v' . $v, $timestamp)); + $view->unlink('/files_trashbin/versions/' . static::getTrashFilename($filename . '.v' . $v, $timestamp)); } else { $size += $view->filesize('/files_trashbin/versions/' . $filename . '.v' . $v); $view->unlink('/files_trashbin/versions/' . $filename . '.v' . $v); @@ -722,7 +722,7 @@ public static function file_exists($filename, $timestamp = null) { $view = new View('/' . $user); if ($timestamp) { - $filename = $filename . '.d' . $timestamp; + $filename = static::getTrashFilename($filename, $timestamp); } $target = Filesystem::normalizePath('files_trashbin/files/' . $filename); @@ -1118,4 +1118,23 @@ public static function isEmpty($user) { public static function preview_icon($path) { return \OC::$server->getURLGenerator()->linkToRoute('core_ajax_trashbin_preview', ['x' => 32, 'y' => 32, 'file' => $path]); } + + /** + * Return the filename used in the trash bin + */ + public static function getTrashFilename(string $filename, int $timestamp): string { + $trashFilename = $filename . '.d' . $timestamp; + $length = strlen($trashFilename); + // oc_filecache `name` column has a limit of 250 chars + $maxLength = 250; + if ($length > $maxLength) { + $trashFilename = substr_replace( + $trashFilename, + '', + $maxLength / 2, + $length - $maxLength + ); + } + return $trashFilename; + } } diff --git a/apps/files_trashbin/tests/StorageTest.php b/apps/files_trashbin/tests/StorageTest.php index 0a7a129ca28d2..10909d26e0d44 100644 --- a/apps/files_trashbin/tests/StorageTest.php +++ b/apps/files_trashbin/tests/StorageTest.php @@ -88,6 +88,11 @@ class StorageTest extends \Test\TestCase { */ private $userView; + // 239 chars so appended timestamp of 12 chars will exceed max length of 250 chars + private const LONG_FILENAME = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.txt'; + // 250 chars + private const MAX_FILENAME = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.txt'; + protected function setUp(): void { parent::setUp(); @@ -107,6 +112,8 @@ protected function setUp(): void { $this->rootView = new \OC\Files\View('/'); $this->userView = new \OC\Files\View('/' . $this->user . '/files/'); $this->userView->file_put_contents('test.txt', 'foo'); + $this->userView->file_put_contents(static::LONG_FILENAME, 'foo'); + $this->userView->file_put_contents(static::MAX_FILENAME, 'foo'); $this->userView->mkdir('folder'); $this->userView->file_put_contents('folder/inside.txt', 'bar'); @@ -162,6 +169,44 @@ public function testSingleStorageDeleteFolder() { $this->assertEquals('inside.txt', $name); } + /** + * Test that deleting a file with a long filename puts it into the trashbin. + */ + public function testSingleStorageDeleteLongFilename() { + $truncatedFilename = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.txt'; + + $this->assertTrue($this->userView->file_exists(static::LONG_FILENAME)); + $this->userView->unlink(static::LONG_FILENAME); + [$storage,] = $this->userView->resolvePath(static::LONG_FILENAME); + $storage->getScanner()->scan(''); // make sure we check the storage + $this->assertFalse($this->userView->getFileInfo(static::LONG_FILENAME)); + + // check if file is in trashbin + $results = $this->rootView->getDirectoryContent($this->user . '/files_trashbin/files/'); + $this->assertEquals(1, count($results)); + $name = $results[0]->getName(); + $this->assertEquals($truncatedFilename, substr($name, 0, strrpos($name, '.'))); + } + + /** + * Test that deleting a file with the max filename length puts it into the trashbin. + */ + public function testSingleStorageDeleteMaxLengthFilename() { + $truncatedFilename = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.txt'; + + $this->assertTrue($this->userView->file_exists(static::MAX_FILENAME)); + $this->userView->unlink(static::MAX_FILENAME); + [$storage,] = $this->userView->resolvePath(static::MAX_FILENAME); + $storage->getScanner()->scan(''); // make sure we check the storage + $this->assertFalse($this->userView->getFileInfo(static::MAX_FILENAME)); + + // check if file is in trashbin + $results = $this->rootView->getDirectoryContent($this->user . '/files_trashbin/files/'); + $this->assertEquals(1, count($results)); + $name = $results[0]->getName(); + $this->assertEquals($truncatedFilename, substr($name, 0, strrpos($name, '.'))); + } + /** * Test that deleting a file from another mounted storage properly * lands in the trashbin. This is a cross-storage situation because