diff --git a/lib/private/Files/FilenameValidator.php b/lib/private/Files/FilenameValidator.php index 2fe3c93d02664..5972fdb9af1e1 100644 --- a/lib/private/Files/FilenameValidator.php +++ b/lib/private/Files/FilenameValidator.php @@ -27,6 +27,8 @@ class FilenameValidator implements IFilenameValidator { private IL10N $l10n; + private ?IDBConnection $database; + /** * @var list */ @@ -48,7 +50,6 @@ class FilenameValidator implements IFilenameValidator { public function __construct( IFactory $l10nFactory, - private IDBConnection $database, private IConfig $config, private LoggerInterface $logger, ) { @@ -187,6 +188,11 @@ public function validateFilename(string $filename): void { throw new FileNameTooLongException(); } + // We need to lazy load the database as otherwise there is a cyclic dependency + if (!isset($this->database)) { + $this->database = \OCP\Server::get(IDBConnection::class); + } + if (!$this->database->supports4ByteText()) { // verify database - e.g. mysql only 3-byte chars if (preg_match('%(?: diff --git a/tests/lib/Files/FilenameValidatorTest.php b/tests/lib/Files/FilenameValidatorTest.php index 1fcc03064a2a2..45013bf761ce4 100644 --- a/tests/lib/Files/FilenameValidatorTest.php +++ b/tests/lib/Files/FilenameValidatorTest.php @@ -24,12 +24,16 @@ use Psr\Log\LoggerInterface; use Test\TestCase; +/** + * @group DB + */ class FilenameValidatorTest extends TestCase { protected IFactory&MockObject $l10n; protected IConfig&MockObject $config; protected IDBConnection&MockObject $database; protected LoggerInterface&MockObject $logger; + protected bool $dbSupportsUtf8 = true; protected function setUp(): void { parent::setUp(); @@ -45,7 +49,13 @@ protected function setUp(): void { $this->config = $this->createMock(IConfig::class); $this->logger = $this->createMock(LoggerInterface::class); $this->database = $this->createMock(IDBConnection::class); - $this->database->method('supports4ByteText')->willReturn(true); + $this->database->method('supports4ByteText')->willReturnCallback(fn () => $this->dbSupportsUtf8); + $this->overwriteService(IDBConnection::class, $this->database); + } + + protected function tearDown(): void { + $this->restoreAllServices(); + parent::tearDown(); } /** @@ -67,7 +77,7 @@ public function testValidateFilename( 'getForbiddenExtensions', 'getForbiddenFilenames', ]) - ->setConstructorArgs([$this->l10n, $this->database, $this->config, $this->logger]) + ->setConstructorArgs([$this->l10n, $this->config, $this->logger]) ->getMock(); $validator->method('getForbiddenBasenames') @@ -106,7 +116,7 @@ public function testIsFilenameValid( 'getForbiddenFilenames', 'getForbiddenCharacters', ]) - ->setConstructorArgs([$this->l10n, $this->database, $this->config, $this->logger]) + ->setConstructorArgs([$this->l10n, $this->config, $this->logger]) ->getMock(); $validator->method('getForbiddenBasenames') @@ -186,12 +196,10 @@ public function dataValidateFilename(): array { * @dataProvider data4ByteUnicode */ public function testDatabaseDoesNotSupport4ByteText($filename): void { - $database = $this->createMock(IDBConnection::class); - $database->expects($this->once()) - ->method('supports4ByteText') - ->willReturn(false); + $this->dbSupportsUtf8 = false; + $this->expectException(InvalidCharacterInPathException::class); - $validator = new FilenameValidator($this->l10n, $database, $this->config, $this->logger); + $validator = new FilenameValidator($this->l10n, $this->config, $this->logger); $validator->validateFilename($filename); } @@ -199,7 +207,6 @@ public function data4ByteUnicode(): array { return [ ['plane 1 πͺ…'], ['emoji πŸ˜Άβ€πŸŒ«οΈ'], - ]; } @@ -208,7 +215,7 @@ public function data4ByteUnicode(): array { */ public function testInvalidAsciiCharactersAreAlwaysForbidden(string $filename): void { $this->expectException(InvalidPathException::class); - $validator = new FilenameValidator($this->l10n, $this->database, $this->config, $this->logger); + $validator = new FilenameValidator($this->l10n, $this->config, $this->logger); $validator->validateFilename($filename); } @@ -252,15 +259,13 @@ public function dataInvalidAsciiCharacters(): array { /** * @dataProvider dataIsForbidden */ - public function testIsForbidden(string $filename, array $forbiddenNames, array $forbiddenBasenames, bool $expected): void { + public function testIsForbidden(string $filename, array $forbiddenNames, bool $expected): void { /** @var FilenameValidator&MockObject */ $validator = $this->getMockBuilder(FilenameValidator::class) ->onlyMethods(['getForbiddenFilenames', 'getForbiddenBasenames']) - ->setConstructorArgs([$this->l10n, $this->database, $this->config, $this->logger]) + ->setConstructorArgs([$this->l10n, $this->config, $this->logger]) ->getMock(); - $validator->method('getForbiddenBasenames') - ->willReturn($forbiddenBasenames); $validator->method('getForbiddenFilenames') ->willReturn($forbiddenNames); @@ -270,27 +275,19 @@ public function testIsForbidden(string $filename, array $forbiddenNames, array $ public function dataIsForbidden(): array { return [ 'valid name' => [ - 'a: b.txt', ['.htaccess'], [], false + 'a: b.txt', ['.htaccess'], false ], 'valid name with some more parameters' => [ - 'a: b.txt', ['.htaccess'], [], false + 'a: b.txt', ['.htaccess'], false ], 'valid name as only full forbidden should be matched' => [ - '.htaccess.sample', ['.htaccess'], [], false, + '.htaccess.sample', ['.htaccess'], false, ], 'forbidden name' => [ - '.htaccess', ['.htaccess'], [], true + '.htaccess', ['.htaccess'], true ], 'forbidden name - name is case insensitive' => [ - 'COM1', ['.htaccess', 'com1'], [], true, - ], - 'forbidden name - basename is checked' => [ - // needed for Windows namespaces - 'com1.suffix', ['.htaccess'], ['com1'], true - ], - 'forbidden name - basename is checked also with multiple extensions' => [ - // needed for Windows namespaces - 'com1.tar.gz', ['.htaccess'], ['com1'], true + 'COM1', ['.htaccess', 'com1'], true, ], ]; } diff --git a/tests/lib/TestCase.php b/tests/lib/TestCase.php index 8c97c184c6fda..f6155cc1ee46b 100644 --- a/tests/lib/TestCase.php +++ b/tests/lib/TestCase.php @@ -508,9 +508,9 @@ protected function getGroupAnnotations(): array { } protected function IsDatabaseAccessAllowed() { - // on travis-ci.org we allow database access in any case - otherwise + // on CI we allow database access in any case - otherwise // this will break all apps right away - if (getenv('TRAVIS') == true) { + if (getenv('CI') == true) { return true; } $annotations = $this->getGroupAnnotations();