Add function param/return types, fix a lot more phpstorm complaints

10 files changed, 44 insertions, 53 deletions

diff --git a/modules-available/rebootcontrol/hooks/client-update.inc.php b/modules-available/rebootcontrol/hooks/client-update.inc.php
index 006a5e11..e934988d 100644
--- a/modules-available/rebootcontrol/hooks/client-update.inc.php
+++ b/modules-available/rebootcontrol/hooks/client-update.inc.php
@@ -7,7 +7,7 @@ if ($type === '~poweron') {
 		&& $subnet[0] === $ip && $subnet[1] >= 8 && $subnet[1] < 32) {
 		$start = ip2long($ip);
 		if ($start !== false) {
-			$maskHost = (int)(pow(2, 32 - $subnet[1]) - 1);
+			$maskHost = (int)(2 ** (32 - $subnet[1]) - 1);
 			$maskNet = ~$maskHost & 0xffffffff;
 			$end = $start | $maskHost;
 			$start &= $maskNet;
diff --git a/modules-available/rebootcontrol/hooks/config-tgz.inc.php b/modules-available/rebootcontrol/hooks/config-tgz.inc.php
index 90e32e8a..c9ce1255 100644
--- a/modules-available/rebootcontrol/hooks/config-tgz.inc.php
+++ b/modules-available/rebootcontrol/hooks/config-tgz.inc.php
@@ -8,8 +8,10 @@ if (!is_file($tmpfile) || !is_readable($tmpfile) || filemtime($tmpfile) + 86400
 	}
 	try {
 		$a = new PharData($tmpfile);
-		$a["/etc/ssh/mgmt/authorized_keys"] = $pubkey;
-		$a["/etc/ssh/mgmt/authorized_keys"]->chmod(0600);
+		$a->addFromString("/etc/ssh/mgmt/authorized_keys", $pubkey);
+		$fi = $a->offsetGet("/etc/ssh/mgmt/authorized_keys");
+		/** @var PharFileInfo $fi */
+		$fi->chmod(0600);
 		$file = $tmpfile;
 	} catch (Exception $e) {
 		EventLog::failure('Could not include ssh key for reboot-control in config.tgz', (string)$e);
diff --git a/modules-available/rebootcontrol/hooks/cron.inc.php b/modules-available/rebootcontrol/hooks/cron.inc.php
index cdecc31b..289426c7 100644
--- a/modules-available/rebootcontrol/hooks/cron.inc.php
+++ b/modules-available/rebootcontrol/hooks/cron.inc.php
@@ -24,7 +24,7 @@ class Stuff
 	public static $subnets;
 }
 
-function destSawPw($destTask, $destMachine, $passwd)
+function destSawPw(array $destTask, array $destMachine, string $passwd): bool
 {
 	return strpos($destTask['data']['result'][$destMachine['machineuuid']]['stdout'], "passwd=$passwd") !== false;
 }
diff --git a/modules-available/rebootcontrol/inc/rebootcontrol.inc.php b/modules-available/rebootcontrol/inc/rebootcontrol.inc.php
index a37b070c..ab308084 100644
--- a/modules-available/rebootcontrol/inc/rebootcontrol.inc.php
+++ b/modules-available/rebootcontrol/inc/rebootcontrol.inc.php
@@ -61,13 +61,8 @@ class RebootControl
 
 	/**
 	 * Add wake task metadata to database, so we can display job details on the summary page.
-	 * @param string $taskId
-	 * @param string $type
-	 * @param array $clients
-	 * @param ?array $other
-	 * @return void
 	 */
-	private static function addTask(string $taskId, string $type, array $clients, array $other = null)
+	private static function addTask(string $taskId, string $type, array $clients, array $other = null): void
 	{
 		$lids = ArrayUtil::flattenByKey($clients, 'locationid');
 		$lids = array_unique($lids);
@@ -220,12 +215,10 @@ class RebootControl
 	}
 
 	/**
-	 * @param string|string[] $macs
-	 * @param ?string $bcast
-	 * @param ?string $passwd
-	 * @return string
+	 * Wake clients given by MAC address(es) via jawol util.
+	 * Multiple MAC addresses can be passed as a space separated list.
 	 */
-	private static function buildClientWakeCommand($macs, string $bcast = null, string $passwd = null): string
+	private static function buildClientWakeCommand(string $macs, string $bcast = null, string $passwd = null): string
 	{
 		$command = 'jawol';
 		if (!empty($bcast)) {
@@ -236,21 +229,18 @@ class RebootControl
 		if (!empty($passwd)) {
 			$command .= " -p '$passwd'";
 		}
-		if (is_array($macs)) {
-			$macs = implode(" ", $macs);
-		}
 		$command .= " $macs";
 		return $command;
 	}
 
 	/**
 	 * @param array $sourceMachines list of source machines. array of [clientip, machineuuid] entries
-	 * @param string|string[] $macaddr destination mac address(es)
-	 * @param ?string $bcast directed broadcast address to send to
-	 * @param ?string $passwd optional WOL password, mac address or ipv4 notation
+	 * @param string $macaddr destination mac address(es)
+	 * @param string $bcast directed broadcast address to send to
+	 * @param string $passwd optional WOL password, mac address or ipv4 notation
 	 * @return array|false task struct, false on error
 	 */
-	public static function wakeViaClient($sourceMachines, $macaddr, string $bcast = null, string $passwd = null)
+	public static function wakeViaClient(array $sourceMachines, string $macaddr, string $bcast = null, string $passwd = null)
 	{
 		$command = self::buildClientWakeCommand($macaddr, $bcast, $passwd);
 		// Yes there is one zero "missing" from the usleep -- that's the whole point: we prefer 100ms sleeps
@@ -320,7 +310,7 @@ class RebootControl
 	 * @param ?array $failed list of failed clients from $clientList
 	 * @return ?string taskid of this job
 	 */
-	public static function wakeMachines(array $clientList, array &$failed = null)
+	public static function wakeMachines(array $clientList, array &$failed = null): ?string
 	{
 		$errors = '';
 		$sent = $unknown = $unreachable = $failed = [];
diff --git a/modules-available/rebootcontrol/inc/rebootutils.inc.php b/modules-available/rebootcontrol/inc/rebootutils.inc.php
index f6843150..e05d90dc 100644
--- a/modules-available/rebootcontrol/inc/rebootutils.inc.php
+++ b/modules-available/rebootcontrol/inc/rebootutils.inc.php
@@ -8,13 +8,12 @@ class RebootUtils
 	 * @param string[] $list list of system UUIDs
 	 * @return array list of machines with machineuuid, hostname, clientip, state and locationid
 	 */
-	public static function getMachinesByUuid($list, $assoc = false, $columns = ['machineuuid', 'hostname', 'clientip', 'state', 'locationid'])
+	public static function getMachinesByUuid(array $list, bool $assoc = false,
+														  array $columns = ['machineuuid', 'hostname', 'clientip', 'state', 'locationid']): array
 	{
 		if (empty($list))
 			return array();
-		if (is_array($columns)) {
-			$columns = implode(',', $columns);
-		}
+		$columns = implode(',', $columns);
 		$res = Database::simpleQuery("SELECT $columns FROM machine
 				WHERE machineuuid IN (:list)", compact('list'));
 		if (!$assoc)
@@ -31,7 +30,7 @@ class RebootUtils
 	 * Requires the array elements to have key "state" from machine table.
 	 * @param array $clients list of clients
 	 */
-	public static function sortRunningFirst(&$clients)
+	public static function sortRunningFirst(array &$clients): void
 	{
 		usort($clients, function($a, $b) {
 			$a = ($a['state'] === 'IDLE' || $a['state'] === 'OCCUPIED');
@@ -49,7 +48,7 @@ class RebootUtils
 	 * @param string $permission name of location-aware permission to check
 	 * @return array|false List of clients the user has access to.
 	 */
-	public static function getFilteredMachineList($requestedClients, $permission)
+	public static function getFilteredMachineList(array $requestedClients, string $permission)
 	{
 		$actualClients = RebootUtils::getMachinesByUuid($requestedClients);
 		if (count($actualClients) !== count($requestedClients)) {
diff --git a/modules-available/rebootcontrol/inc/scheduler.inc.php b/modules-available/rebootcontrol/inc/scheduler.inc.php
index 937cc000..19a01beb 100644
--- a/modules-available/rebootcontrol/inc/scheduler.inc.php
+++ b/modules-available/rebootcontrol/inc/scheduler.inc.php
@@ -199,8 +199,7 @@ class Scheduler
 	}
 
 	/**
-	 * Get current settings for given location, or false if none.
-	 * @param int $id
+	 * Get current settings for given location.
 	 */
 	public static function getLocationOptions(int $id): array
 	{
@@ -216,7 +215,6 @@ class Scheduler
 
 	/**
 	 * Write new WOL/Shutdown options for given location.
-	 * @param int $locationId
 	 * @param array $options 'wol' 'sd' 'wol-offset' 'sd-offset' 'ra-mode'
 	 */
 	public static function setLocationOptions(int $locationId, array $options)
diff --git a/modules-available/rebootcontrol/inc/sshkey.inc.php b/modules-available/rebootcontrol/inc/sshkey.inc.php
index cce9b3dc..e0954415 100644
--- a/modules-available/rebootcontrol/inc/sshkey.inc.php
+++ b/modules-available/rebootcontrol/inc/sshkey.inc.php
@@ -3,13 +3,17 @@
 class SSHKey
 {
 
-	public static function getPrivateKey(&$regen = false) {
+	public static function getPrivateKey(?bool &$regen = false): ?string
+	{
 		$privKey = Property::get("rebootcontrol-private-key");
 		if (!$privKey) {
-			$rsaKey = openssl_pkey_new(array(
+			$rsaKey = openssl_pkey_new([
 				'private_key_bits' => 2048,
-				'private_key_type' => OPENSSL_KEYTYPE_RSA));
-			openssl_pkey_export( openssl_pkey_get_private($rsaKey), $privKey);
+				'private_key_type' => OPENSSL_KEYTYPE_RSA]);
+			if (!openssl_pkey_export( openssl_pkey_get_private($rsaKey), $privKey)) {
+				$regen = false;
+				return null;
+			}
 			Property::set("rebootcontrol-private-key", $privKey);
 			if (Module::isAvailable('sysconfig')) {
 				ConfigTgz::rebuildAllConfigs();
@@ -19,21 +23,30 @@ class SSHKey
 		return $privKey;
 	}
 
-	public static function getPublicKey() {
+	public static function getPublicKey(): ?string
+	{
 		$pkImport = openssl_pkey_get_private(self::getPrivateKey());
+		if ($pkImport === false)
+			return null;
 		return self::sshEncodePublicKey($pkImport);
 	}
 
-	private static function sshEncodePublicKey($privKey) {
+	private static function sshEncodePublicKey($privKey): ?string
+	{
 		$keyInfo = openssl_pkey_get_details($privKey);
+		if ($keyInfo === false)
+			return null;
 		$buffer  = pack("N", 7) . "ssh-rsa" .
 			self::sshEncodeBuffer($keyInfo['rsa']['e']) .
 			self::sshEncodeBuffer($keyInfo['rsa']['n']);
 		return "ssh-rsa " . base64_encode($buffer);
 	}
 
-	private static function sshEncodeBuffer($buffer) {
+	private static function sshEncodeBuffer(string $buffer): string
+	{
 		$len = strlen($buffer);
+		// Prefix with extra null byte if the MSB is set, to ensure
+		// nobody will ever interpret this as a negative number
 		if (ord($buffer[0]) & 0x80) {
 			$len++;
 			$buffer = "\x00" . $buffer;
diff --git a/modules-available/rebootcontrol/page.inc.php b/modules-available/rebootcontrol/page.inc.php
index c189e1fc..80eff842 100644
--- a/modules-available/rebootcontrol/page.inc.php
+++ b/modules-available/rebootcontrol/page.inc.php
@@ -21,10 +21,10 @@ class Page_RebootControl extends Page
 		}
 
 		if (User::hasPermission('jumphost.*')) {
-			Dashboard::addSubmenu('?do=rebootcontrol&show=jumphost', Dictionary::translate('jumphosts', true));
+			Dashboard::addSubmenu('?do=rebootcontrol&show=jumphost', Dictionary::translate('jumphosts'));
 		}
 		if (User::hasPermission('subnet.*')) {
-			Dashboard::addSubmenu('?do=rebootcontrol&show=subnet', Dictionary::translate('subnets', true));
+			Dashboard::addSubmenu('?do=rebootcontrol&show=subnet', Dictionary::translate('subnets'));
 		}
 
 		$section = Request::any('show', false, 'string');
diff --git a/modules-available/rebootcontrol/pages/subnet.inc.php b/modules-available/rebootcontrol/pages/subnet.inc.php
index c1631cbd..3e593505 100644
--- a/modules-available/rebootcontrol/pages/subnet.inc.php
+++ b/modules-available/rebootcontrol/pages/subnet.inc.php
@@ -24,7 +24,7 @@ class SubPage
 		User::assertPermission('subnet.edit');
 		$cidr = Request::post('cidr', Request::REQUIRED, 'string');
 		$range = IpUtil::parseCidr($cidr);
-		if ($range === false) {
+		if ($range === null) {
 			Message::addError('invalid-cidr', $cidr);
 			return;
 		}
diff --git a/modules-available/rebootcontrol/pages/task.inc.php b/modules-available/rebootcontrol/pages/task.inc.php
index dc50bfff..7e1f6fbf 100644
--- a/modules-available/rebootcontrol/pages/task.inc.php
+++ b/modules-available/rebootcontrol/pages/task.inc.php
@@ -145,14 +145,3 @@ class SubPage
 	}
 
 }
-
-
-// Remove when we require >= 7.3.0
-if (!function_exists('array_key_first')) {
-	function array_key_first(array $arr) {
-		foreach($arr as $key => $unused) {
-			return $key;
-		}
-		return NULL;
-	}
-}