Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[stable26] fix(trashbin): Truncate long filenames #38573

Merged
merged 1 commit into from Jun 5, 2023
Merged

[stable26] fix(trashbin): Truncate long filenames #38573

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/files_trashbin/lib/Command/RestoreAllFiles.php
View file
Open in desktop
Expand Up @@ -146,7 +146,7 @@ protected function restoreDeletedFiles(string $uid, OutputInterface $output): vo
$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 = '';
Expand Down
6 changes: 4 additions & 2 deletions apps/files_trashbin/lib/Sabre/TrashFile.php
View file
Open in desktop
Expand Up @@ -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());
}
}
4 changes: 3 additions & 1 deletion apps/files_trashbin/lib/Sabre/TrashFolder.php
View file
Open in desktop
Expand Up @@ -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());
}
}
3 changes: 2 additions & 1 deletion apps/files_trashbin/lib/Trash/LegacyTrashBackend.php
View file
Open in desktop
Expand Up @@ -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
);
Expand Down
47 changes: 33 additions & 14 deletions apps/files_trashbin/lib/Trashbin.php
View file
Open in desktop
Expand Up @@ -203,8 +203,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;
Expand Down Expand Up @@ -278,7 +278,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) {
Expand All @@ -294,7 +294,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);
}
}

Expand Down Expand Up @@ -358,7 +358,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);

Expand Down Expand Up @@ -395,15 +395,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));
}
}
}
Expand Down Expand Up @@ -561,7 +561,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);
}
Expand Down Expand Up @@ -662,7 +662,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;
}
Expand Down Expand Up @@ -705,8 +705,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);
Expand All @@ -729,7 +729,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);
Expand Down Expand Up @@ -1125,4 +1125,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;
}
}
45 changes: 45 additions & 0 deletions apps/files_trashbin/tests/StorageTest.php
View file
Open in desktop
Expand Up @@ -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();

Expand All @@ -109,6 +114,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');
Expand Down Expand Up @@ -164,6 +171,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
Expand Down