diff --git a/.arclint b/.arclint --- a/.arclint +++ b/.arclint @@ -79,7 +79,8 @@ "xhpast": { "type": "xhpast", "include": "(\\.php$)", - "standard": "phutil.xhpast" + "standard": "phutil.xhpast", + "xhpast.php-version": "5.5" } } } diff --git a/scripts/sql/manage_storage.php b/scripts/sql/manage_storage.php --- a/scripts/sql/manage_storage.php +++ b/scripts/sql/manage_storage.php @@ -95,7 +95,7 @@ $host = $args->getArg('host'); $ref_key = $args->getArg('ref'); -if (strlen($host) || strlen($ref_key)) { +if (($host !== null) || ($ref_key !== null)) { if ($host && $ref_key) { throw new PhutilArgumentUsageException( pht( diff --git a/src/applications/files/engine/PhabricatorLocalDiskFileStorageEngine.php b/src/applications/files/engine/PhabricatorLocalDiskFileStorageEngine.php --- a/src/applications/files/engine/PhabricatorLocalDiskFileStorageEngine.php +++ b/src/applications/files/engine/PhabricatorLocalDiskFileStorageEngine.php @@ -26,6 +26,7 @@ public function canWriteFiles() { $path = PhabricatorEnv::getEnvConfig('storage.local-disk.path'); + $path = phutil_string_cast($path); return (bool)strlen($path); } diff --git a/src/applications/meta/query/PhabricatorApplicationQuery.php b/src/applications/meta/query/PhabricatorApplicationQuery.php --- a/src/applications/meta/query/PhabricatorApplicationQuery.php +++ b/src/applications/meta/query/PhabricatorApplicationQuery.php @@ -89,7 +89,7 @@ } } - if (strlen($this->nameContains)) { + if ($this->nameContains !== null) { foreach ($apps as $key => $app) { if (stripos($app->getName(), $this->nameContains) === false) { unset($apps[$key]); diff --git a/src/applications/people/query/PhabricatorPeopleQuery.php b/src/applications/people/query/PhabricatorPeopleQuery.php --- a/src/applications/people/query/PhabricatorPeopleQuery.php +++ b/src/applications/people/query/PhabricatorPeopleQuery.php @@ -341,7 +341,7 @@ (int)$this->isMailingList); } - if (strlen($this->nameLike)) { + if ($this->nameLike !== null) { $where[] = qsprintf( $conn, 'user.username LIKE %~ OR user.realname LIKE %~', diff --git a/src/applications/people/storage/PhabricatorUser.php b/src/applications/people/storage/PhabricatorUser.php --- a/src/applications/people/storage/PhabricatorUser.php +++ b/src/applications/people/storage/PhabricatorUser.php @@ -275,7 +275,8 @@ $this->setConduitCertificate($this->generateConduitCertificate()); } - if (!strlen($this->getAccountSecret())) { + $secret = $this->getAccountSecret(); + if (($secret === null) || !strlen($secret)) { $this->setAccountSecret(Filesystem::readRandomCharacters(64)); } diff --git a/src/applications/phid/handle/pool/PhabricatorHandleList.php b/src/applications/phid/handle/pool/PhabricatorHandleList.php --- a/src/applications/phid/handle/pool/PhabricatorHandleList.php +++ b/src/applications/phid/handle/pool/PhabricatorHandleList.php @@ -126,22 +126,27 @@ /* -( Iterator )----------------------------------------------------------- */ + #[\ReturnTypeWillChange] public function rewind() { $this->cursor = 0; } + #[\ReturnTypeWillChange] public function current() { return $this->getHandle($this->phids[$this->cursor]); } + #[\ReturnTypeWillChange] public function key() { return $this->phids[$this->cursor]; } + #[\ReturnTypeWillChange] public function next() { ++$this->cursor; } + #[\ReturnTypeWillChange] public function valid() { return ($this->cursor < $this->count); } @@ -150,6 +155,7 @@ /* -( ArrayAccess )-------------------------------------------------------- */ + #[\ReturnTypeWillChange] public function offsetExists($offset) { // NOTE: We're intentionally not loading handles here so that isset() // checks do not trigger fetches. This gives us better bulk loading @@ -162,6 +168,7 @@ return isset($this->map[$offset]); } + #[\ReturnTypeWillChange] public function offsetGet($offset) { if ($this->handles === null) { $this->loadHandles(); @@ -169,10 +176,12 @@ return $this->handles[$offset]; } + #[\ReturnTypeWillChange] public function offsetSet($offset, $value) { $this->raiseImmutableException(); } + #[\ReturnTypeWillChange] public function offsetUnset($offset) { $this->raiseImmutableException(); } @@ -189,6 +198,7 @@ /* -( Countable )---------------------------------------------------------- */ + #[\ReturnTypeWillChange] public function count() { return $this->count; } diff --git a/src/infrastructure/cluster/PhabricatorDatabaseRef.php b/src/infrastructure/cluster/PhabricatorDatabaseRef.php --- a/src/infrastructure/cluster/PhabricatorDatabaseRef.php +++ b/src/infrastructure/cluster/PhabricatorDatabaseRef.php @@ -322,6 +322,7 @@ $default_user = PhabricatorEnv::getEnvConfig('mysql.user'); $default_pass = PhabricatorEnv::getEnvConfig('mysql.pass'); + $default_pass = phutil_string_cast($default_pass); $default_pass = new PhutilOpaqueEnvelope($default_pass); $config = PhabricatorEnv::getEnvConfig('cluster.databases'); diff --git a/src/infrastructure/query/PhabricatorQuery.php b/src/infrastructure/query/PhabricatorQuery.php --- a/src/infrastructure/query/PhabricatorQuery.php +++ b/src/infrastructure/query/PhabricatorQuery.php @@ -87,7 +87,7 @@ foreach ($this->flattenSubclause($part) as $subpart) { $result[] = $subpart; } - } else if (strlen($part)) { + } else if (($part !== null) && strlen($part)) { $result[] = $part; } } diff --git a/src/infrastructure/storage/connection/mysql/AphrontMySQLiDatabaseConnection.php b/src/infrastructure/storage/connection/mysql/AphrontMySQLiDatabaseConnection.php --- a/src/infrastructure/storage/connection/mysql/AphrontMySQLiDatabaseConnection.php +++ b/src/infrastructure/storage/connection/mysql/AphrontMySQLiDatabaseConnection.php @@ -57,6 +57,13 @@ } } + // 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(); $timeout = $this->getConfiguration('timeout'); diff --git a/src/infrastructure/storage/lisk/LiskDAO.php b/src/infrastructure/storage/lisk/LiskDAO.php --- a/src/infrastructure/storage/lisk/LiskDAO.php +++ b/src/infrastructure/storage/lisk/LiskDAO.php @@ -193,6 +193,8 @@ private static $connections = array(); + private static $liskMetadata = array(); + protected $id; protected $phid; protected $dateCreated; @@ -403,10 +405,11 @@ * @task config */ public function getConfigOption($option_name) { - static $options = null; + $options = $this->getLiskMetadata('config'); - if (!isset($options)) { + if ($options === null) { $options = $this->getConfiguration(); + $this->setLiskMetadata('config', $options); } return idx($options, $option_name); @@ -439,7 +442,7 @@ return $this->loadOneWhere( '%C = %d', - $this->getIDKeyForUse(), + $this->getIDKey(), $id); } @@ -554,7 +557,7 @@ $result = $this->loadOneWhere( '%C = %d', - $this->getIDKeyForUse(), + $this->getIDKey(), $this->getID()); if (!$result) { @@ -579,9 +582,10 @@ * @task load */ public function loadFromArray(array $row) { - static $valid_properties = array(); + $valid_map = $this->getLiskMetadata('validMap', array()); $map = array(); + $updated = false; foreach ($row as $k => $v) { // We permit (but ignore) extra properties in the array because a // common approach to building the array is to issue a raw SELECT query @@ -594,14 +598,15 @@ // path (assigning an invalid property which we've already seen) costs // an empty() plus an isset(). - if (empty($valid_properties[$k])) { - if (isset($valid_properties[$k])) { + if (empty($valid_map[$k])) { + if (isset($valid_map[$k])) { // 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. continue; } - $valid_properties[$k] = $this->hasProperty($k); - if (!$valid_properties[$k]) { + $valid_map[$k] = $this->hasProperty($k); + $updated = true; + if (!$valid_map[$k]) { continue; } } @@ -609,6 +614,10 @@ $map[$k] = $v; } + if ($updated) { + $this->setLiskMetadata('validMap', $valid_map); + } + $this->willReadData($map); foreach ($map as $prop => $value) { @@ -686,10 +695,7 @@ * @task save */ public function setID($id) { - static $id_key = null; - if ($id_key === null) { - $id_key = $this->getIDKeyForUse(); - } + $id_key = $this->getIDKey(); $this->$id_key = $id; return $this; } @@ -704,10 +710,7 @@ * @task info */ public function getID() { - static $id_key = null; - if ($id_key === null) { - $id_key = $this->getIDKeyForUse(); - } + $id_key = $this->getIDKey(); return $this->$id_key; } @@ -742,9 +745,10 @@ * @task info */ protected function getAllLiskProperties() { - static $properties = null; - if (!isset($properties)) { - $class = new ReflectionClass(get_class($this)); + $properties = $this->getLiskMetadata('properties'); + + if ($properties === null) { + $class = new ReflectionClass(static::class); $properties = array(); foreach ($class->getProperties(ReflectionProperty::IS_PROTECTED) as $p) { $properties[strtolower($p->getName())] = $p->getName(); @@ -763,7 +767,10 @@ if ($id_key != 'phid' && !$this->getConfigOption(self::CONFIG_AUX_PHID)) { unset($properties['phid']); } + + $this->setLiskMetadata('properties', $properties); } + return $properties; } @@ -777,10 +784,7 @@ * @task info */ protected function checkProperty($property) { - static $properties = null; - if ($properties === null) { - $properties = $this->getAllLiskProperties(); - } + $properties = $this->getAllLiskProperties(); $property = strtolower($property); if (empty($properties[$property])) { @@ -996,7 +1000,7 @@ 'UPDATE %R SET %LQ WHERE %C = '.(is_int($id) ? '%d' : '%s'), $this, $map, - $this->getIDKeyForUse(), + $this->getIDKey(), $id); // We can't detect a missing object because updating an object without // changing any values doesn't affect rows. We could jiggle timestamps @@ -1023,7 +1027,7 @@ $conn->query( 'DELETE FROM %R WHERE %C = %d', $this, - $this->getIDKeyForUse(), + $this->getIDKey(), $this->getID()); $this->didDelete(); @@ -1051,7 +1055,7 @@ // 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 // value, use it. - $id_key = $this->getIDKeyForUse(); + $id_key = $this->getIDKey(); if (empty($data[$id_key])) { unset($data[$id_key]); } @@ -1059,7 +1063,7 @@ case self::IDS_COUNTER: // If we are using counter IDs, assign a new ID if we don't already have // one. - $id_key = $this->getIDKeyForUse(); + $id_key = $this->getIDKey(); if (empty($data[$id_key])) { $counter_name = $this->getTableName(); $id = self::loadNextCounterValue($conn, $counter_name); @@ -1175,19 +1179,6 @@ 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. * @@ -1592,22 +1583,12 @@ * @task util */ public function __call($method, $args) { - // NOTE: PHP has a bug that static variables defined in __call() are shared - // across all children classes. Call a different method to work around this - // bug. - return $this->call($method, $args); - } + $dispatch_map = $this->getLiskMetadata('dispatchMap', array()); - /** - * @task util - */ - final protected function call($method, $args) { // 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 // optimizations. - static $dispatch_map = array(); - if ($method[0] === 'g') { if (isset($dispatch_map[$method])) { $property = $dispatch_map[$method]; @@ -1620,6 +1601,7 @@ throw new Exception(pht('Bad getter call: %s', $method)); } $dispatch_map[$method] = $property; + $this->setLiskMetadata('dispatchMap', $dispatch_map); } return $this->readField($property); @@ -1632,12 +1614,14 @@ if (substr($method, 0, 3) !== 'set') { throw new Exception(pht("Unable to resolve method '%s'!", $method)); } + $property = substr($method, 3); $property = $this->checkProperty($property); if (!$property) { throw new Exception(pht('Bad setter call: %s', $method)); } $dispatch_map[$method] = $property; + $this->setLiskMetadata('dispatchMap', $dispatch_map); } $this->writeField($property, $args[0]); @@ -1909,4 +1893,20 @@ } + 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; + } + } diff --git a/src/infrastructure/util/PhabricatorHash.php b/src/infrastructure/util/PhabricatorHash.php --- a/src/infrastructure/util/PhabricatorHash.php +++ b/src/infrastructure/util/PhabricatorHash.php @@ -224,7 +224,7 @@ $cache_key = "hmac.key({$hmac_name})"; $hmac_key = $cache->getKey($cache_key); - if (!strlen($hmac_key)) { + if (($hmac_key === null) || !strlen($hmac_key)) { $hmac_key = self::readHMACKey($hmac_name); if ($hmac_key === null) { diff --git a/src/infrastructure/util/PhabricatorMetronome.php b/src/infrastructure/util/PhabricatorMetronome.php --- a/src/infrastructure/util/PhabricatorMetronome.php +++ b/src/infrastructure/util/PhabricatorMetronome.php @@ -49,7 +49,7 @@ } public function setOffsetFromSeed($seed) { - $offset = PhabricatorHash::digestToRange($seed, 0, PHP_INT_MAX); + $offset = PhabricatorHash::digestToRange($seed, 0, 0x7FFFFFFF); return $this->setOffset($offset); }