Skip to content

Commit 2be74ee

Browse files
blizzzblizzz
authored
Merge pull request #38355 from nextcloud/fix/trashbin-long-filenames
fix(trashbin): Truncate long filenames
2 parents aaa226d + 7c430a8 commit 2be74ee

File tree

6 files changed

+88
-19
lines changed

6 files changed

+88
-19
lines changed

‎apps/files_trashbin/lib/Command/RestoreAllFiles.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ protected function restoreDeletedFiles(string $uid, OutputInterface $output): vo
146146
$timestamp = $trashFile->getMtime();
147147
$humanTime = $this->l10n->l('datetime', $timestamp);
148148
$output->write("File <info>$filename</info> originally deleted at <info>$humanTime</info> ");
149-
$file = $filename . '.d' . $timestamp;
149+
$file = Trashbin::getTrashFilename($filename, $timestamp);
150150
$location = Trashbin::getLocation($uid, $filename, (string) $timestamp);
151151
if ($location === '.') {
152152
$location = '';

‎apps/files_trashbin/lib/Sabre/TrashFile.php‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,14 @@
2626
*/
2727
namespace OCA\Files_Trashbin\Sabre;
2828

29+
use OCA\Files_Trashbin\Trashbin;
30+
2931
class TrashFile extends AbstractTrashFile {
3032
public function get() {
31-
return $this->data->getStorage()->fopen($this->data->getInternalPath() . '.d' . $this->getLastModified(), 'rb');
33+
return $this->data->getStorage()->fopen(Trashbin::getTrashFilename($this->data->getInternalPath(), $this->getLastModified()), 'rb');
3234
}
3335

3436
public function getName(): string {
35-
return $this->data->getName() . '.d' . $this->getLastModified();
37+
return Trashbin::getTrashFilename($this->data->getName(), $this->getLastModified());
3638
}
3739
}

‎apps/files_trashbin/lib/Sabre/TrashFolder.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,10 @@
2626
*/
2727
namespace OCA\Files_Trashbin\Sabre;
2828

29+
use OCA\Files_Trashbin\Trashbin;
30+
2931
class TrashFolder extends AbstractTrashFolder {
3032
public function getName(): string {
31-
return $this->data->getName() . '.d' . $this->getLastModified();
33+
return Trashbin::getTrashFilename($this->data->getName(), $this->getLastModified());
3234
}
3335
}

‎apps/files_trashbin/lib/Trash/LegacyTrashBackend.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,12 @@ private function mapTrashItems(array $items, IUser $user, ITrashItem $parent = n
5858
if (!$originalLocation) {
5959
$originalLocation = $file->getName();
6060
}
61+
$trashFilename = Trashbin::getTrashFilename($file->getName(), $file->getMtime());
6162
return new TrashItem(
6263
$this,
6364
$originalLocation,
6465
$file->getMTime(),
65-
$parentTrashPath . '/' . $file->getName() . ($isRoot ? '.d' . $file->getMtime() : ''),
66+
$parentTrashPath . '/' . ($isRoot ? $trashFilename : $file->getName()),
6667
$file,
6768
$user
6869
);

‎apps/files_trashbin/lib/Trashbin.php‎

Lines changed: 33 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -203,8 +203,8 @@ private static function copyFilesToUser($sourcePath, $owner, $targetPath, $user,
203203

204204
$view = new View('/');
205205

206-
$target = $user . '/files_trashbin/files/' . $targetFilename . '.d' . $timestamp;
207-
$source = $owner . '/files_trashbin/files/' . $sourceFilename . '.d' . $timestamp;
206+
$target = $user . '/files_trashbin/files/' . static::getTrashFilename($targetFilename, $timestamp);
207+
$source = $owner . '/files_trashbin/files/' . static::getTrashFilename($sourceFilename, $timestamp);
208208
$free = $view->free_space($target);
209209
$isUnknownOrUnlimitedFreeSpace = $free < 0;
210210
$isEnoughFreeSpaceLeft = $view->filesize($source) < $free;
@@ -278,7 +278,7 @@ public static function move2trash($file_path, $ownerOnly = false) {
278278
$lockingProvider = \OC::$server->getLockingProvider();
279279

280280
// disable proxy to prevent recursive calls
281-
$trashPath = '/files_trashbin/files/' . $filename . '.d' . $timestamp;
281+
$trashPath = '/files_trashbin/files/' . static::getTrashFilename($filename, $timestamp);
282282
$gotLock = false;
283283

284284
while (!$gotLock) {
@@ -294,7 +294,7 @@ public static function move2trash($file_path, $ownerOnly = false) {
294294

295295
$timestamp = $timestamp + 1;
296296

297-
$trashPath = '/files_trashbin/files/' . $filename . '.d' . $timestamp;
297+
$trashPath = '/files_trashbin/files/' . static::getTrashFilename($filename, $timestamp);
298298
}
299299
}
300300

@@ -358,7 +358,7 @@ public static function move2trash($file_path, $ownerOnly = false) {
358358
\OC::$server->get(LoggerInterface::class)->error('trash bin database couldn\'t be updated', ['app' => 'files_trashbin']);
359359
}
360360
\OCP\Util::emitHook('\OCA\Files_Trashbin\Trashbin', 'post_moveToTrash', ['filePath' => Filesystem::normalizePath($file_path),
361-
'trashPath' => Filesystem::normalizePath($filename . '.d' . $timestamp)]);
361+
'trashPath' => Filesystem::normalizePath(static::getTrashFilename($filename, $timestamp))]);
362362

363363
self::retainVersions($filename, $owner, $ownerPath, $timestamp);
364364

@@ -395,15 +395,15 @@ private static function retainVersions($filename, $owner, $ownerPath, $timestamp
395395

396396
if ($rootView->is_dir($owner . '/files_versions/' . $ownerPath)) {
397397
if ($owner !== $user) {
398-
self::copy_recursive($owner . '/files_versions/' . $ownerPath, $owner . '/files_trashbin/versions/' . basename($ownerPath) . '.d' . $timestamp, $rootView);
398+
self::copy_recursive($owner . '/files_versions/' . $ownerPath, $owner . '/files_trashbin/versions/' . static::getTrashFilename(basename($ownerPath), $timestamp), $rootView);
399399
}
400-
self::move($rootView, $owner . '/files_versions/' . $ownerPath, $user . '/files_trashbin/versions/' . $filename . '.d' . $timestamp);
400+
self::move($rootView, $owner . '/files_versions/' . $ownerPath, $user . '/files_trashbin/versions/' . static::getTrashFilename($filename, $timestamp));
401401
} elseif ($versions = \OCA\Files_Versions\Storage::getVersions($owner, $ownerPath)) {
402402
foreach ($versions as $v) {
403403
if ($owner !== $user) {
404-
self::copy($rootView, $owner . '/files_versions' . $v['path'] . '.v' . $v['version'], $owner . '/files_trashbin/versions/' . $v['name'] . '.v' . $v['version'] . '.d' . $timestamp);
404+
self::copy($rootView, $owner . '/files_versions' . $v['path'] . '.v' . $v['version'], $owner . '/files_trashbin/versions/' . static::getTrashFilename($v['name'] . '.v' . $v['version'], $timestamp));
405405
}
406-
self::move($rootView, $owner . '/files_versions' . $v['path'] . '.v' . $v['version'], $user . '/files_trashbin/versions/' . $filename . '.v' . $v['version'] . '.d' . $timestamp);
406+
self::move($rootView, $owner . '/files_versions' . $v['path'] . '.v' . $v['version'], $user . '/files_trashbin/versions/' . static::getTrashFilename($filename . '.v' . $v['version'], $timestamp));
407407
}
408408
}
409409
}
@@ -561,7 +561,7 @@ private static function restoreVersions(View $view, $file, $filename, $uniqueFil
561561
} elseif ($versions = self::getVersionsFromTrash($versionedFile, $timestamp, $user)) {
562562
foreach ($versions as $v) {
563563
if ($timestamp) {
564-
$rootView->rename($user . '/files_trashbin/versions/' . $versionedFile . '.v' . $v . '.d' . $timestamp, $owner . '/files_versions/' . $ownerPath . '.v' . $v);
564+
$rootView->rename($user . '/files_trashbin/versions/' . static::getTrashFilename($versionedFile . '.v' . $v, $timestamp), $owner . '/files_versions/' . $ownerPath . '.v' . $v);
565565
} else {
566566
$rootView->rename($user . '/files_trashbin/versions/' . $versionedFile . '.v' . $v, $owner . '/files_versions/' . $ownerPath . '.v' . $v);
567567
}
@@ -662,7 +662,7 @@ public static function delete($filename, $user, $timestamp = null) {
662662
->andWhere($query->expr()->eq('timestamp', $query->createNamedParameter($timestamp)));
663663
$query->executeStatement();
664664

665-
$file = $filename . '.d' . $timestamp;
665+
$file = static::getTrashFilename($filename, $timestamp);
666666
} else {
667667
$file = $filename;
668668
}
@@ -705,8 +705,8 @@ private static function deleteVersions(View $view, $file, $filename, $timestamp,
705705
} elseif ($versions = self::getVersionsFromTrash($filename, $timestamp, $user)) {
706706
foreach ($versions as $v) {
707707
if ($timestamp) {
708-
$size += $view->filesize('/files_trashbin/versions/' . $filename . '.v' . $v . '.d' . $timestamp);
709-
$view->unlink('/files_trashbin/versions/' . $filename . '.v' . $v . '.d' . $timestamp);
708+
$size += $view->filesize('/files_trashbin/versions/' . static::getTrashFilename($filename . '.v' . $v, $timestamp));
709+
$view->unlink('/files_trashbin/versions/' . static::getTrashFilename($filename . '.v' . $v, $timestamp));
710710
} else {
711711
$size += $view->filesize('/files_trashbin/versions/' . $filename . '.v' . $v);
712712
$view->unlink('/files_trashbin/versions/' . $filename . '.v' . $v);
@@ -729,7 +729,7 @@ public static function file_exists($filename, $timestamp = null) {
729729
$view = new View('/' . $user);
730730

731731
if ($timestamp) {
732-
$filename = $filename . '.d' . $timestamp;
732+
$filename = static::getTrashFilename($filename, $timestamp);
733733
}
734734

735735
$target = Filesystem::normalizePath('files_trashbin/files/' . $filename);
@@ -1125,4 +1125,23 @@ public static function isEmpty($user) {
11251125
public static function preview_icon($path) {
11261126
return \OC::$server->getURLGenerator()->linkToRoute('core_ajax_trashbin_preview', ['x' => 32, 'y' => 32, 'file' => $path]);
11271127
}
1128+
1129+
/**
1130+
* Return the filename used in the trash bin
1131+
*/
1132+
public static function getTrashFilename(string $filename, int $timestamp): string {
1133+
$trashFilename = $filename . '.d' . $timestamp;
1134+
$length = strlen($trashFilename);
1135+
// oc_filecache `name` column has a limit of 250 chars
1136+
$maxLength = 250;
1137+
if ($length > $maxLength) {
1138+
$trashFilename = substr_replace(
1139+
$trashFilename,
1140+
'',
1141+
$maxLength / 2,
1142+
$length - $maxLength
1143+
);
1144+
}
1145+
return $trashFilename;
1146+
}
11281147
}

‎apps/files_trashbin/tests/StorageTest.php‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,11 @@ class StorageTest extends \Test\TestCase {
8888
*/
8989
private $userView;
9090

91+
// 239 chars so appended timestamp of 12 chars will exceed max length of 250 chars
92+
private const LONG_FILENAME = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.txt';
93+
// 250 chars
94+
private const MAX_FILENAME = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.txt';
95+
9196
protected function setUp(): void {
9297
parent::setUp();
9398

@@ -109,6 +114,8 @@ protected function setUp(): void {
109114
$this->rootView = new \OC\Files\View('/');
110115
$this->userView = new \OC\Files\View('/' . $this->user . '/files/');
111116
$this->userView->file_put_contents('test.txt', 'foo');
117+
$this->userView->file_put_contents(static::LONG_FILENAME, 'foo');
118+
$this->userView->file_put_contents(static::MAX_FILENAME, 'foo');
112119

113120
$this->userView->mkdir('folder');
114121
$this->userView->file_put_contents('folder/inside.txt', 'bar');
@@ -164,6 +171,44 @@ public function testSingleStorageDeleteFolder() {
164171
$this->assertEquals('inside.txt', $name);
165172
}
166173

174+
/**
175+
* Test that deleting a file with a long filename puts it into the trashbin.
176+
*/
177+
public function testSingleStorageDeleteLongFilename() {
178+
$truncatedFilename = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.txt';
179+
180+
$this->assertTrue($this->userView->file_exists(static::LONG_FILENAME));
181+
$this->userView->unlink(static::LONG_FILENAME);
182+
[$storage,] = $this->userView->resolvePath(static::LONG_FILENAME);
183+
$storage->getScanner()->scan(''); // make sure we check the storage
184+
$this->assertFalse($this->userView->getFileInfo(static::LONG_FILENAME));
185+
186+
// check if file is in trashbin
187+
$results = $this->rootView->getDirectoryContent($this->user . '/files_trashbin/files/');
188+
$this->assertEquals(1, count($results));
189+
$name = $results[0]->getName();
190+
$this->assertEquals($truncatedFilename, substr($name, 0, strrpos($name, '.')));
191+
}
192+
193+
/**
194+
* Test that deleting a file with the max filename length puts it into the trashbin.
195+
*/
196+
public function testSingleStorageDeleteMaxLengthFilename() {
197+
$truncatedFilename = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.txt';
198+
199+
$this->assertTrue($this->userView->file_exists(static::MAX_FILENAME));
200+
$this->userView->unlink(static::MAX_FILENAME);
201+
[$storage,] = $this->userView->resolvePath(static::MAX_FILENAME);
202+
$storage->getScanner()->scan(''); // make sure we check the storage
203+
$this->assertFalse($this->userView->getFileInfo(static::MAX_FILENAME));
204+
205+
// check if file is in trashbin
206+
$results = $this->rootView->getDirectoryContent($this->user . '/files_trashbin/files/');
207+
$this->assertEquals(1, count($results));
208+
$name = $results[0]->getName();
209+
$this->assertEquals($truncatedFilename, substr($name, 0, strrpos($name, '.')));
210+
}
211+
167212
/**
168213
* Test that deleting a file from another mounted storage properly
169214
* lands in the trashbin. This is a cross-storage situation because

0 commit comments

Comments
 (0)