fix(trashbin): Truncate long filenames

Signed-off-by: Christopher Ng <chrng8@gmail.com>

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 748ead798d8..bff116c4ec4 100644
--- a/apps/files_trashbin/lib/Command/RestoreAllFiles.php
+++ b/apps/files_trashbin/lib/Command/RestoreAllFiles.php
@@ -146,7 +146,7 @@ class RestoreAllFiles extends Base {
 			$timestamp = $trashFile->getMtime();
 			$humanTime = $this->l10n->l('datetime', $timestamp);
 			$output->write("File <info>$filename</info> originally deleted at <info>$humanTime</info> ");
-			$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 13df15ff084..d9dc102ac83 100644
--- a/apps/files_trashbin/lib/Sabre/TrashFile.php
+++ b/apps/files_trashbin/lib/Sabre/TrashFile.php
@@ -26,12 +26,14 @@ declare(strict_types=1);
  */
 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 094f80dad98..4623913b18a 100644
--- a/apps/files_trashbin/lib/Sabre/TrashFolder.php
+++ b/apps/files_trashbin/lib/Sabre/TrashFolder.php
@@ -26,8 +26,10 @@ declare(strict_types=1);
  */
 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 22f12c9a062..3d1f3c42bc4 100644
--- a/apps/files_trashbin/lib/Trash/LegacyTrashBackend.php
+++ b/apps/files_trashbin/lib/Trash/LegacyTrashBackend.php
@@ -58,11 +58,12 @@ class LegacyTrashBackend implements ITrashBackend {
 			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 00e6b35cc62..db6109e62f3 100644
--- a/apps/files_trashbin/lib/Trashbin.php
+++ b/apps/files_trashbin/lib/Trashbin.php
@@ -203,8 +203,8 @@ class Trashbin {
 
 		$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;
@@ -278,7 +278,7 @@ class Trashbin {
 		$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) {
@@ -294,7 +294,7 @@ class Trashbin {
 
 				$timestamp = $timestamp + 1;
 
-				$trashPath = '/files_trashbin/files/' . $filename . '.d' . $timestamp;
+				$trashPath = '/files_trashbin/files/' . static::getTrashFilename($filename, $timestamp);
 			}
 		}
 
@@ -358,7 +358,7 @@ class Trashbin {
 				\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);
 
@@ -395,15 +395,15 @@ class Trashbin {
 
 			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));
 				}
 			}
 		}
@@ -561,7 +561,7 @@ class Trashbin {
 			} 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);
 					}
@@ -662,7 +662,7 @@ class Trashbin {
 				->andWhere($query->expr()->eq('timestamp', $query->createNamedParameter($timestamp)));
 			$query->executeStatement();
 
-			$file = $filename . '.d' . $timestamp;
+			$file = static::getTrashFilename($filename, $timestamp);
 		} else {
 			$file = $filename;
 		}
@@ -705,8 +705,8 @@ class Trashbin {
 			} 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);
@@ -729,7 +729,7 @@ class Trashbin {
 		$view = new View('/' . $user);
 
 		if ($timestamp) {
-			$filename = $filename . '.d' . $timestamp;
+			$filename = static::getTrashFilename($filename, $timestamp);
 		}
 
 		$target = Filesystem::normalizePath('files_trashbin/files/' . $filename);
@@ -1125,4 +1125,23 @@ class Trashbin {
 	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 b4892d2deb0..15b47385d45 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();
 
@@ -109,6 +114,8 @@ class StorageTest extends \Test\TestCase {
 		$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');
@@ -165,6 +172,44 @@ class StorageTest extends \Test\TestCase {
 	}
 
 	/**
+	 * 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
 	 * the trashbin folder is in the root storage while the mounted one