From a7aa03158de643bb7a6ba7dc9c36a3ae9d101bfa Mon Sep 17 00:00:00 2001 From: Samuel Stancl Date: Tue, 9 Jun 2026 18:38:56 -0700 Subject: [PATCH] refactor: only accept single values in validateParameter() this is to make handling null easier (previous Arr::wrap() approach turned null into an empty array rather than [null] requiring two separate null checks) and testing easier (we use empty arrays as examples of invalid values in some tests which would previously be accepted when passed individually as validateParameter([]) rather than being part of a wider [something, [], ...] array) also restrict passing null to validatePassword() also minor grammar fix in the validateParameter() docblock --- .../Concerns/ValidatesDatabaseParameters.php | 42 +++++++++---------- .../MySQLDatabaseManager.php | 4 +- ...olledMicrosoftSQLServerDatabaseManager.php | 3 +- ...rmissionControlledMySQLDatabaseManager.php | 3 +- ...ionControlledPostgreSQLDatabaseManager.php | 4 +- ...ssionControlledPostgreSQLSchemaManager.php | 4 +- 6 files changed, 33 insertions(+), 27 deletions(-) diff --git a/src/Database/Concerns/ValidatesDatabaseParameters.php b/src/Database/Concerns/ValidatesDatabaseParameters.php index b66a31c7..412af16a 100644 --- a/src/Database/Concerns/ValidatesDatabaseParameters.php +++ b/src/Database/Concerns/ValidatesDatabaseParameters.php @@ -38,40 +38,34 @@ trait ValidatesDatabaseParameters public static string $allowedPasswordCharacters = ' !#$%&()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[]^_abcdefghijklmnopqrstuvwxyz{|}~'; /** - * Ensure that parameters (database names, usernames, etc.) - * only contain allowed characters before used in SQL statements + * Ensure that parameter (database name, username, etc.) + * only contains allowed characters before being used in SQL statements * (or paths in the case of SQLiteDatabaseManager). * * By default, only the characters in allowedParameterCharacters() are allowed. * * @throws InvalidArgumentException */ - protected function validateParameter(string|array|null $parameters, string|null $allowedCharacters = null): void + protected function validateParameter(mixed $parameter, string|null $allowedCharacters = null): void { - if ($parameters === null) { + if (is_null($parameter)) { throw new InvalidArgumentException('Parameter cannot be null.'); } + if (is_numeric($parameter)) { + $parameter = (string) $parameter; + } + + if (! is_string($parameter)) { + // E.g. if a parameter is retrieved from the config, it isn't necessarily a string + throw new InvalidArgumentException('Parameter has to be a string.'); + } + $allowedCharacters ??= static::$allowedParameterCharacters; - foreach (Arr::wrap($parameters) as $parameter) { - if (is_null($parameter)) { - throw new InvalidArgumentException('Parameter cannot be null.'); - } - - if (is_numeric($parameter)) { - $parameter = (string) $parameter; - } - - if (! is_string($parameter)) { - // E.g. if a parameter is retrieved from the config, it isn't necessarily a string - throw new InvalidArgumentException('Parameter has to be a string.'); - } - - foreach (str_split($parameter) as $character) { - if (! str_contains($allowedCharacters, $character)) { - throw new InvalidArgumentException("Forbidden character '{$character}' in parameter."); - } + foreach (str_split($parameter) as $character) { + if (! str_contains($allowedCharacters, $character)) { + throw new InvalidArgumentException("Forbidden character '{$character}' in parameter."); } } } @@ -87,6 +81,10 @@ trait ValidatesDatabaseParameters */ protected function validatePassword(string|null $password): void { + if (is_null($password)) { + throw new InvalidArgumentException('Parameter cannot be null.'); + } + $this->validateParameter($password, allowedCharacters: static::$allowedPasswordCharacters); } } diff --git a/src/Database/TenantDatabaseManagers/MySQLDatabaseManager.php b/src/Database/TenantDatabaseManagers/MySQLDatabaseManager.php index 912533b8..10385208 100644 --- a/src/Database/TenantDatabaseManagers/MySQLDatabaseManager.php +++ b/src/Database/TenantDatabaseManagers/MySQLDatabaseManager.php @@ -14,7 +14,7 @@ class MySQLDatabaseManager extends TenantDatabaseManager $charset = $this->connection()->getConfig('charset'); $collation = $this->connection()->getConfig('collation'); - $this->validateParameter(array_filter([$database, $charset, $collation], fn ($param) => $param !== null)); + $this->validateParameter($database); // MySQL defaults to the server's charset and collation // if charset and collation are not specified. @@ -23,10 +23,12 @@ class MySQLDatabaseManager extends TenantDatabaseManager $statement = "CREATE DATABASE `{$database}`"; if ($charset !== null) { + $this->validateParameter($charset); $statement .= " CHARACTER SET `{$charset}`"; } if ($collation !== null) { + $this->validateParameter($collation); $statement .= " COLLATE `{$collation}`"; } diff --git a/src/Database/TenantDatabaseManagers/PermissionControlledMicrosoftSQLServerDatabaseManager.php b/src/Database/TenantDatabaseManagers/PermissionControlledMicrosoftSQLServerDatabaseManager.php index cbc43d18..b10b82bc 100644 --- a/src/Database/TenantDatabaseManagers/PermissionControlledMicrosoftSQLServerDatabaseManager.php +++ b/src/Database/TenantDatabaseManagers/PermissionControlledMicrosoftSQLServerDatabaseManager.php @@ -24,7 +24,8 @@ class PermissionControlledMicrosoftSQLServerDatabaseManager extends MicrosoftSQL $username = $databaseConfig->getUsername(); $password = $databaseConfig->getPassword(); - $this->validateParameter([$database, $username]); + $this->validateParameter($database); + $this->validateParameter($username); $this->validatePassword($password); // Create login diff --git a/src/Database/TenantDatabaseManagers/PermissionControlledMySQLDatabaseManager.php b/src/Database/TenantDatabaseManagers/PermissionControlledMySQLDatabaseManager.php index a4f74f8f..421b3bc3 100644 --- a/src/Database/TenantDatabaseManagers/PermissionControlledMySQLDatabaseManager.php +++ b/src/Database/TenantDatabaseManagers/PermissionControlledMySQLDatabaseManager.php @@ -25,7 +25,8 @@ class PermissionControlledMySQLDatabaseManager extends MySQLDatabaseManager impl $username = $databaseConfig->getUsername(); $password = $databaseConfig->getPassword(); - $this->validateParameter([$database, $username]); + $this->validateParameter($database); + $this->validateParameter($username); $this->validatePassword($password); $this->connection()->statement("CREATE USER `{$username}`@`%` IDENTIFIED BY '{$password}'"); diff --git a/src/Database/TenantDatabaseManagers/PermissionControlledPostgreSQLDatabaseManager.php b/src/Database/TenantDatabaseManagers/PermissionControlledPostgreSQLDatabaseManager.php index 6b4856b0..c846ace9 100644 --- a/src/Database/TenantDatabaseManagers/PermissionControlledPostgreSQLDatabaseManager.php +++ b/src/Database/TenantDatabaseManagers/PermissionControlledPostgreSQLDatabaseManager.php @@ -20,7 +20,9 @@ class PermissionControlledPostgreSQLDatabaseManager extends PostgreSQLDatabaseMa $username = $databaseConfig->getUsername(); $schema = $databaseConfig->connection()['search_path']; - $this->validateParameter([$database, $username, $schema]); + $this->validateParameter($database); + $this->validateParameter($username); + $this->validateParameter($schema); // Host config $connectionName = $this->connection()->getConfig('name'); diff --git a/src/Database/TenantDatabaseManagers/PermissionControlledPostgreSQLSchemaManager.php b/src/Database/TenantDatabaseManagers/PermissionControlledPostgreSQLSchemaManager.php index 057726b0..b972ba0b 100644 --- a/src/Database/TenantDatabaseManagers/PermissionControlledPostgreSQLSchemaManager.php +++ b/src/Database/TenantDatabaseManagers/PermissionControlledPostgreSQLSchemaManager.php @@ -23,7 +23,9 @@ class PermissionControlledPostgreSQLSchemaManager extends PostgreSQLSchemaManage // Central database name $database = DB::connection(config('tenancy.database.central_connection'))->getDatabaseName(); - $this->validateParameter([$username, $schema, $database]); + $this->validateParameter($username); + $this->validateParameter($schema); + $this->validateParameter($database); $this->connection()->statement("GRANT CONNECT ON DATABASE \"{$database}\" TO \"{$username}\""); $this->connection()->statement("GRANT USAGE, CREATE ON SCHEMA \"{$schema}\" TO \"{$username}\"");