fix(IFilenameValidator): correctly handle case insensitivityfix/file-name-validator-case-sensitivity

- forbidden names and forbidden base names are case **insensitive**
  so we need to check all lowercase here.
- add test that config value is also read case insensitive.

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

2 files changed, 94 insertions, 6 deletions

diff --git a/lib/private/Files/FilenameValidator.php b/lib/private/Files/FilenameValidator.php
index fde45068df7..b1979789ec8 100644
--- a/lib/private/Files/FilenameValidator.php
+++ b/lib/private/Files/FilenameValidator.php
@@ -127,9 +127,6 @@ class FilenameValidator implements IFilenameValidator {
 		if (empty($this->forbiddenCharacters)) {
 			// Get always forbidden characters
 			$forbiddenCharacters = str_split(\OCP\Constants::FILENAME_INVALID_CHARS);
-			if ($forbiddenCharacters === false) {
-				$forbiddenCharacters = [];
-			}
 
 			// Get admin defined invalid characters
 			$additionalChars = $this->config->getSystemValue('forbidden_filename_characters', []);
@@ -231,7 +228,8 @@ class FilenameValidator implements IFilenameValidator {
 		return false;
 	}
 
-	protected function checkForbiddenName($filename): void {
+	protected function checkForbiddenName(string $filename): void {
+		$filename = mb_strtolower($filename);
 		if ($this->isForbidden($filename)) {
 			throw new ReservedWordException($this->l10n->t('"%1$s" is a forbidden file or folder name.', [$filename]));
 		}
@@ -295,6 +293,6 @@ class FilenameValidator implements IFilenameValidator {
 			$values = $fallback;
 		}
 
-		return array_map('mb_strtolower', $values);
+		return array_map(mb_strtolower(...), $values);
 	}
 };
diff --git a/tests/lib/Files/FilenameValidatorTest.php b/tests/lib/Files/FilenameValidatorTest.php
index ac9ac032b64..45f681bd453 100644
--- a/tests/lib/Files/FilenameValidatorTest.php
+++ b/tests/lib/Files/FilenameValidatorTest.php
@@ -146,6 +146,10 @@ class FilenameValidatorTest extends TestCase {
 				// needed for Windows namespaces
 				'com1.suffix', ['.htaccess'], ['com1'], [], [], ReservedWordException::class
 			],
+			'forbidden basename case insensitive' => [
+				// needed for Windows namespaces
+				'COM1.suffix', ['.htaccess'], ['com1'], [], [], ReservedWordException::class
+			],
 			'forbidden basename for hidden files' => [
 				// needed for Windows namespaces
 				'.thumbs.db', ['.htaccess'], ['.thumbs'], [], [], ReservedWordException::class
@@ -159,6 +163,9 @@ class FilenameValidatorTest extends TestCase {
 			'invalid extension' => [
 				'a: b.txt', ['.htaccess'], [], ['.txt'], [], InvalidPathException::class
 			],
+			'invalid extension case insensitive' => [
+				'a: b.TXT', ['.htaccess'], [], ['.txt'], [], InvalidPathException::class
+			],
 			'empty filename' => [
 				'', [], [], [], [], EmptyFileNameException::class
 			],
@@ -199,7 +206,6 @@ class FilenameValidatorTest extends TestCase {
 		return [
 			['plane 1 𐪅'],
 			['emoji 😶‍🌫️'],
-
 		];
 	}
 
@@ -284,4 +290,88 @@ class FilenameValidatorTest extends TestCase {
 			],
 		];
 	}
+
+	/**
+	 * @dataProvider dataGetForbiddenExtensions
+	 */
+	public function testGetForbiddenExtensions(array $configValue, array $expectedValue): void {
+		$validator = new FilenameValidator($this->l10n, $this->database, $this->config, $this->logger);
+		$this->config
+			// only once - then cached
+			->expects(self::once())
+			->method('getSystemValue')
+			->with('forbidden_filename_extensions', ['.filepart'])
+			->willReturn($configValue);
+
+		self::assertEqualsCanonicalizing($expectedValue, $validator->getForbiddenExtensions());
+	}
+
+	public static function dataGetForbiddenExtensions(): array {
+		return [
+			// default
+			[['.filepart'], ['.filepart', '.part']],
+			// always include .part
+			[[], ['.part']],
+			// handle case insensitivity
+			[['.TXT'], ['.txt', '.part']],
+		];
+	}
+
+	/**
+	 * @dataProvider dataGetForbiddenFilenames
+	 */
+	public function testGetForbiddenFilenames(array $configValue, array $legacyValue, array $expectedValue): void {
+		$validator = new FilenameValidator($this->l10n, $this->database, $this->config, $this->logger);
+		$this->config
+			// only once - then cached
+			->expects(self::exactly(2))
+			->method('getSystemValue')
+			->willReturnMap([
+				['forbidden_filenames', ['.htaccess'], $configValue],
+				['blacklisted_files', [], $legacyValue],
+			]);
+
+		$this->logger
+			->expects(empty($legacyValue) ? self::never() : self::once())
+			->method('warning');
+
+		self::assertEqualsCanonicalizing($expectedValue, $validator->getForbiddenFilenames());
+	}
+
+	public static function dataGetForbiddenFilenames(): array {
+		return [
+			// default
+			[['.htaccess'], [], ['.htaccess']],
+			// with legacy values
+			[['.htaccess'], ['legacy'], ['.htaccess', 'legacy']],
+			// handle case insensitivity
+			[['FileName', '.htaccess'], ['LegAcy'], ['.htaccess', 'filename', 'legacy']],
+		];
+	}
+
+	/**
+	 * @dataProvider dataGetForbiddenBasenames
+	 */
+	public function testGetForbiddenBasenames(array $configValue, array $expectedValue): void {
+		$validator = new FilenameValidator($this->l10n, $this->database, $this->config, $this->logger);
+		$this->config
+			// only once - then cached
+			->expects(self::once())
+			->method('getSystemValue')
+			->with('forbidden_filename_basenames', [])
+			->willReturn($configValue);
+
+		self::assertEqualsCanonicalizing($expectedValue, $validator->getForbiddenBasenames());
+	}
+
+	public static function dataGetForbiddenBasenames(): array {
+		return [
+			// default
+			[[], []],
+			// with values
+			[['aux', 'com0'], ['aux', 'com0']],
+			// handle case insensitivity
+			[['AuX', 'COM1'], ['aux', 'com1']],
+		];
+	}
 }