fix(richobjectstrings): Add missing placeholder validation

Signed-off-by: Joas Schilling <coding@schilljs.com>

3 files changed, 72 insertions, 21 deletions

diff --git a/lib/private/RichObjectStrings/Validator.php b/lib/private/RichObjectStrings/Validator.php
index c7e4dcf50b9..8b099047221 100644
--- a/lib/private/RichObjectStrings/Validator.php
+++ b/lib/private/RichObjectStrings/Validator.php
@@ -1,4 +1,6 @@
 <?php
+
+declare(strict_types=1);
 /**
  * SPDX-FileCopyrightText: 2016 Nextcloud GmbH and Nextcloud contributors
  * SPDX-License-Identifier: AGPL-3.0-or-later
@@ -16,30 +18,22 @@ use OCP\RichObjectStrings\IValidator;
  * @since 11.0.0
  */
 class Validator implements IValidator {
-	/** @var Definitions */
-	protected $definitions;
-
-	/** @var array[] */
-	protected $requiredParameters = [];
+	protected array $requiredParameters = [];
 
-	/**
-	 * Constructor
-	 *
-	 * @param Definitions $definitions
-	 */
-	public function __construct(Definitions $definitions) {
-		$this->definitions = $definitions;
+	public function __construct(
+		protected Definitions $definitions,
+	) {
 	}
 
 	/**
 	 * @param string $subject
-	 * @param array[] $parameters
+	 * @param array<non-empty-string, array> $parameters
 	 * @throws InvalidObjectExeption
 	 * @since 11.0.0
 	 */
-	public function validate($subject, array $parameters) {
+	public function validate(string $subject, array $parameters): void {
 		$matches = [];
-		$result = preg_match_all('/\{([a-z0-9]+)\}/i', $subject, $matches);
+		$result = preg_match_all('/\{(' . self::PLACEHOLDER_REGEX . ')\}/', $subject, $matches);
 
 		if ($result === false) {
 			throw new InvalidObjectExeption();
@@ -53,7 +47,10 @@ class Validator implements IValidator {
 			}
 		}
 
-		foreach ($parameters as $parameter) {
+		foreach ($parameters as $placeholder => $parameter) {
+			if (!\is_string($placeholder) || !preg_match('/^(' . self::PLACEHOLDER_REGEX . ')$/i', $placeholder)) {
+				throw new InvalidObjectExeption('Parameter key is invalid');
+			}
 			if (!\is_array($parameter)) {
 				throw new InvalidObjectExeption('Parameter is malformed');
 			}
@@ -66,7 +63,7 @@ class Validator implements IValidator {
 	 * @param array $parameter
 	 * @throws InvalidObjectExeption
 	 */
-	protected function validateParameter(array $parameter) {
+	protected function validateParameter(array $parameter): void {
 		if (!isset($parameter['type'])) {
 			throw new InvalidObjectExeption('Object type is undefined');
 		}
@@ -94,7 +91,7 @@ class Validator implements IValidator {
 	 * @param array $definition
 	 * @return string[]
 	 */
-	protected function getRequiredParameters($type, array $definition) {
+	protected function getRequiredParameters(string $type, array $definition): array {
 		if (isset($this->requiredParameters[$type])) {
 			return $this->requiredParameters[$type];
 		}
diff --git a/lib/public/RichObjectStrings/IValidator.php b/lib/public/RichObjectStrings/IValidator.php
index 96b3b6ea743..c97eda1aa80 100644
--- a/lib/public/RichObjectStrings/IValidator.php
+++ b/lib/public/RichObjectStrings/IValidator.php
@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * SPDX-FileCopyrightText: 2016 Nextcloud GmbH and Nextcloud contributors
  * SPDX-License-Identifier: AGPL-3.0-or-later
@@ -12,10 +15,16 @@ namespace OCP\RichObjectStrings;
  */
 interface IValidator {
 	/**
+	 * Only alphanumeric, dash, underscore and got are allowed, starting with a character
+	 * @since 31.0.0
+	 */
+	public const PLACEHOLDER_REGEX = '[A-Za-z][A-Za-z0-9\-_.]+';
+
+	/**
 	 * @param string $subject
-	 * @param array[] $parameters
+	 * @param array<non-empty-string, array> $parameters
 	 * @throws InvalidObjectExeption
 	 * @since 11.0.0
 	 */
-	public function validate($subject, array $parameters);
+	public function validate(string $subject, array $parameters): void;
 }
diff --git a/tests/lib/RichObjectStrings/ValidatorTest.php b/tests/lib/RichObjectStrings/ValidatorTest.php
index e5230efe462..c5ce1f04dad 100644
--- a/tests/lib/RichObjectStrings/ValidatorTest.php
+++ b/tests/lib/RichObjectStrings/ValidatorTest.php
@@ -1,4 +1,6 @@
 <?php
+
+declare(strict_types=1);
 /**
  * SPDX-FileCopyrightText: 2016 Nextcloud GmbH and Nextcloud contributors
  * SPDX-License-Identifier: AGPL-3.0-or-later
@@ -12,7 +14,7 @@ use OCP\RichObjectStrings\InvalidObjectExeption;
 use Test\TestCase;
 
 class ValidatorTest extends TestCase {
-	public function test(): void {
+	public function testValidate(): void {
 		$v = new Validator(new Definitions());
 		$v->validate('test', []);
 		$v->validate('test {string1} test {foo} test {bar}.', [
@@ -57,4 +59,47 @@ class ValidatorTest extends TestCase {
 			],
 		]);
 	}
+
+	public static function dataValidateParameterKeys(): array {
+		return [
+			'not a string' => ['key' => 0, 'throws' => 'Parameter key is invalid'],
+			'@ is not allowed' => ['key' => 'user@0', 'throws' => 'Parameter key is invalid'],
+			'? is not allowed' => ['key' => 'user?0', 'throws' => 'Parameter key is invalid'],
+			'slash is not allowed' => ['key' => 'user/0', 'throws' => 'Parameter key is invalid'],
+			'backslash is not allowed' => ['key' => 'user\\0', 'throws' => 'Parameter key is invalid'],
+			'hash is not allowed' => ['key' => 'user#0', 'throws' => 'Parameter key is invalid'],
+			'space is not allowed' => ['key' => 'user 0', 'throws' => 'Parameter key is invalid'],
+			'has to start with letter, but is number' => ['key' => '0abc', 'throws' => 'Parameter key is invalid'],
+			'has to start with letter, but is dot' => ['key' => '.abc', 'throws' => 'Parameter key is invalid'],
+			'has to start with letter, but is slash' => ['key' => '-abc', 'throws' => 'Parameter key is invalid'],
+			'has to start with letter, but is underscore' => ['key' => '_abc', 'throws' => 'Parameter key is invalid'],
+			['key' => 'user-0', 'throws' => null],
+			['key' => 'user_0', 'throws' => null],
+			['key' => 'user.0', 'throws' => null],
+			['key' => 'a._-0', 'throws' => null],
+		];
+	}
+
+	/**
+	 * @dataProvider dataValidateParameterKeys
+	 */
+	public function testValidateParameterKeys(mixed $key, ?string $throws): void {
+
+		if ($throws !== null) {
+			$this->expectExceptionMessage($throws);
+		}
+
+		$v = new Validator(new Definitions());
+		$v->validate('{' . $key . '}', [
+			$key => [
+				'type' => 'highlight',
+				'id' => 'identifier',
+				'name' => 'Display name',
+			],
+		]);
+
+		if ($throws === null) {
+			$this->addToAssertionCount(1);
+		}
+	}
 }