fix(files): do not let a failing move hook break the move or hide files

Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com>
pull/61937/head
Git'Fellow 1 month ago
parent fb9a073571
commit 586e8a0c41
  1. 16
      lib/private/Files/Cache/Updater.php
  2. 4
      lib/private/legacy/OC_Hook.php
  3. 42
      tests/lib/Files/Cache/UpdaterTest.php
  4. 52
      tests/lib/LegacyHookTest.php

@ -123,6 +123,16 @@ class Updater implements IUpdater {
#[Override]
public function renameFromStorage(IStorage $sourceStorage, string $source, string $target): void {
$this->copyOrRenameFromStorage($sourceStorage, $source, $target, function (ICache $sourceCache) use ($sourceStorage, $source, $target): void {
$parent = dirname($target);
if ($parent === '.') {
$parent = '';
}
if (!$this->cache->inCache($parent)) {
// scan the parent first, moving the entry below a parent that is not in
// the cache would hide it from folder listings until the next scan
$this->scanner->scan($parent, Scanner::SCAN_SHALLOW, -1, false);
}
// Remove existing cache entry to no reuse the fileId.
if ($this->cache->inCache($target)) {
$this->cache->remove($target);
@ -181,6 +191,12 @@ class Updater implements IUpdater {
$isDir = $sourceInfo->getMimeType() === FileInfo::MIMETYPE_FOLDER;
} else {
if (!$this->storage->instanceOfStorage(ObjectStoreStorage::class) && !$this->cache->inCache($target)) {
// the source was not in the cache, so the operation could not transfer
// an entry to the target. Scan the target to not leave it invisible
// until the next scan
$this->scanner->scan($target, Scanner::SCAN_SHALLOW, -1, false);
}
$isDir = $this->storage->is_dir($target);
}

@ -85,7 +85,9 @@ class OC_Hook {
foreach (self::$registered[$signalClass][$signalName] as $i) {
try {
call_user_func([ $i['class'], $i['name'] ], $params);
} catch (Exception $e) {
} catch (Throwable $e) {
// a failing hook handler must not break the operation that emitted
// the signal, this includes Errors like TypeError
self::$thrownExceptions[] = $e;
Server::get(LoggerInterface::class)->error($e->getMessage(), ['exception' => $e]);
if ($e instanceof HintException) {

@ -256,6 +256,48 @@ class UpdaterTest extends \Test\TestCase {
$this->assertEquals($cached['fileid'], $cachedTarget['fileid']);
}
public function testMoveCrossStorageMissingTargetParent(): void {
$storage2 = new Temporary([]);
$cache2 = $storage2->getCache();
Filesystem::mount($storage2, [], '/bar');
$this->storage->file_put_contents('foo.txt', 'qwerty');
$this->updater->update('foo.txt');
$cached = $this->cache->get('foo.txt');
// the target parent exists on disk but is missing from the cache
$storage2->mkdir('sub');
$this->assertFalse($cache2->inCache('sub'));
$storage2->moveFromStorage($this->storage, 'foo.txt', 'sub/bar.txt');
$storage2->getUpdater()->renameFromStorage($this->storage, 'foo.txt', 'sub/bar.txt');
$this->assertFalse($this->cache->inCache('foo.txt'));
$this->assertTrue($cache2->inCache('sub'));
$this->assertTrue($cache2->inCache('sub/bar.txt'));
// the moved entry is attached to the scanned parent instead of being orphaned
$cachedTarget = $cache2->get('sub/bar.txt');
$this->assertEquals($cache2->getId('sub'), $cachedTarget['parent']);
$this->assertEquals($cached['fileid'], $cachedTarget['fileid']);
}
public function testMoveCrossStorageSourceNotInCache(): void {
$storage2 = new Temporary([]);
$cache2 = $storage2->getCache();
Filesystem::mount($storage2, [], '/bar');
$this->storage->file_put_contents('foo.txt', 'qwerty');
// the source was never scanned into the cache
$this->assertFalse($this->cache->inCache('foo.txt'));
$storage2->moveFromStorage($this->storage, 'foo.txt', 'bar.txt');
$storage2->getUpdater()->renameFromStorage($this->storage, 'foo.txt', 'bar.txt');
// the target still gets a cache entry
$this->assertTrue($cache2->inCache('bar.txt'));
$cachedTarget = $cache2->get('bar.txt');
$this->assertEquals(6, $cachedTarget['size']);
}
public function testMoveFolderCrossStorage(): void {
$storage2 = new Temporary([]);
$cache2 = $storage2->getCache();

@ -0,0 +1,52 @@
<?php
declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace Test;
use OCP\HintException;
class LegacyHookTest extends TestCase {
#[\Override]
protected function setUp(): void {
parent::setUp();
\OC_Hook::clear('LegacyHookTest');
}
#[\Override]
protected function tearDown(): void {
\OC_Hook::clear('LegacyHookTest');
// the exceptions thrown by the handlers below are expected, do not let
// the base test case rethrow them
\OC_Hook::$thrownExceptions = [];
parent::tearDown();
}
public static function throwTypeError(): void {
throw new \TypeError('type error thrown by a hook handler');
}
public static function throwHintException(): void {
throw new HintException('hint exception thrown by a hook handler');
}
public function testEmitDoesNotPropagateThrowable(): void {
\OC_Hook::connect('LegacyHookTest', 'error', self::class, 'throwTypeError');
$this->assertTrue(\OC_Hook::emit('LegacyHookTest', 'error'));
$this->assertCount(1, \OC_Hook::$thrownExceptions);
$this->assertInstanceOf(\TypeError::class, \OC_Hook::$thrownExceptions[0]);
}
public function testEmitRethrowsHintException(): void {
\OC_Hook::connect('LegacyHookTest', 'hint', self::class, 'throwHintException');
$this->expectException(HintException::class);
\OC_Hook::emit('LegacyHookTest', 'hint');
}
}
Loading…
Cancel
Save