Config Storage Prefix: Add basic validation

Summary: Closes T16142

Test Plan:
Try to set a very long value, like this, 100 characters:

./bin/config set storage.default-namespace looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooool

Enjoy the new validation message:

Usage Exception: Option "storage.default-namespace" is dangerously long for a database prefix in MySQL/MariaDB. The current value is 100 characters long but it should be less than 45 to be safe.

Try to set a value with a dot:

./bin/config set storage.default-namespace asd.asd

Enjoy the new validation message:

Usage Exception: Option "storage.default-namespace" only supports numbers,
letters, underscores and (for some reason) the dollar sign.
This is necessary to avoid potential MySQL/MariaDB escape issues.
Remove the invalid characters.

Set something good:

./bin/config set storage.default-namespace phabricator

No regressions.

Additionally, visit this page:

http://phorge.localhost/config/edit/storage.default-namespace/

No regressions.

Reviewers: O1 Blessed Committers, mainframe98

Reviewed By: O1 Blessed Committers, mainframe98

Subscribers: mainframe98, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T16142

Differential Revision: https://we.phorge.it/D26284

Valerio Bozzolan 8 months ago 07d543a6 529c7042

+75

4 changed files

expand all

src

__phutil_library_map__.php

applications

config

option

PhabricatorConfigOption.php

PhabricatorMySQLConfigOptions.php

type

PhabricatorDatabaseNamePrefixType.php

src/__phutil_library_map__.php

··· 3180 3180 'PhabricatorDashboardViewController' => 'applications/dashboard/controller/dashboard/PhabricatorDashboardViewController.php', 3181 3181 'PhabricatorDataCacheSpec' => 'applications/cache/spec/PhabricatorDataCacheSpec.php', 3182 3182 'PhabricatorDataNotAttachedException' => 'infrastructure/storage/lisk/PhabricatorDataNotAttachedException.php', 3183 + 'PhabricatorDatabaseNamePrefixType' => 'applications/config/type/PhabricatorDatabaseNamePrefixType.php', 3183 3184 'PhabricatorDatabaseRef' => 'infrastructure/cluster/PhabricatorDatabaseRef.php', 3184 3185 'PhabricatorDatabaseRefParser' => 'infrastructure/cluster/PhabricatorDatabaseRefParser.php', 3185 3186 'PhabricatorDatabaseSetupCheck' => 'applications/config/check/PhabricatorDatabaseSetupCheck.php', ··· 9631 9632 'PhabricatorDashboardViewController' => 'PhabricatorDashboardProfileController', 9632 9633 'PhabricatorDataCacheSpec' => 'PhabricatorCacheSpec', 9633 9634 'PhabricatorDataNotAttachedException' => 'Exception', 9635 + 'PhabricatorDatabaseNamePrefixType' => 'PhabricatorTextConfigType', 9634 9636 'PhabricatorDatabaseRef' => 'Phobject', 9635 9637 'PhabricatorDatabaseRefParser' => 'Phobject', 9636 9638 'PhabricatorDatabaseSetupCheck' => 'PhabricatorSetupCheck',

+23

src/applications/config/option/PhabricatorConfigOption.php

··· 126 126 throw new PhutilInvalidStateException('setEnumOptions'); 127 127 } 128 128 129 + /** 130 + * Set the config key. 131 + * 132 + * @param string $key 133 + * @return $this 134 + */ 129 135 public function setKey($key) { 130 136 $this->key = $key; 131 137 return $this; 132 138 } 133 139 140 + /** 141 + * Get the config key. 142 + * 143 + * @return string 144 + */ 134 145 public function getKey() { 135 146 return $this->key; 136 147 } ··· 176 187 return $this->description; 177 188 } 178 189 190 + /** 191 + * Set the type key. 192 + * 193 + * @param string $type Type key. 194 + * @return $this 195 + */ 179 196 public function setType($type) { 180 197 $this->type = $type; 181 198 return $this; 182 199 } 183 200 201 + /** 202 + * Get the type key. 203 + * 204 + * @param string $type Type key. 205 + * @return string|null 206 + */ 184 207 public function getType() { 185 208 return $this->type; 186 209 }

src/applications/config/option/PhabricatorMySQLConfigOptions.php

··· 36 36 ->setDescription( 37 37 pht('MySQL password to use when connecting to the database.')), 38 38 $this->newOption('storage.default-namespace', 'string', 'phabricator') 39 + ->setType(PhabricatorDatabaseNamePrefixType::TYPEKEY) 39 40 ->setLocked(true) 40 41 ->setSummary( 41 42 pht('The namespace that databases should use.'))

+49

src/applications/config/type/PhabricatorDatabaseNamePrefixType.php

··· 1 + <?php 2 + 3 + /** 4 + * Type describing a generic database name prefix. 5 + */ 6 + final class PhabricatorDatabaseNamePrefixType 7 + extends PhabricatorTextConfigType { 8 + 9 + const TYPEKEY = 'dbprefix'; 10 + 11 + public function validateStoredValue( 12 + PhabricatorConfigOption $option, 13 + $value) { 14 + 15 + // A long database prefix can lead to errors, since MySQL and MariaDB 16 + // only supports databases 64 characters long. 17 + // https://dev.mysql.com/doc/refman/9.4/en/identifier-length.html 18 + // The database '_differential' is an example of a long database suffix. 19 + // It has 13 characters. 20 + // This means the hard-limit for the prefix is 64 - 13 = 51 characters. 21 + // Let's stay a bit under this hard limit to stay safe. 22 + $max_length = 45; 23 + $len = strlen($value); 24 + if ($len >= $max_length) { 25 + throw $this->newException( 26 + pht( 27 + 'Option "%s" is dangerously long for a database prefix in '. 28 + 'MySQL/MariaDB. The current value is %d characters long. '. 29 + 'It should be less than %d to be safe for future changes.', 30 + $option->getKey(), 31 + $len, 32 + $max_length)); 33 + } 34 + 35 + // MySQL and MariaDB support these characters very well: 36 + // [0-9,a-z,A-Z$_] 37 + // https://dev.mysql.com/doc/refman/8.4/en/identifiers.html 38 + if (!phutil_preg_match('/^[0-9a-zA-Z$_]+$/', $value)) { 39 + throw $this->newException( 40 + pht( 41 + 'Option "%s" only supports numbers, letters, underscores '. 42 + 'and (for some reason) the dollar sign. This is necessary '. 43 + 'to avoid potential MySQL/MariaDB escape issues. '. 44 + 'Remove the invalid characters.', 45 + $option->getKey())); 46 + } 47 + } 48 + 49 + }

Configure Feed

Configure Feed