fix: Enforce forbidden filename characters on backend

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

1 files changed, 48 insertions, 3 deletions

diff --git a/tests/lib/Files/Storage/CommonTest.php b/tests/lib/Files/Storage/CommonTest.php
index 0900765c510..3740289df95 100644
--- a/tests/lib/Files/Storage/CommonTest.php
+++ b/tests/lib/Files/Storage/CommonTest.php
@@ -24,6 +24,7 @@ namespace Test\Files\Storage;
 
 use OC\Files\Storage\Wrapper\Jail;
 use OC\Files\Storage\Wrapper\Wrapper;
+use OCP\Files\InvalidPathException;
 use PHPUnit\Framework\MockObject\MockObject;
 
 /**
@@ -39,22 +40,66 @@ class CommonTest extends Storage {
 	 */
 	private $tmpDir;
 
+	private array $invalidCharsBackup;
+
 	protected function setUp(): void {
 		parent::setUp();
 
 		$this->tmpDir = \OC::$server->getTempManager()->getTemporaryFolder();
 		$this->instance = new \OC\Files\Storage\CommonTest(['datadir' => $this->tmpDir]);
+		$this->invalidCharsBackup = \OC::$server->getConfig()->getSystemValue('forbidden_chars', []);
 	}
 
 	protected function tearDown(): void {
 		\OC_Helper::rmdirr($this->tmpDir);
+		\OC::$server->getConfig()->setSystemValue('forbidden_chars', $this->invalidCharsBackup);
 		parent::tearDown();
 	}
 
+	/**
+	 * @dataProvider dataVerifyPath
+	 */
+	public function testVerifyPath(string $filename, array $additionalChars, bool $throws) {
+		/** @var \OC\Files\Storage\CommonTest|MockObject $instance */
+		$instance = $this->getMockBuilder(\OC\Files\Storage\CommonTest::class)
+			->onlyMethods(['copyFromStorage', 'rmdir', 'unlink'])
+			->setConstructorArgs([['datadir' => $this->tmpDir]])
+			->getMock();
+		$instance->method('copyFromStorage')
+			->willThrowException(new \Exception('copy'));
+
+		\OC::$server->getConfig()->setSystemValue('forbidden_chars', $additionalChars);
+
+		if ($throws) {
+			$this->expectException(InvalidPathException::class);
+		} else {
+			$this->expectNotToPerformAssertions();
+		}
+		$instance->verifyPath('/', $filename);
+	}
+
+	public function dataVerifyPath(): array {
+		return [
+			// slash is always forbidden
+			'invalid slash' => ['a/b.txt', [], true],
+			// backslash is also forbidden
+			'invalid backslash' => ['a\\b.txt', [], true],
+			// by default colon is not forbidden
+			'valid name' => ['a: b.txt', [], false],
+			// colon can be added to the list of forbidden character
+			'invalid custom character' => ['a: b.txt', [':'], true],
+			// make sure to not split the list entries as they migh contain Unicode sequences
+			// in this example the "face in clouds" emoji contains the clouds emoji so only having clouds is ok
+			'valid unicode sequence' => ['🌫️.txt', ['😶‍🌫️'], false],
+			// This is the reverse: clouds are forbidden -> so is also the face in the clouds emoji
+			'valid unicode sequence' => ['😶‍🌫️.txt', ['🌫️'], true],
+		];
+	}
+
 	public function testMoveFromStorageWrapped() {
 		/** @var \OC\Files\Storage\CommonTest|MockObject $instance */
 		$instance = $this->getMockBuilder(\OC\Files\Storage\CommonTest::class)
-			->setMethods(['copyFromStorage', 'rmdir', 'unlink'])
+			->onlyMethods(['copyFromStorage', 'rmdir', 'unlink'])
 			->setConstructorArgs([['datadir' => $this->tmpDir]])
 			->getMock();
 		$instance->method('copyFromStorage')
@@ -72,7 +117,7 @@ class CommonTest extends Storage {
 	public function testMoveFromStorageJailed() {
 		/** @var \OC\Files\Storage\CommonTest|MockObject $instance */
 		$instance = $this->getMockBuilder(\OC\Files\Storage\CommonTest::class)
-			->setMethods(['copyFromStorage', 'rmdir', 'unlink'])
+			->onlyMethods(['copyFromStorage', 'rmdir', 'unlink'])
 			->setConstructorArgs([['datadir' => $this->tmpDir]])
 			->getMock();
 		$instance->method('copyFromStorage')
@@ -95,7 +140,7 @@ class CommonTest extends Storage {
 	public function testMoveFromStorageNestedJail() {
 		/** @var \OC\Files\Storage\CommonTest|MockObject $instance */
 		$instance = $this->getMockBuilder(\OC\Files\Storage\CommonTest::class)
-			->setMethods(['copyFromStorage', 'rmdir', 'unlink'])
+			->onlyMethods(['copyFromStorage', 'rmdir', 'unlink'])
 			->setConstructorArgs([['datadir' => $this->tmpDir]])
 			->getMock();
 		$instance->method('copyFromStorage')