1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-25 16:22:43 +01:00

Fix a PHP 8.1 strlen() issue with "mysql.pass" configuration

Summary:
Ref T13588. This configuration value may not be set.

Also fix an issue in `bin/storage` and whatever else I hit between now and this diff actually uploading.

Also fix a MySQLi report mode difference, beginning in PHP 8.1.

Also update a bunch of "static" property usage in Lisk.

Test Plan: Ran `bin/files ...` locally under PHP 8.1.

Maniphest Tasks: T13588

Differential Revision: https://secure.phabricator.com/D21744
This commit is contained in:
epriestley 2021-12-16 14:07:59 -08:00
parent dc705cea7f
commit 6136964093
13 changed files with 81 additions and 60 deletions

View file

@ -79,7 +79,8 @@
"xhpast": { "xhpast": {
"type": "xhpast", "type": "xhpast",
"include": "(\\.php$)", "include": "(\\.php$)",
"standard": "phutil.xhpast" "standard": "phutil.xhpast",
"xhpast.php-version": "5.5"
} }
} }
} }

View file

@ -95,7 +95,7 @@ if (!$refs) {
$host = $args->getArg('host'); $host = $args->getArg('host');
$ref_key = $args->getArg('ref'); $ref_key = $args->getArg('ref');
if (strlen($host) || strlen($ref_key)) { if (($host !== null) || ($ref_key !== null)) {
if ($host && $ref_key) { if ($host && $ref_key) {
throw new PhutilArgumentUsageException( throw new PhutilArgumentUsageException(
pht( pht(

View file

@ -26,6 +26,7 @@ final class PhabricatorLocalDiskFileStorageEngine
public function canWriteFiles() { public function canWriteFiles() {
$path = PhabricatorEnv::getEnvConfig('storage.local-disk.path'); $path = PhabricatorEnv::getEnvConfig('storage.local-disk.path');
$path = phutil_string_cast($path);
return (bool)strlen($path); return (bool)strlen($path);
} }

View file

@ -89,7 +89,7 @@ final class PhabricatorApplicationQuery
} }
} }
if (strlen($this->nameContains)) { if ($this->nameContains !== null) {
foreach ($apps as $key => $app) { foreach ($apps as $key => $app) {
if (stripos($app->getName(), $this->nameContains) === false) { if (stripos($app->getName(), $this->nameContains) === false) {
unset($apps[$key]); unset($apps[$key]);

View file

@ -341,7 +341,7 @@ final class PhabricatorPeopleQuery
(int)$this->isMailingList); (int)$this->isMailingList);
} }
if (strlen($this->nameLike)) { if ($this->nameLike !== null) {
$where[] = qsprintf( $where[] = qsprintf(
$conn, $conn,
'user.username LIKE %~ OR user.realname LIKE %~', 'user.username LIKE %~ OR user.realname LIKE %~',

View file

@ -275,7 +275,8 @@ final class PhabricatorUser
$this->setConduitCertificate($this->generateConduitCertificate()); $this->setConduitCertificate($this->generateConduitCertificate());
} }
if (!strlen($this->getAccountSecret())) { $secret = $this->getAccountSecret();
if (($secret === null) || !strlen($secret)) {
$this->setAccountSecret(Filesystem::readRandomCharacters(64)); $this->setAccountSecret(Filesystem::readRandomCharacters(64));
} }

View file

@ -126,22 +126,27 @@ final class PhabricatorHandleList
/* -( Iterator )----------------------------------------------------------- */ /* -( Iterator )----------------------------------------------------------- */
#[\ReturnTypeWillChange]
public function rewind() { public function rewind() {
$this->cursor = 0; $this->cursor = 0;
} }
#[\ReturnTypeWillChange]
public function current() { public function current() {
return $this->getHandle($this->phids[$this->cursor]); return $this->getHandle($this->phids[$this->cursor]);
} }
#[\ReturnTypeWillChange]
public function key() { public function key() {
return $this->phids[$this->cursor]; return $this->phids[$this->cursor];
} }
#[\ReturnTypeWillChange]
public function next() { public function next() {
++$this->cursor; ++$this->cursor;
} }
#[\ReturnTypeWillChange]
public function valid() { public function valid() {
return ($this->cursor < $this->count); return ($this->cursor < $this->count);
} }
@ -150,6 +155,7 @@ final class PhabricatorHandleList
/* -( ArrayAccess )-------------------------------------------------------- */ /* -( ArrayAccess )-------------------------------------------------------- */
#[\ReturnTypeWillChange]
public function offsetExists($offset) { public function offsetExists($offset) {
// NOTE: We're intentionally not loading handles here so that isset() // NOTE: We're intentionally not loading handles here so that isset()
// checks do not trigger fetches. This gives us better bulk loading // checks do not trigger fetches. This gives us better bulk loading
@ -162,6 +168,7 @@ final class PhabricatorHandleList
return isset($this->map[$offset]); return isset($this->map[$offset]);
} }
#[\ReturnTypeWillChange]
public function offsetGet($offset) { public function offsetGet($offset) {
if ($this->handles === null) { if ($this->handles === null) {
$this->loadHandles(); $this->loadHandles();
@ -169,10 +176,12 @@ final class PhabricatorHandleList
return $this->handles[$offset]; return $this->handles[$offset];
} }
#[\ReturnTypeWillChange]
public function offsetSet($offset, $value) { public function offsetSet($offset, $value) {
$this->raiseImmutableException(); $this->raiseImmutableException();
} }
#[\ReturnTypeWillChange]
public function offsetUnset($offset) { public function offsetUnset($offset) {
$this->raiseImmutableException(); $this->raiseImmutableException();
} }
@ -189,6 +198,7 @@ final class PhabricatorHandleList
/* -( Countable )---------------------------------------------------------- */ /* -( Countable )---------------------------------------------------------- */
#[\ReturnTypeWillChange]
public function count() { public function count() {
return $this->count; return $this->count;
} }

View file

@ -322,6 +322,7 @@ final class PhabricatorDatabaseRef
$default_user = PhabricatorEnv::getEnvConfig('mysql.user'); $default_user = PhabricatorEnv::getEnvConfig('mysql.user');
$default_pass = PhabricatorEnv::getEnvConfig('mysql.pass'); $default_pass = PhabricatorEnv::getEnvConfig('mysql.pass');
$default_pass = phutil_string_cast($default_pass);
$default_pass = new PhutilOpaqueEnvelope($default_pass); $default_pass = new PhutilOpaqueEnvelope($default_pass);
$config = PhabricatorEnv::getEnvConfig('cluster.databases'); $config = PhabricatorEnv::getEnvConfig('cluster.databases');

View file

@ -87,7 +87,7 @@ abstract class PhabricatorQuery extends Phobject {
foreach ($this->flattenSubclause($part) as $subpart) { foreach ($this->flattenSubclause($part) as $subpart) {
$result[] = $subpart; $result[] = $subpart;
} }
} else if (strlen($part)) { } else if (($part !== null) && strlen($part)) {
$result[] = $part; $result[] = $part;
} }
} }

View file

@ -57,6 +57,13 @@ final class AphrontMySQLiDatabaseConnection
} }
} }
// See T13588. In PHP 8.1, the default "report mode" for MySQLi has
// changed, which causes MySQLi to raise exceptions. Disable exceptions
// to align behavior with older default behavior under MySQLi, which
// this code expects. Plausibly, this code could be updated to use
// MySQLi exceptions to handle errors under a wider range of PHP versions.
mysqli_report(MYSQLI_REPORT_OFF);
$conn = mysqli_init(); $conn = mysqli_init();
$timeout = $this->getConfiguration('timeout'); $timeout = $this->getConfiguration('timeout');

View file

@ -193,6 +193,8 @@ abstract class LiskDAO extends Phobject
private static $connections = array(); private static $connections = array();
private static $liskMetadata = array();
protected $id; protected $id;
protected $phid; protected $phid;
protected $dateCreated; protected $dateCreated;
@ -403,10 +405,11 @@ abstract class LiskDAO extends Phobject
* @task config * @task config
*/ */
public function getConfigOption($option_name) { public function getConfigOption($option_name) {
static $options = null; $options = $this->getLiskMetadata('config');
if (!isset($options)) { if ($options === null) {
$options = $this->getConfiguration(); $options = $this->getConfiguration();
$this->setLiskMetadata('config', $options);
} }
return idx($options, $option_name); return idx($options, $option_name);
@ -439,7 +442,7 @@ abstract class LiskDAO extends Phobject
return $this->loadOneWhere( return $this->loadOneWhere(
'%C = %d', '%C = %d',
$this->getIDKeyForUse(), $this->getIDKey(),
$id); $id);
} }
@ -554,7 +557,7 @@ abstract class LiskDAO extends Phobject
$result = $this->loadOneWhere( $result = $this->loadOneWhere(
'%C = %d', '%C = %d',
$this->getIDKeyForUse(), $this->getIDKey(),
$this->getID()); $this->getID());
if (!$result) { if (!$result) {
@ -579,9 +582,10 @@ abstract class LiskDAO extends Phobject
* @task load * @task load
*/ */
public function loadFromArray(array $row) { public function loadFromArray(array $row) {
static $valid_properties = array(); $valid_map = $this->getLiskMetadata('validMap', array());
$map = array(); $map = array();
$updated = false;
foreach ($row as $k => $v) { foreach ($row as $k => $v) {
// We permit (but ignore) extra properties in the array because a // We permit (but ignore) extra properties in the array because a
// common approach to building the array is to issue a raw SELECT query // common approach to building the array is to issue a raw SELECT query
@ -594,14 +598,15 @@ abstract class LiskDAO extends Phobject
// path (assigning an invalid property which we've already seen) costs // path (assigning an invalid property which we've already seen) costs
// an empty() plus an isset(). // an empty() plus an isset().
if (empty($valid_properties[$k])) { if (empty($valid_map[$k])) {
if (isset($valid_properties[$k])) { if (isset($valid_map[$k])) {
// The value is set but empty, which means it's false, so we've // The value is set but empty, which means it's false, so we've
// already determined it's not valid. We don't need to check again. // already determined it's not valid. We don't need to check again.
continue; continue;
} }
$valid_properties[$k] = $this->hasProperty($k); $valid_map[$k] = $this->hasProperty($k);
if (!$valid_properties[$k]) { $updated = true;
if (!$valid_map[$k]) {
continue; continue;
} }
} }
@ -609,6 +614,10 @@ abstract class LiskDAO extends Phobject
$map[$k] = $v; $map[$k] = $v;
} }
if ($updated) {
$this->setLiskMetadata('validMap', $valid_map);
}
$this->willReadData($map); $this->willReadData($map);
foreach ($map as $prop => $value) { foreach ($map as $prop => $value) {
@ -686,10 +695,7 @@ abstract class LiskDAO extends Phobject
* @task save * @task save
*/ */
public function setID($id) { public function setID($id) {
static $id_key = null; $id_key = $this->getIDKey();
if ($id_key === null) {
$id_key = $this->getIDKeyForUse();
}
$this->$id_key = $id; $this->$id_key = $id;
return $this; return $this;
} }
@ -704,10 +710,7 @@ abstract class LiskDAO extends Phobject
* @task info * @task info
*/ */
public function getID() { public function getID() {
static $id_key = null; $id_key = $this->getIDKey();
if ($id_key === null) {
$id_key = $this->getIDKeyForUse();
}
return $this->$id_key; return $this->$id_key;
} }
@ -742,9 +745,10 @@ abstract class LiskDAO extends Phobject
* @task info * @task info
*/ */
protected function getAllLiskProperties() { protected function getAllLiskProperties() {
static $properties = null; $properties = $this->getLiskMetadata('properties');
if (!isset($properties)) {
$class = new ReflectionClass(get_class($this)); if ($properties === null) {
$class = new ReflectionClass(static::class);
$properties = array(); $properties = array();
foreach ($class->getProperties(ReflectionProperty::IS_PROTECTED) as $p) { foreach ($class->getProperties(ReflectionProperty::IS_PROTECTED) as $p) {
$properties[strtolower($p->getName())] = $p->getName(); $properties[strtolower($p->getName())] = $p->getName();
@ -763,7 +767,10 @@ abstract class LiskDAO extends Phobject
if ($id_key != 'phid' && !$this->getConfigOption(self::CONFIG_AUX_PHID)) { if ($id_key != 'phid' && !$this->getConfigOption(self::CONFIG_AUX_PHID)) {
unset($properties['phid']); unset($properties['phid']);
} }
$this->setLiskMetadata('properties', $properties);
} }
return $properties; return $properties;
} }
@ -777,10 +784,7 @@ abstract class LiskDAO extends Phobject
* @task info * @task info
*/ */
protected function checkProperty($property) { protected function checkProperty($property) {
static $properties = null; $properties = $this->getAllLiskProperties();
if ($properties === null) {
$properties = $this->getAllLiskProperties();
}
$property = strtolower($property); $property = strtolower($property);
if (empty($properties[$property])) { if (empty($properties[$property])) {
@ -996,7 +1000,7 @@ abstract class LiskDAO extends Phobject
'UPDATE %R SET %LQ WHERE %C = '.(is_int($id) ? '%d' : '%s'), 'UPDATE %R SET %LQ WHERE %C = '.(is_int($id) ? '%d' : '%s'),
$this, $this,
$map, $map,
$this->getIDKeyForUse(), $this->getIDKey(),
$id); $id);
// We can't detect a missing object because updating an object without // We can't detect a missing object because updating an object without
// changing any values doesn't affect rows. We could jiggle timestamps // changing any values doesn't affect rows. We could jiggle timestamps
@ -1023,7 +1027,7 @@ abstract class LiskDAO extends Phobject
$conn->query( $conn->query(
'DELETE FROM %R WHERE %C = %d', 'DELETE FROM %R WHERE %C = %d',
$this, $this,
$this->getIDKeyForUse(), $this->getIDKey(),
$this->getID()); $this->getID());
$this->didDelete(); $this->didDelete();
@ -1051,7 +1055,7 @@ abstract class LiskDAO extends Phobject
// If we are using autoincrement IDs, let MySQL assign the value for the // If we are using autoincrement IDs, let MySQL assign the value for the
// ID column, if it is empty. If the caller has explicitly provided a // ID column, if it is empty. If the caller has explicitly provided a
// value, use it. // value, use it.
$id_key = $this->getIDKeyForUse(); $id_key = $this->getIDKey();
if (empty($data[$id_key])) { if (empty($data[$id_key])) {
unset($data[$id_key]); unset($data[$id_key]);
} }
@ -1059,7 +1063,7 @@ abstract class LiskDAO extends Phobject
case self::IDS_COUNTER: case self::IDS_COUNTER:
// If we are using counter IDs, assign a new ID if we don't already have // If we are using counter IDs, assign a new ID if we don't already have
// one. // one.
$id_key = $this->getIDKeyForUse(); $id_key = $this->getIDKey();
if (empty($data[$id_key])) { if (empty($data[$id_key])) {
$counter_name = $this->getTableName(); $counter_name = $this->getTableName();
$id = self::loadNextCounterValue($conn, $counter_name); $id = self::loadNextCounterValue($conn, $counter_name);
@ -1175,19 +1179,6 @@ abstract class LiskDAO extends Phobject
return 'id'; return 'id';
} }
protected function getIDKeyForUse() {
$id_key = $this->getIDKey();
if (!$id_key) {
throw new Exception(
pht(
'This DAO does not have a single-part primary key. The method you '.
'called requires a single-part primary key.'));
}
return $id_key;
}
/** /**
* Generate a new PHID, used by CONFIG_AUX_PHID. * Generate a new PHID, used by CONFIG_AUX_PHID.
* *
@ -1592,22 +1583,12 @@ abstract class LiskDAO extends Phobject
* @task util * @task util
*/ */
public function __call($method, $args) { public function __call($method, $args) {
// NOTE: PHP has a bug that static variables defined in __call() are shared $dispatch_map = $this->getLiskMetadata('dispatchMap', array());
// across all children classes. Call a different method to work around this
// bug.
return $this->call($method, $args);
}
/**
* @task util
*/
final protected function call($method, $args) {
// NOTE: This method is very performance-sensitive (many thousands of calls // NOTE: This method is very performance-sensitive (many thousands of calls
// per page on some pages), and thus has some silliness in the name of // per page on some pages), and thus has some silliness in the name of
// optimizations. // optimizations.
static $dispatch_map = array();
if ($method[0] === 'g') { if ($method[0] === 'g') {
if (isset($dispatch_map[$method])) { if (isset($dispatch_map[$method])) {
$property = $dispatch_map[$method]; $property = $dispatch_map[$method];
@ -1620,6 +1601,7 @@ abstract class LiskDAO extends Phobject
throw new Exception(pht('Bad getter call: %s', $method)); throw new Exception(pht('Bad getter call: %s', $method));
} }
$dispatch_map[$method] = $property; $dispatch_map[$method] = $property;
$this->setLiskMetadata('dispatchMap', $dispatch_map);
} }
return $this->readField($property); return $this->readField($property);
@ -1632,12 +1614,14 @@ abstract class LiskDAO extends Phobject
if (substr($method, 0, 3) !== 'set') { if (substr($method, 0, 3) !== 'set') {
throw new Exception(pht("Unable to resolve method '%s'!", $method)); throw new Exception(pht("Unable to resolve method '%s'!", $method));
} }
$property = substr($method, 3); $property = substr($method, 3);
$property = $this->checkProperty($property); $property = $this->checkProperty($property);
if (!$property) { if (!$property) {
throw new Exception(pht('Bad setter call: %s', $method)); throw new Exception(pht('Bad setter call: %s', $method));
} }
$dispatch_map[$method] = $property; $dispatch_map[$method] = $property;
$this->setLiskMetadata('dispatchMap', $dispatch_map);
} }
$this->writeField($property, $args[0]); $this->writeField($property, $args[0]);
@ -1909,4 +1893,20 @@ abstract class LiskDAO extends Phobject
} }
private function getLiskMetadata($key, $default = null) {
if (isset(self::$liskMetadata[static::class][$key])) {
return self::$liskMetadata[static::class][$key];
}
if (!isset(self::$liskMetadata[static::class])) {
self::$liskMetadata[static::class] = array();
}
return idx(self::$liskMetadata[static::class], $key, $default);
}
private function setLiskMetadata($key, $value) {
self::$liskMetadata[static::class][$key] = $value;
}
} }

View file

@ -224,7 +224,7 @@ final class PhabricatorHash extends Phobject {
$cache_key = "hmac.key({$hmac_name})"; $cache_key = "hmac.key({$hmac_name})";
$hmac_key = $cache->getKey($cache_key); $hmac_key = $cache->getKey($cache_key);
if (!strlen($hmac_key)) { if (($hmac_key === null) || !strlen($hmac_key)) {
$hmac_key = self::readHMACKey($hmac_name); $hmac_key = self::readHMACKey($hmac_name);
if ($hmac_key === null) { if ($hmac_key === null) {

View file

@ -49,7 +49,7 @@ final class PhabricatorMetronome
} }
public function setOffsetFromSeed($seed) { public function setOffsetFromSeed($seed) {
$offset = PhabricatorHash::digestToRange($seed, 0, PHP_INT_MAX); $offset = PhabricatorHash::digestToRange($seed, 0, 0x7FFFFFFF);
return $this->setOffset($offset); return $this->setOffset($offset);
} }