fix(dav): Pre-check if node can be copied or moved in DAV files plugin

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

5 files changed, 259 insertions, 199 deletions

diff --git a/apps/dav/lib/Connector/Sabre/FilesPlugin.php b/apps/dav/lib/Connector/Sabre/FilesPlugin.php
index 3289c913107..60da92e3bdf 100644
--- a/apps/dav/lib/Connector/Sabre/FilesPlugin.php
+++ b/apps/dav/lib/Connector/Sabre/FilesPlugin.php
@@ -8,8 +8,11 @@
 namespace OCA\DAV\Connector\Sabre;
 
 use OC\AppFramework\Http\Request;
+use OCA\DAV\Connector\Sabre\Exception\InvalidPath;
 use OCP\Constants;
 use OCP\Files\ForbiddenException;
+use OCP\Files\IFilenameValidator;
+use OCP\Files\InvalidPathException;
 use OCP\Files\StorageNotAvailableException;
 use OCP\FilesMetadata\Exceptions\FilesMetadataException;
 use OCP\FilesMetadata\Exceptions\FilesMetadataNotFoundException;
@@ -19,6 +22,7 @@ use OCP\IConfig;
 use OCP\IPreview;
 use OCP\IRequest;
 use OCP\IUserSession;
+use OCP\L10N\IFactory;
 use Sabre\DAV\Exception\Forbidden;
 use Sabre\DAV\Exception\NotFound;
 use Sabre\DAV\IFile;
@@ -64,33 +68,27 @@ class FilesPlugin extends ServerPlugin {
 
 	/** Reference to main server object */
 	private ?Server $server = null;
-	private Tree $tree;
-	private IUserSession $userSession;
 
 	/**
-	 * Whether this is public webdav.
-	 * If true, some returned information will be stripped off.
+	 * @param Tree $tree
+	 * @param IConfig $config
+	 * @param IRequest $request
+	 * @param IPreview $previewManager
+	 * @param IUserSession $userSession
+	 * @param bool $isPublic Whether this is public WebDAV. If true, some returned information will be stripped off.
+	 * @param bool $downloadAttachment
+	 * @return void
 	 */
-	private bool $isPublic;
-	private bool $downloadAttachment;
-	private IConfig $config;
-	private IRequest $request;
-	private IPreview $previewManager;
-
-	public function __construct(Tree $tree,
-		IConfig $config,
-		IRequest $request,
-		IPreview $previewManager,
-		IUserSession $userSession,
-		bool $isPublic = false,
-		bool $downloadAttachment = true) {
-		$this->tree = $tree;
-		$this->config = $config;
-		$this->request = $request;
-		$this->userSession = $userSession;
-		$this->isPublic = $isPublic;
-		$this->downloadAttachment = $downloadAttachment;
-		$this->previewManager = $previewManager;
+	public function __construct(
+		private Tree $tree,
+		private IConfig $config,
+		private IRequest $request,
+		private IPreview $previewManager,
+		private IUserSession $userSession,
+		private IFilenameValidator $validator,
+		private bool $isPublic = false,
+		private bool $downloadAttachment = true,
+	) {
 	}
 
 	/**
@@ -140,33 +138,67 @@ class FilesPlugin extends ServerPlugin {
 			}
 		});
 		$this->server->on('beforeMove', [$this, 'checkMove']);
+		$this->server->on('beforeCopy', [$this, 'checkCopy']);
 	}
 
 	/**
-	 * Plugin that checks if a move can actually be performed.
+	 * Plugin that checks if a copy can actually be performed.
 	 *
 	 * @param string $source source path
-	 * @param string $destination destination path
-	 * @throws Forbidden
-	 * @throws NotFound
+	 * @param string $target target path
+	 * @throws NotFound If the source does not exist
+	 * @throws InvalidPath If the target is invalid
 	 */
-	public function checkMove($source, $destination) {
+	public function checkCopy($source, $target): void {
 		$sourceNode = $this->tree->getNodeForPath($source);
 		if (!$sourceNode instanceof Node) {
 			return;
 		}
-		[$sourceDir,] = \Sabre\Uri\split($source);
-		[$destinationDir,] = \Sabre\Uri\split($destination);
 
-		if ($sourceDir !== $destinationDir) {
-			$sourceNodeFileInfo = $sourceNode->getFileInfo();
-			if ($sourceNodeFileInfo === null) {
-				throw new NotFound($source . ' does not exist');
+		// Ensure source exists
+		$sourceNodeFileInfo = $sourceNode->getFileInfo();
+		if ($sourceNodeFileInfo === null) {
+			throw new NotFound($source . ' does not exist');
+		}
+		// Ensure the target name is valid
+		try {
+			[$targetPath, $targetName] = \Sabre\Uri\split($target);
+			$this->validator->validateFilename($targetName);
+		} catch (InvalidPathException $e) {
+			throw new InvalidPath($e->getMessage(), false);
+		}
+		// Ensure the target path is valid
+		$segments = array_slice(explode('/', $targetPath), 2);
+		foreach ($segments as $segment) {
+			if ($this->validator->isFilenameValid($segment) === false) {
+				$l = \OCP\Server::get(IFactory::class)->get('dav');
+				throw new InvalidPath($l->t('Invalid target path'));
 			}
+		}
+	}
 
-			if (!$sourceNodeFileInfo->isDeletable()) {
-				throw new Forbidden($source . ' cannot be deleted');
-			}
+	/**
+	 * Plugin that checks if a move can actually be performed.
+	 *
+	 * @param string $source source path
+	 * @param string $target target path
+	 * @throws Forbidden If the source is not deletable
+	 * @throws NotFound If the source does not exist
+	 * @throws InvalidPath If the target name is invalid
+	 */
+	public function checkMove(string $source, string $target): void {
+		$sourceNode = $this->tree->getNodeForPath($source);
+		if (!$sourceNode instanceof Node) {
+			return;
+		}
+
+		// First check copyable (move only needs additional delete permission)
+		$this->checkCopy($source, $target);
+
+		// The source needs to be deletable for moving
+		$sourceNodeFileInfo = $sourceNode->getFileInfo();
+		if (!$sourceNodeFileInfo->isDeletable()) {
+			throw new Forbidden($source . ' cannot be deleted');
 		}
 	}
 
diff --git a/apps/dav/lib/Connector/Sabre/ServerFactory.php b/apps/dav/lib/Connector/Sabre/ServerFactory.php
index 89d9b86becc..fc35d78a2f7 100644
--- a/apps/dav/lib/Connector/Sabre/ServerFactory.php
+++ b/apps/dav/lib/Connector/Sabre/ServerFactory.php
@@ -13,6 +13,7 @@ use OCA\DAV\DAV\ViewOnlyPlugin;
 use OCA\DAV\Files\BrowserErrorPagePlugin;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Files\Folder;
+use OCP\Files\IFilenameValidator;
 use OCP\Files\Mount\IMountManager;
 use OCP\IConfig;
 use OCP\IDBConnection;
@@ -129,6 +130,7 @@ class ServerFactory {
 					$this->request,
 					$this->previewManager,
 					$this->userSession,
+					\OCP\Server::get(IFilenameValidator::class),
 					false,
 					!$this->config->getSystemValue('debug', false)
 				)
diff --git a/apps/dav/lib/Server.php b/apps/dav/lib/Server.php
index 4fdd70d05c7..449b8ef0ceb 100644
--- a/apps/dav/lib/Server.php
+++ b/apps/dav/lib/Server.php
@@ -53,9 +53,13 @@ use OCA\DAV\Upload\ChunkingV2Plugin;
 use OCP\AppFramework\Http\Response;
 use OCP\Diagnostics\IEventLogger;
 use OCP\EventDispatcher\IEventDispatcher;
+use OCP\Files\IFilenameValidator;
 use OCP\FilesMetadata\IFilesMetadataManager;
 use OCP\ICacheFactory;
+use OCP\IConfig;
+use OCP\IPreview;
 use OCP\IRequest;
+use OCP\IUserSession;
 use OCP\Profiler\IProfiler;
 use OCP\SabrePluginEvent;
 use Psr\Log\LoggerInterface;
@@ -236,15 +240,17 @@ class Server {
 			$user = $userSession->getUser();
 			if ($user !== null) {
 				$view = \OC\Files\Filesystem::getView();
+				$config = \OCP\Server::get(IConfig::class);
 				$this->server->addPlugin(
 					new FilesPlugin(
 						$this->server->tree,
-						\OC::$server->getConfig(),
+						$config,
 						$this->request,
-						\OC::$server->getPreviewManager(),
-						\OC::$server->getUserSession(),
+						\OCP\Server::get(IPreview::class),
+						\OCP\Server::get(IUserSession::class),
+						\OCP\Server::get(IFilenameValidator::class),
 						false,
-						!\OC::$server->getConfig()->getSystemValue('debug', false)
+						$config->getSystemValueBool('debug', false) === false,
 					)
 				);
 				$this->server->addPlugin(new ChecksumUpdatePlugin());
diff --git a/apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php
index 144b8af7659..9d8f4e8d4c4 100644
--- a/apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php
+++ b/apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php
@@ -9,10 +9,13 @@ namespace OCA\DAV\Tests\unit\Connector\Sabre;
 
 use OC\User\User;
 use OCA\DAV\Connector\Sabre\Directory;
+use OCA\DAV\Connector\Sabre\Exception\InvalidPath;
 use OCA\DAV\Connector\Sabre\File;
 use OCA\DAV\Connector\Sabre\FilesPlugin;
 use OCA\DAV\Connector\Sabre\Node;
 use OCP\Files\FileInfo;
+use OCP\Files\IFilenameValidator;
+use OCP\Files\InvalidPathException;
 use OCP\Files\StorageNotAvailableException;
 use OCP\IConfig;
 use OCP\IPreview;
@@ -32,51 +35,15 @@ use Test\TestCase;
  * @group DB
  */
 class FilesPluginTest extends TestCase {
-	public const GETETAG_PROPERTYNAME = FilesPlugin::GETETAG_PROPERTYNAME;
-	public const FILEID_PROPERTYNAME = FilesPlugin::FILEID_PROPERTYNAME;
-	public const INTERNAL_FILEID_PROPERTYNAME = FilesPlugin::INTERNAL_FILEID_PROPERTYNAME;
-	public const SIZE_PROPERTYNAME = FilesPlugin::SIZE_PROPERTYNAME;
-	public const PERMISSIONS_PROPERTYNAME = FilesPlugin::PERMISSIONS_PROPERTYNAME;
-	public const LASTMODIFIED_PROPERTYNAME = FilesPlugin::LASTMODIFIED_PROPERTYNAME;
-	public const CREATIONDATE_PROPERTYNAME = FilesPlugin::CREATIONDATE_PROPERTYNAME;
-	public const DOWNLOADURL_PROPERTYNAME = FilesPlugin::DOWNLOADURL_PROPERTYNAME;
-	public const OWNER_ID_PROPERTYNAME = FilesPlugin::OWNER_ID_PROPERTYNAME;
-	public const OWNER_DISPLAY_NAME_PROPERTYNAME = FilesPlugin::OWNER_DISPLAY_NAME_PROPERTYNAME;
-	public const DATA_FINGERPRINT_PROPERTYNAME = FilesPlugin::DATA_FINGERPRINT_PROPERTYNAME;
-	public const HAS_PREVIEW_PROPERTYNAME = FilesPlugin::HAS_PREVIEW_PROPERTYNAME;
 
-	/**
-	 * @var \Sabre\DAV\Server | \PHPUnit\Framework\MockObject\MockObject
-	 */
-	private $server;
-
-	/**
-	 * @var \Sabre\DAV\Tree | \PHPUnit\Framework\MockObject\MockObject
-	 */
-	private $tree;
-
-	/**
-	 * @var FilesPlugin
-	 */
-	private $plugin;
-
-	/**
-	 * @var \OCP\IConfig | \PHPUnit\Framework\MockObject\MockObject
-	 */
-	private $config;
-
-	/**
-	 * @var \OCP\IRequest | \PHPUnit\Framework\MockObject\MockObject
-	 */
-	private $request;
-
-	/**
-	 * @var \OCP\IPreview | \PHPUnit\Framework\MockObject\MockObject
-	 */
-	private $previewManager;
-
-	/** @var IUserSession|MockObject */
-	private $userSession;
+	private Tree&MockObject $tree;
+	private Server&MockObject $server;
+	private IConfig&MockObject $config;
+	private IRequest&MockObject $request;
+	private IPreview&MockObject $previewManager;
+	private IUserSession&MockObject $userSession;
+	private IFilenameValidator&MockObject $filenameValidator;
+	private FilesPlugin $plugin;
 
 	protected function setUp(): void {
 		parent::setUp();
@@ -89,13 +56,15 @@ class FilesPluginTest extends TestCase {
 		$this->request = $this->createMock(IRequest::class);
 		$this->previewManager = $this->createMock(IPreview::class);
 		$this->userSession = $this->createMock(IUserSession::class);
+		$this->filenameValidator = $this->createMock(IFilenameValidator::class);
 
 		$this->plugin = new FilesPlugin(
 			$this->tree,
 			$this->config,
 			$this->request,
 			$this->previewManager,
-			$this->userSession
+			$this->userSession,
+			$this->filenameValidator,
 		);
 
 		$response = $this->getMockBuilder(ResponseInterface::class)
@@ -160,16 +129,16 @@ class FilesPluginTest extends TestCase {
 		$propFind = new PropFind(
 			'/dummyPath',
 			[
-				self::GETETAG_PROPERTYNAME,
-				self::FILEID_PROPERTYNAME,
-				self::INTERNAL_FILEID_PROPERTYNAME,
-				self::SIZE_PROPERTYNAME,
-				self::PERMISSIONS_PROPERTYNAME,
-				self::DOWNLOADURL_PROPERTYNAME,
-				self::OWNER_ID_PROPERTYNAME,
-				self::OWNER_DISPLAY_NAME_PROPERTYNAME,
-				self::DATA_FINGERPRINT_PROPERTYNAME,
-				self::CREATIONDATE_PROPERTYNAME,
+				FilesPlugin::GETETAG_PROPERTYNAME,
+				FilesPlugin::FILEID_PROPERTYNAME,
+				FilesPlugin::INTERNAL_FILEID_PROPERTYNAME,
+				FilesPlugin::SIZE_PROPERTYNAME,
+				FilesPlugin::PERMISSIONS_PROPERTYNAME,
+				FilesPlugin::DOWNLOADURL_PROPERTYNAME,
+				FilesPlugin::OWNER_ID_PROPERTYNAME,
+				FilesPlugin::OWNER_DISPLAY_NAME_PROPERTYNAME,
+				FilesPlugin::DATA_FINGERPRINT_PROPERTYNAME,
+				FilesPlugin::CREATIONDATE_PROPERTYNAME,
 			],
 			0
 		);
@@ -197,16 +166,16 @@ class FilesPluginTest extends TestCase {
 			$node
 		);
 
-		$this->assertEquals('"abc"', $propFind->get(self::GETETAG_PROPERTYNAME));
-		$this->assertEquals('00000123instanceid', $propFind->get(self::FILEID_PROPERTYNAME));
-		$this->assertEquals('123', $propFind->get(self::INTERNAL_FILEID_PROPERTYNAME));
-		$this->assertEquals('1973-11-29T21:33:09+00:00', $propFind->get(self::CREATIONDATE_PROPERTYNAME));
-		$this->assertEquals(0, $propFind->get(self::SIZE_PROPERTYNAME));
-		$this->assertEquals('DWCKMSR', $propFind->get(self::PERMISSIONS_PROPERTYNAME));
-		$this->assertEquals('http://example.com/', $propFind->get(self::DOWNLOADURL_PROPERTYNAME));
-		$this->assertEquals('foo', $propFind->get(self::OWNER_ID_PROPERTYNAME));
-		$this->assertEquals('M. Foo', $propFind->get(self::OWNER_DISPLAY_NAME_PROPERTYNAME));
-		$this->assertEquals('my_fingerprint', $propFind->get(self::DATA_FINGERPRINT_PROPERTYNAME));
+		$this->assertEquals('"abc"', $propFind->get(FilesPlugin::GETETAG_PROPERTYNAME));
+		$this->assertEquals('00000123instanceid', $propFind->get(FilesPlugin::FILEID_PROPERTYNAME));
+		$this->assertEquals('123', $propFind->get(FilesPlugin::INTERNAL_FILEID_PROPERTYNAME));
+		$this->assertEquals('1973-11-29T21:33:09+00:00', $propFind->get(FilesPlugin::CREATIONDATE_PROPERTYNAME));
+		$this->assertEquals(0, $propFind->get(FilesPlugin::SIZE_PROPERTYNAME));
+		$this->assertEquals('DWCKMSR', $propFind->get(FilesPlugin::PERMISSIONS_PROPERTYNAME));
+		$this->assertEquals('http://example.com/', $propFind->get(FilesPlugin::DOWNLOADURL_PROPERTYNAME));
+		$this->assertEquals('foo', $propFind->get(FilesPlugin::OWNER_ID_PROPERTYNAME));
+		$this->assertEquals('M. Foo', $propFind->get(FilesPlugin::OWNER_DISPLAY_NAME_PROPERTYNAME));
+		$this->assertEquals('my_fingerprint', $propFind->get(FilesPlugin::DATA_FINGERPRINT_PROPERTYNAME));
 		$this->assertEquals([], $propFind->get404Properties());
 	}
 
@@ -217,7 +186,7 @@ class FilesPluginTest extends TestCase {
 		$propFind = new PropFind(
 			'/dummyPath',
 			[
-				self::DOWNLOADURL_PROPERTYNAME,
+				FilesPlugin::DOWNLOADURL_PROPERTYNAME,
 			],
 			0
 		);
@@ -231,25 +200,29 @@ class FilesPluginTest extends TestCase {
 			$node
 		);
 
-		$this->assertEquals(null, $propFind->get(self::DOWNLOADURL_PROPERTYNAME));
+		$this->assertEquals(null, $propFind->get(FilesPlugin::DOWNLOADURL_PROPERTYNAME));
 	}
 
 	public function testGetPublicPermissions(): void {
+		/** @var IRequest&MockObject */
+		$request = $this->getMockBuilder(IRequest::class)
+			->disableOriginalConstructor()
+			->getMock();
 		$this->plugin = new FilesPlugin(
 			$this->tree,
 			$this->config,
-			$this->getMockBuilder(IRequest::class)
-				->disableOriginalConstructor()
-				->getMock(),
+			$request,
 			$this->previewManager,
 			$this->userSession,
-			true);
+			$this->filenameValidator,
+			true,
+		);
 		$this->plugin->initialize($this->server);
 
 		$propFind = new PropFind(
 			'/dummyPath',
 			[
-				self::PERMISSIONS_PROPERTYNAME,
+				FilesPlugin::PERMISSIONS_PROPERTYNAME,
 			],
 			0
 		);
@@ -265,7 +238,7 @@ class FilesPluginTest extends TestCase {
 			$node
 		);
 
-		$this->assertEquals('DWCKR', $propFind->get(self::PERMISSIONS_PROPERTYNAME));
+		$this->assertEquals('DWCKR', $propFind->get(FilesPlugin::PERMISSIONS_PROPERTYNAME));
 	}
 
 	public function testGetPropertiesForDirectory(): void {
@@ -275,12 +248,12 @@ class FilesPluginTest extends TestCase {
 		$propFind = new PropFind(
 			'/dummyPath',
 			[
-				self::GETETAG_PROPERTYNAME,
-				self::FILEID_PROPERTYNAME,
-				self::SIZE_PROPERTYNAME,
-				self::PERMISSIONS_PROPERTYNAME,
-				self::DOWNLOADURL_PROPERTYNAME,
-				self::DATA_FINGERPRINT_PROPERTYNAME,
+				FilesPlugin::GETETAG_PROPERTYNAME,
+				FilesPlugin::FILEID_PROPERTYNAME,
+				FilesPlugin::SIZE_PROPERTYNAME,
+				FilesPlugin::PERMISSIONS_PROPERTYNAME,
+				FilesPlugin::DOWNLOADURL_PROPERTYNAME,
+				FilesPlugin::DATA_FINGERPRINT_PROPERTYNAME,
 			],
 			0
 		);
@@ -294,13 +267,13 @@ class FilesPluginTest extends TestCase {
 			$node
 		);
 
-		$this->assertEquals('"abc"', $propFind->get(self::GETETAG_PROPERTYNAME));
-		$this->assertEquals('00000123instanceid', $propFind->get(self::FILEID_PROPERTYNAME));
-		$this->assertEquals(1025, $propFind->get(self::SIZE_PROPERTYNAME));
-		$this->assertEquals('DWCKMSR', $propFind->get(self::PERMISSIONS_PROPERTYNAME));
-		$this->assertEquals(null, $propFind->get(self::DOWNLOADURL_PROPERTYNAME));
-		$this->assertEquals('my_fingerprint', $propFind->get(self::DATA_FINGERPRINT_PROPERTYNAME));
-		$this->assertEquals([self::DOWNLOADURL_PROPERTYNAME], $propFind->get404Properties());
+		$this->assertEquals('"abc"', $propFind->get(FilesPlugin::GETETAG_PROPERTYNAME));
+		$this->assertEquals('00000123instanceid', $propFind->get(FilesPlugin::FILEID_PROPERTYNAME));
+		$this->assertEquals(1025, $propFind->get(FilesPlugin::SIZE_PROPERTYNAME));
+		$this->assertEquals('DWCKMSR', $propFind->get(FilesPlugin::PERMISSIONS_PROPERTYNAME));
+		$this->assertEquals(null, $propFind->get(FilesPlugin::DOWNLOADURL_PROPERTYNAME));
+		$this->assertEquals('my_fingerprint', $propFind->get(FilesPlugin::DATA_FINGERPRINT_PROPERTYNAME));
+		$this->assertEquals([FilesPlugin::DOWNLOADURL_PROPERTYNAME], $propFind->get404Properties());
 	}
 
 	public function testGetPropertiesForRootDirectory(): void {
@@ -322,7 +295,7 @@ class FilesPluginTest extends TestCase {
 		$propFind = new PropFind(
 			'/',
 			[
-				self::DATA_FINGERPRINT_PROPERTYNAME,
+				FilesPlugin::DATA_FINGERPRINT_PROPERTYNAME,
 			],
 			0
 		);
@@ -332,7 +305,7 @@ class FilesPluginTest extends TestCase {
 			$node
 		);
 
-		$this->assertEquals('my_fingerprint', $propFind->get(self::DATA_FINGERPRINT_PROPERTYNAME));
+		$this->assertEquals('my_fingerprint', $propFind->get(FilesPlugin::DATA_FINGERPRINT_PROPERTYNAME));
 	}
 
 	public function testGetPropertiesWhenNoPermission(): void {
@@ -358,7 +331,7 @@ class FilesPluginTest extends TestCase {
 		$propFind = new PropFind(
 			'/test',
 			[
-				self::DATA_FINGERPRINT_PROPERTYNAME,
+				FilesPlugin::DATA_FINGERPRINT_PROPERTYNAME,
 			],
 			0
 		);
@@ -392,9 +365,9 @@ class FilesPluginTest extends TestCase {
 
 		// properties to set
 		$propPatch = new PropPatch([
-			self::GETETAG_PROPERTYNAME => 'newetag',
-			self::LASTMODIFIED_PROPERTYNAME => $testDate,
-			self::CREATIONDATE_PROPERTYNAME => $testCreationDate,
+			FilesPlugin::GETETAG_PROPERTYNAME => 'newetag',
+			FilesPlugin::LASTMODIFIED_PROPERTYNAME => $testDate,
+			FilesPlugin::CREATIONDATE_PROPERTYNAME => $testCreationDate,
 		]);
 
 
@@ -408,19 +381,19 @@ class FilesPluginTest extends TestCase {
 		$this->assertEmpty($propPatch->getRemainingMutations());
 
 		$result = $propPatch->getResult();
-		$this->assertEquals(200, $result[self::LASTMODIFIED_PROPERTYNAME]);
-		$this->assertEquals(200, $result[self::GETETAG_PROPERTYNAME]);
-		$this->assertEquals(200, $result[self::CREATIONDATE_PROPERTYNAME]);
+		$this->assertEquals(200, $result[FilesPlugin::LASTMODIFIED_PROPERTYNAME]);
+		$this->assertEquals(200, $result[FilesPlugin::GETETAG_PROPERTYNAME]);
+		$this->assertEquals(200, $result[FilesPlugin::CREATIONDATE_PROPERTYNAME]);
 	}
 
 	public function testUpdatePropsForbidden(): void {
 		$propPatch = new PropPatch([
-			self::OWNER_ID_PROPERTYNAME => 'user2',
-			self::OWNER_DISPLAY_NAME_PROPERTYNAME => 'User Two',
-			self::FILEID_PROPERTYNAME => 12345,
-			self::PERMISSIONS_PROPERTYNAME => 'C',
-			self::SIZE_PROPERTYNAME => 123,
-			self::DOWNLOADURL_PROPERTYNAME => 'http://example.com/',
+			FilesPlugin::OWNER_ID_PROPERTYNAME => 'user2',
+			FilesPlugin::OWNER_DISPLAY_NAME_PROPERTYNAME => 'User Two',
+			FilesPlugin::FILEID_PROPERTYNAME => 12345,
+			FilesPlugin::PERMISSIONS_PROPERTYNAME => 'C',
+			FilesPlugin::SIZE_PROPERTYNAME => 123,
+			FilesPlugin::DOWNLOADURL_PROPERTYNAME => 'http://example.com/',
 		]);
 
 		$this->plugin->handleUpdateProperties(
@@ -433,16 +406,16 @@ class FilesPluginTest extends TestCase {
 		$this->assertEmpty($propPatch->getRemainingMutations());
 
 		$result = $propPatch->getResult();
-		$this->assertEquals(403, $result[self::OWNER_ID_PROPERTYNAME]);
-		$this->assertEquals(403, $result[self::OWNER_DISPLAY_NAME_PROPERTYNAME]);
-		$this->assertEquals(403, $result[self::FILEID_PROPERTYNAME]);
-		$this->assertEquals(403, $result[self::PERMISSIONS_PROPERTYNAME]);
-		$this->assertEquals(403, $result[self::SIZE_PROPERTYNAME]);
-		$this->assertEquals(403, $result[self::DOWNLOADURL_PROPERTYNAME]);
+		$this->assertEquals(403, $result[FilesPlugin::OWNER_ID_PROPERTYNAME]);
+		$this->assertEquals(403, $result[FilesPlugin::OWNER_DISPLAY_NAME_PROPERTYNAME]);
+		$this->assertEquals(403, $result[FilesPlugin::FILEID_PROPERTYNAME]);
+		$this->assertEquals(403, $result[FilesPlugin::PERMISSIONS_PROPERTYNAME]);
+		$this->assertEquals(403, $result[FilesPlugin::SIZE_PROPERTYNAME]);
+		$this->assertEquals(403, $result[FilesPlugin::DOWNLOADURL_PROPERTYNAME]);
 	}
 
 	/**
-	 * Testcase from https://github.com/owncloud/core/issues/5251
+	 * Test case from https://github.com/owncloud/core/issues/5251
 	 *
 	 * |-FolderA
 	 *  |-text.txt
@@ -466,11 +439,12 @@ class FilesPluginTest extends TestCase {
 		$node = $this->getMockBuilder(Node::class)
 			->disableOriginalConstructor()
 			->getMock();
-		$node->expects($this->once())
+		$node->expects($this->atLeastOnce())
 			->method('getFileInfo')
 			->willReturn($fileInfoFolderATestTXT);
 
-		$this->tree->expects($this->once())->method('getNodeForPath')
+		$this->tree->expects($this->atLeastOnce())
+			->method('getNodeForPath')
 			->willReturn($node);
 
 		$this->plugin->checkMove('FolderA/test.txt', 'test.txt');
@@ -487,17 +461,17 @@ class FilesPluginTest extends TestCase {
 		$node = $this->getMockBuilder(Node::class)
 			->disableOriginalConstructor()
 			->getMock();
-		$node->expects($this->once())
+		$node->expects($this->atLeastOnce())
 			->method('getFileInfo')
 			->willReturn($fileInfoFolderATestTXT);
 
-		$this->tree->expects($this->once())->method('getNodeForPath')
+		$this->tree->expects($this->atLeastOnce())
+			->method('getNodeForPath')
 			->willReturn($node);
 
 		$this->plugin->checkMove('FolderA/test.txt', 'test.txt');
 	}
 
-
 	public function testMoveSrcNotExist(): void {
 		$this->expectException(\Sabre\DAV\Exception\NotFound::class);
 		$this->expectExceptionMessage('FolderA/test.txt does not exist');
@@ -505,16 +479,81 @@ class FilesPluginTest extends TestCase {
 		$node = $this->getMockBuilder(Node::class)
 			->disableOriginalConstructor()
 			->getMock();
-		$node->expects($this->once())
+		$node->expects($this->atLeastOnce())
 			->method('getFileInfo')
 			->willReturn(null);
 
-		$this->tree->expects($this->once())->method('getNodeForPath')
+		$this->tree->expects($this->atLeastOnce())
+			->method('getNodeForPath')
 			->willReturn($node);
 
 		$this->plugin->checkMove('FolderA/test.txt', 'test.txt');
 	}
 
+	public function testMoveDestinationInvalid(): void {
+		$this->expectException(InvalidPath::class);
+		$this->expectExceptionMessage('Mocked exception');
+
+		$fileInfoFolderATestTXT = $this->createMock(FileInfo::class);
+		$fileInfoFolderATestTXT->expects(self::any())
+			->method('isDeletable')
+			->willReturn(true);
+
+		$node = $this->createMock(Node::class);
+		$node->expects($this->atLeastOnce())
+			->method('getFileInfo')
+			->willReturn($fileInfoFolderATestTXT);
+
+		$this->tree->expects($this->atLeastOnce())
+			->method('getNodeForPath')
+			->willReturn($node);
+
+		$this->filenameValidator->expects(self::once())
+			->method('validateFilename')
+			->with('invalid\\path.txt')
+			->willThrowException(new InvalidPathException('Mocked exception'));
+
+		$this->plugin->checkMove('FolderA/test.txt', 'invalid\\path.txt');
+	}
+
+	public function testCopySrcNotExist(): void {
+		$this->expectException(\Sabre\DAV\Exception\NotFound::class);
+		$this->expectExceptionMessage('FolderA/test.txt does not exist');
+
+		$node = $this->createMock(Node::class);
+		$node->expects($this->atLeastOnce())
+			->method('getFileInfo')
+			->willReturn(null);
+
+		$this->tree->expects($this->atLeastOnce())
+			->method('getNodeForPath')
+			->willReturn($node);
+
+		$this->plugin->checkCopy('FolderA/test.txt', 'test.txt');
+	}
+
+	public function testCopyDestinationInvalid(): void {
+		$this->expectException(InvalidPath::class);
+		$this->expectExceptionMessage('Mocked exception');
+
+		$fileInfoFolderATestTXT = $this->createMock(FileInfo::class);
+		$node = $this->createMock(Node::class);
+		$node->expects($this->atLeastOnce())
+			->method('getFileInfo')
+			->willReturn($fileInfoFolderATestTXT);
+
+		$this->tree->expects($this->atLeastOnce())
+			->method('getNodeForPath')
+			->willReturn($node);
+
+		$this->filenameValidator->expects(self::once())
+			->method('validateFilename')
+			->with('invalid\\path.txt')
+			->willThrowException(new InvalidPathException('Mocked exception'));
+
+		$this->plugin->checkCopy('FolderA/test.txt', 'invalid\\path.txt');
+	}
+
 	public function downloadHeadersProvider() {
 		return [
 			[
@@ -581,7 +620,7 @@ class FilesPluginTest extends TestCase {
 		$propFind = new PropFind(
 			'/dummyPath',
 			[
-				self::HAS_PREVIEW_PROPERTYNAME
+				FilesPlugin::HAS_PREVIEW_PROPERTYNAME
 			],
 			0
 		);
@@ -595,6 +634,6 @@ class FilesPluginTest extends TestCase {
 			$node
 		);
 
-		$this->assertEquals('false', $propFind->get(self::HAS_PREVIEW_PROPERTYNAME));
+		$this->assertEquals('false', $propFind->get(FilesPlugin::HAS_PREVIEW_PROPERTYNAME));
 	}
 }
diff --git a/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php
index 95fd79fa5f1..76a70a93e13 100644
--- a/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php
+++ b/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php
@@ -14,6 +14,7 @@ use OCP\App\IAppManager;
 use OCP\Files\File;
 use OCP\Files\FileInfo;
 use OCP\Files\Folder;
+use OCP\Files\IFilenameValidator;
 use OCP\IConfig;
 use OCP\IGroupManager;
 use OCP\IPreview;
@@ -32,43 +33,20 @@ use Sabre\DAV\Tree;
 use Sabre\HTTP\ResponseInterface;
 
 class FilesReportPluginTest extends \Test\TestCase {
-	/** @var \Sabre\DAV\Server|MockObject */
-	private $server;
 
-	/** @var \Sabre\DAV\Tree|MockObject */
-	private $tree;
-
-	/** @var ISystemTagObjectMapper|MockObject */
-	private $tagMapper;
-
-	/** @var ISystemTagManager|MockObject */
-	private $tagManager;
-
-	/** @var ITags|MockObject */
-	private $privateTags;
-
-	private ITagManager|MockObject $privateTagManager;
-
-	/** @var \OCP\IUserSession */
-	private $userSession;
-
-	/** @var FilesReportPluginImplementation */
-	private $plugin;
-
-	/** @var View|MockObject * */
-	private $view;
-
-	/** @var IGroupManager|MockObject * */
-	private $groupManager;
-
-	/** @var Folder|MockObject * */
-	private $userFolder;
-
-	/** @var IPreview|MockObject * */
-	private $previewManager;
-
-	/** @var IAppManager|MockObject * */
-	private $appManager;
+	private \Sabre\DAV\Server&MockObject $server;
+	private Tree&MockObject $tree;
+	private ISystemTagObjectMapper&MockObject $tagMapper;
+	private ISystemTagManager&MockObject $tagManager;
+	private ITags&MockObject $privateTags;
+	private ITagManager&MockObject $privateTagManager;
+	private IUserSession&MockObject $userSession;
+	private FilesReportPluginImplementation $plugin;
+	private View&MockObject $view;
+	private IGroupManager&MockObject $groupManager;
+	private Folder&MockObject $userFolder;
+	private IPreview&MockObject $previewManager;
+	private IAppManager&MockObject $appManager;
 
 	protected function setUp(): void {
 		parent::setUp();
@@ -82,7 +60,7 @@ class FilesReportPluginTest extends \Test\TestCase {
 
 		$this->server = $this->getMockBuilder('\Sabre\DAV\Server')
 			->setConstructorArgs([$this->tree])
-			->setMethods(['getRequestUri', 'getBaseUri'])
+			->onlyMethods(['getRequestUri', 'getBaseUri'])
 			->getMock();
 
 		$this->server->expects($this->any())
@@ -311,7 +289,7 @@ class FilesReportPluginTest extends \Test\TestCase {
 				$filesNode2,
 			);
 
-		/** @var \OCA\DAV\Connector\Sabre\Directory|MockObject $reportTargetNode */
+		/** @var \OCA\DAV\Connector\Sabre\Directory&MockObject $reportTargetNode */
 		$result = $this->plugin->findNodesByFileIds($reportTargetNode, ['111', '222']);
 
 		$this->assertCount(2, $result);
@@ -364,7 +342,7 @@ class FilesReportPluginTest extends \Test\TestCase {
 				$filesNode2,
 			);
 
-		/** @var \OCA\DAV\Connector\Sabre\Directory|MockObject $reportTargetNode */
+		/** @var \OCA\DAV\Connector\Sabre\Directory&MockObject $reportTargetNode */
 		$result = $this->plugin->findNodesByFileIds($reportTargetNode, ['111', '222']);
 
 		$this->assertCount(2, $result);
@@ -409,13 +387,16 @@ class FilesReportPluginTest extends \Test\TestCase {
 			->disableOriginalConstructor()
 			->getMock();
 
+		$validator = $this->createMock(IFilenameValidator::class);
+
 		$this->server->addPlugin(
 			new \OCA\DAV\Connector\Sabre\FilesPlugin(
 				$this->tree,
 				$config,
 				$this->createMock(IRequest::class),
 				$this->previewManager,
-				$this->createMock(IUserSession::class)
+				$this->createMock(IUserSession::class),
+				$validator,
 			)
 		);
 		$this->plugin->initialize($this->server);