Skip to content

Commit be36f97

Browse files
Pytalblizzz
Pytal
authored and
blizzz
committed
fix(trashbin): Truncate long filenames
Signed-off-by: Christopher Ng <[email protected]>
1 parent 5bfae1c commit be36f97

File tree

6 files changed

+88
-19
lines changed

6 files changed

+88
-19
lines changed

‎apps/dav/lib/CardDAV/SystemAddressbook.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public function getChildren() {
5252
}
5353

5454
public function getACL() {
55-
return array_filter(parent::getACL(), function($acl) {
55+
return array_filter(parent::getACL(), function ($acl) {
5656
if (in_array($acl['privilege'], ['{DAV:}write', '{DAV:}all'], true)) {
5757
return false;
5858
}

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,14 @@
2727

2828
namespace OCA\Files_Trashbin\Sabre;
2929

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

3537
public function getName(): string {
36-
return $this->data->getName() . '.d' . $this->getLastModified();
38+
return Trashbin::getTrashFilename($this->data->getName(), $this->getLastModified());
3739
}
3840
}

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,10 @@
2727

2828
namespace OCA\Files_Trashbin\Sabre;
2929

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

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

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

‎apps/files_trashbin/lib/Trashbin.php‎

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

209209
$view = new View('/');
210210

211-
$target = $user . '/files_trashbin/files/' . $targetFilename . '.d' . $timestamp;
212-
$source = $owner . '/files_trashbin/files/' . $sourceFilename . '.d' . $timestamp;
211+
$target = $user . '/files_trashbin/files/' . static::getTrashFilename($targetFilename, $timestamp);
212+
$source = $owner . '/files_trashbin/files/' . static::getTrashFilename($sourceFilename, $timestamp);
213213
$free = $view->free_space($target);
214214
$isUnknownOrUnlimitedFreeSpace = $free < 0;
215215
$isEnoughFreeSpaceLeft = $view->filesize($source) < $free;
@@ -276,7 +276,7 @@ public static function move2trash($file_path, $ownerOnly = false) {
276276
$lockingProvider = \OC::$server->getLockingProvider();
277277

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

282282
while (!$gotLock) {
@@ -292,7 +292,7 @@ public static function move2trash($file_path, $ownerOnly = false) {
292292

293293
$timestamp = $timestamp + 1;
294294

295-
$trashPath = '/files_trashbin/files/' . $filename . '.d' . $timestamp;
295+
$trashPath = '/files_trashbin/files/' . static::getTrashFilename($filename, $timestamp);
296296
}
297297
}
298298

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

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

@@ -394,15 +394,15 @@ private static function retainVersions($filename, $owner, $ownerPath, $timestamp
394394

395395
if ($rootView->is_dir($owner . '/files_versions/' . $ownerPath)) {
396396
if ($owner !== $user) {
397-
self::copy_recursive($owner . '/files_versions/' . $ownerPath, $owner . '/files_trashbin/versions/' . basename($ownerPath) . '.d' . $timestamp, $rootView);
397+
self::copy_recursive($owner . '/files_versions/' . $ownerPath, $owner . '/files_trashbin/versions/' . static::getTrashFilename(basename($ownerPath), $timestamp), $rootView);
398398
}
399-
self::move($rootView, $owner . '/files_versions/' . $ownerPath, $user . '/files_trashbin/versions/' . $filename . '.d' . $timestamp);
399+
self::move($rootView, $owner . '/files_versions/' . $ownerPath, $user . '/files_trashbin/versions/' . static::getTrashFilename($filename, $timestamp));
400400
} elseif ($versions = \OCA\Files_Versions\Storage::getVersions($owner, $ownerPath)) {
401401
foreach ($versions as $v) {
402402
if ($owner !== $user) {
403-
self::copy($rootView, $owner . '/files_versions' . $v['path'] . '.v' . $v['version'], $owner . '/files_trashbin/versions/' . $v['name'] . '.v' . $v['version'] . '.d' . $timestamp);
403+
self::copy($rootView, $owner . '/files_versions' . $v['path'] . '.v' . $v['version'], $owner . '/files_trashbin/versions/' . static::getTrashFilename($v['name'] . '.v' . $v['version'], $timestamp));
404404
}
405-
self::move($rootView, $owner . '/files_versions' . $v['path'] . '.v' . $v['version'], $user . '/files_trashbin/versions/' . $filename . '.v' . $v['version'] . '.d' . $timestamp);
405+
self::move($rootView, $owner . '/files_versions' . $v['path'] . '.v' . $v['version'], $user . '/files_trashbin/versions/' . static::getTrashFilename($filename . '.v' . $v['version'], $timestamp));
406406
}
407407
}
408408
}
@@ -560,7 +560,7 @@ private static function restoreVersions(View $view, $file, $filename, $uniqueFil
560560
} elseif ($versions = self::getVersionsFromTrash($versionedFile, $timestamp, $user)) {
561561
foreach ($versions as $v) {
562562
if ($timestamp) {
563-
$rootView->rename($user . '/files_trashbin/versions/' . $versionedFile . '.v' . $v . '.d' . $timestamp, $owner . '/files_versions/' . $ownerPath . '.v' . $v);
563+
$rootView->rename($user . '/files_trashbin/versions/' . static::getTrashFilename($versionedFile . '.v' . $v, $timestamp), $owner . '/files_versions/' . $ownerPath . '.v' . $v);
564564
} else {
565565
$rootView->rename($user . '/files_trashbin/versions/' . $versionedFile . '.v' . $v, $owner . '/files_versions/' . $ownerPath . '.v' . $v);
566566
}
@@ -661,7 +661,7 @@ public static function delete($filename, $user, $timestamp = null) {
661661
->andWhere($query->expr()->eq('timestamp', $query->createNamedParameter($timestamp)));
662662
$query->execute();
663663

664-
$file = $filename . '.d' . $timestamp;
664+
$file = static::getTrashFilename($filename, $timestamp);
665665
} else {
666666
$file = $filename;
667667
}
@@ -704,8 +704,8 @@ private static function deleteVersions(View $view, $file, $filename, $timestamp,
704704
} elseif ($versions = self::getVersionsFromTrash($filename, $timestamp, $user)) {
705705
foreach ($versions as $v) {
706706
if ($timestamp) {
707-
$size += $view->filesize('/files_trashbin/versions/' . $filename . '.v' . $v . '.d' . $timestamp);
708-
$view->unlink('/files_trashbin/versions/' . $filename . '.v' . $v . '.d' . $timestamp);
707+
$size += $view->filesize('/files_trashbin/versions/' . static::getTrashFilename($filename . '.v' . $v, $timestamp));
708+
$view->unlink('/files_trashbin/versions/' . static::getTrashFilename($filename . '.v' . $v, $timestamp));
709709
} else {
710710
$size += $view->filesize('/files_trashbin/versions/' . $filename . '.v' . $v);
711711
$view->unlink('/files_trashbin/versions/' . $filename . '.v' . $v);
@@ -728,7 +728,7 @@ public static function file_exists($filename, $timestamp = null) {
728728
$view = new View('/' . $user);
729729

730730
if ($timestamp) {
731-
$filename = $filename . '.d' . $timestamp;
731+
$filename = static::getTrashFilename($filename, $timestamp);
732732
}
733733

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

‎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

@@ -107,6 +112,8 @@ protected function setUp(): void {
107112
$this->rootView = new \OC\Files\View('/');
108113
$this->userView = new \OC\Files\View('/' . $this->user . '/files/');
109114
$this->userView->file_put_contents('test.txt', 'foo');
115+
$this->userView->file_put_contents(static::LONG_FILENAME, 'foo');
116+
$this->userView->file_put_contents(static::MAX_FILENAME, 'foo');
110117

111118
$this->userView->mkdir('folder');
112119
$this->userView->file_put_contents('folder/inside.txt', 'bar');
@@ -162,6 +169,44 @@ public function testSingleStorageDeleteFolder() {
162169
$this->assertEquals('inside.txt', $name);
163170
}
164171

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

0 commit comments

Comments
 (0)