[XMPP] Respond to ping and track time monotonically

This also fetches a necessary update from the XMPPHP upstream.

composer.lock

@@ -191,16 +191,16 @@
         },
         {
             "name": "diogocomposer/xmpphp",
-            "version": "v3.0.2",
+            "version": "v3.0.3",
             "source": {
                 "type": "git",
                 "url": "https://github.com/diogogithub/xmpphp.git",
-                "reference": "d95381f8fb5ffbbda0894aea415742daaa2eab22"
+                "reference": "37f69546e8e24703c4a9116e7bb14864a61ee369"
             },
             "dist": {
                 "type": "zip",
-                "url": "https://api.github.com/repos/diogogithub/xmpphp/zipball/d95381f8fb5ffbbda0894aea415742daaa2eab22",
+                "url": "https://api.github.com/repos/diogogithub/xmpphp/zipball/37f69546e8e24703c4a9116e7bb14864a61ee369",
-                "reference": "d95381f8fb5ffbbda0894aea415742daaa2eab22",
+                "reference": "37f69546e8e24703c4a9116e7bb14864a61ee369",
                 "shasum": ""
             },
             "require": {
@@ -262,7 +262,7 @@
                 "xmpp",
                 "xmpphp"
             ],
-            "time": "2019-11-03T00:18:51+00:00"
+            "time": "2020-09-13T20:00:21+00:00"
         },
         {
             "name": "doctrine/cache",

plugins/Xmpp/lib/xmppmanager.php

@@ -1,25 +1,25 @@
 <?php
+// This file is part of GNU social - https://www.gnu.org/software/social
+//
+// GNU social is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// GNU social is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with GNU social.  If not, see <http://www.gnu.org/licenses/>.
+
 /*
- * StatusNet - the distributed open-source microblogging tool
+ * @copyright 2008, 2009 StatusNet, Inc.
- * Copyright (C) 2008, 2009, StatusNet, Inc.
+ * @license   https://www.gnu.org/licenses/agpl.html GNU AGPL v3 or later
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU Affero General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU Affero General Public License for more details.
- *
- * You should have received a copy of the GNU Affero General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-if (!defined('STATUSNET') && !defined('LACONICA')) {
+defined('GNUSOCIAL') || die();
-    exit(1);
-}
 
 use XMPPHP\Log;
 
@@ -37,8 +37,8 @@ class XmppManager extends ImManager
 {
     const PING_INTERVAL = 120;
     public $conn = null;
-    protected $lastping = null;
+    protected $lastping = 0;
-    protected $pingid = null;
+    protected $pingid = 0;
 
     /**
      * Initialize connection to server.
@@ -55,13 +55,12 @@ class XmppManager extends ImManager
         }
     }
 
-    function connect()
+    protected function connect()
     {
         if (!$this->conn || $this->conn->isDisconnected()) {
             $resource = 'queue' . posix_getpid();
-            $this->conn = new SharingXMPP($this->plugin->host ?
+            $this->conn = new SharingXMPP(
-                $this->plugin->host :
+                $this->plugin->host ?: $this->plugin->server,
-                $this->plugin->server,
                 $this->plugin->port,
                 $this->plugin->user,
                 $this->plugin->password,
@@ -76,8 +75,18 @@ class XmppManager extends ImManager
             if (!$this->conn) {
                 return false;
             }
-            $this->conn->addEventHandler('message', 'handle_xmpp_message', $this);
+            $this->conn->addEventHandler('message', function (&$pl) {
-            $this->conn->addEventHandler('reconnect', 'handle_xmpp_reconnect', $this);
+                $this->handleXmppMessage($pl);
+            });
+            $this->conn->addEventHandler('reconnect', function ($pl) {
+                $this->handleXmppReconnect();
+            });
+            $this->conn->addXPathHandler(
+                'iq/{urn:xmpp:ping}ping',
+                function (&$xml) {
+                    $this->handleXmppPing($xml);
+                }
+            );
             $this->conn->setReconnectTimeout(600);
 
             $this->conn->autoSubscribe();
@@ -87,7 +96,13 @@ class XmppManager extends ImManager
 
             $this->conn->processUntil('session_start');
             // TRANS: Presence announcement for XMPP.
-            $this->send_presence(_m('Send me a message to post a notice'), 'available', null, 'available', 100);
+            $this->sendPresence(
+                _m('Send me a message to post a notice'),
+                'available',
+                null,
+                'available',
+                100
+            );
         }
         return $this->conn;
     }
@@ -95,18 +110,21 @@ class XmppManager extends ImManager
     /**
      * sends a presence stanza on the XMPP network
      *
-     * @param string $status current status, free-form string
+     * @param string|null $status current status, free-form string
      * @param string $show structured status value
-     * @param string $to recipient of presence, null for general
+     * @param string|null $to recipient of presence, null for general
      * @param string $type type of status message, related to $show
-     * @param int $priority priority of the presence
+     * @param int|null $priority priority of the presence
      *
-     * @return boolean success value
+     * @return bool success value
      */
-
+    protected function sendPresence(
-    function send_presence($status, $show = 'available', $to = null,
+        ?string $status,
-                           $type = 'available', $priority = null)
+        string $show = 'available',
-    {
+        ?string $to = null,
+        string $type = 'available',
+        ?int $priority = null
+    ): bool {
         $this->connect();
         if (!$this->conn || $this->conn->isDisconnected()) {
             return false;
@@ -115,7 +133,7 @@ class XmppManager extends ImManager
         return true;
     }
 
-    function send_raw_message($data)
+    public function send_raw_message($data)
     {
         $this->connect();
         if (!$this->conn || $this->conn->isDisconnected()) {
@@ -129,7 +147,7 @@ class XmppManager extends ImManager
      * Message pump is triggered on socket input, so we only need an idle()
      * call often enough to trigger our outgoing pings.
      */
-    function timeout()
+    public function timeout()
     {
         return self::PING_INTERVAL;
     }
@@ -174,48 +192,61 @@ class XmppManager extends ImManager
      */
     public function idle($timeout = 0)
     {
-        $now = time();
+        if (
-        if (empty($this->lastping) || $now - $this->lastping > self::PING_INTERVAL) {
+            hrtime(true) - $this->lastping > self::PING_INTERVAL * 1000000000
-            $this->send_ping();
+        ) {
+            $this->sendPing();
         }
     }
 
-    function send_ping()
+    protected function sendPing(): bool
     {
         $this->connect();
         if (!$this->conn || $this->conn->isDisconnected()) {
             return false;
         }
-        $now = time();
+        ++$this->pingid;
-        if (!isset($this->pingid)) {
-            $this->pingid = 0;
-        } else {
-            $this->pingid++;
-        }
 
         common_log(LOG_DEBUG, "Sending ping #{$this->pingid}");
         $this->conn->send("<iq from='{$this->plugin->daemonScreenname()}' to='{$this->plugin->server}' id='ping_{$this->pingid}' type='get'><ping xmlns='urn:xmpp:ping'/></iq>");
-        $this->lastping = $now;
+        $this->lastping = hrtime(true);
         return true;
     }
 
-    function handle_xmpp_message(&$pl)
+    protected function handleXmppMessage($pl): void
     {
         $this->plugin->enqueueIncomingRaw($pl);
-        return true;
     }
 
     /**
-     * Callback for Jabber reconnect event
+     * Callback for the XMPP reconnect event
-     * @param $pl
+     * @return void
      */
-    function handle_xmpp_reconnect(&$pl)
+    protected function handleXmppReconnect(): void
     {
         common_log(LOG_NOTICE, 'XMPP reconnected');
 
         $this->conn->processUntil('session_start');
         // TRANS: Message for XMPP reconnect.
-        $this->send_presence(_m('Send me a message to post a notice'), 'available', null, 'available', 100);
+        $this->sendPresence(
+            _m('Send me a message to post a notice'),
+            'available',
+            null,
+            'available',
+            100
+        );
+    }
+
+    protected function handleXmppPing($xml): void
+    {
+        if ($xml->attrs['type'] !== 'get') {
+            return;
+        }
+
+        $this->conn->send(
+            "<iq from=\"{$xml->attrs['to']}\" to=\"{$xml->attrs['from']}\" "
+            . "id=\"{$xml->attrs['id']}\" type=\"result\" />"
+        );
     }
 
     /**
@@ -226,14 +257,18 @@ class XmppManager extends ImManager
      * @param string $show show value for presence
      * @param string $status status value for presence
      *
-     * @return boolean success flag
+     * @return bool success flag
      *
-     * @see send_presence()
+     * @see sendPresence()
      */
 
-    function special_presence($type, $to = null, $show = null, $status = null)
+    protected function specialPresence(
-    {
+        string $type,
-        // @todo FIXME: why use this instead of send_presence()?
+        ?string $to = null,
+        ?string $show = null,
+        ?string $status = null
+    ): bool {
+        // @todo @fixme Why use this instead of sendPresence()?
         $this->connect();
         if (!$this->conn || $this->conn->isDisconnected()) {
             return false;

vendor/composer/ClassLoader.php

@@ -60,7 +60,7 @@ class ClassLoader
     public function getPrefixes()
     {
         if (!empty($this->prefixesPsr0)) {
-            return call_user_func_array('array_merge', $this->prefixesPsr0);
+            return call_user_func_array('array_merge', array_values($this->prefixesPsr0));
         }
 
         return array();

vendor/composer/installed.json

@@ -346,17 +346,17 @@
     },
     {
         "name": "diogocomposer/xmpphp",
-        "version": "v3.0.2",
+        "version": "v3.0.3",
-        "version_normalized": "3.0.2.0",
+        "version_normalized": "3.0.3.0",
         "source": {
             "type": "git",
             "url": "https://github.com/diogogithub/xmpphp.git",
-            "reference": "d95381f8fb5ffbbda0894aea415742daaa2eab22"
+            "reference": "37f69546e8e24703c4a9116e7bb14864a61ee369"
         },
         "dist": {
             "type": "zip",
-            "url": "https://api.github.com/repos/diogogithub/xmpphp/zipball/d95381f8fb5ffbbda0894aea415742daaa2eab22",
+            "url": "https://api.github.com/repos/diogogithub/xmpphp/zipball/37f69546e8e24703c4a9116e7bb14864a61ee369",
-            "reference": "d95381f8fb5ffbbda0894aea415742daaa2eab22",
+            "reference": "37f69546e8e24703c4a9116e7bb14864a61ee369",
             "shasum": ""
         },
         "require": {
@@ -368,7 +368,7 @@
             "ext-xml": "*",
             "php": "^7.3.0"
         },
-        "time": "2019-11-03T00:18:51+00:00",
+        "time": "2020-09-13T20:00:21+00:00",
         "type": "library",
         "installation-source": "dist",
         "autoload": {

vendor/diogocomposer/xmpphp/XMPPHP/Roster.php

@@ -79,14 +79,14 @@ class Roster
      * Retrieve contact via jid
      *
      * @param string $jid
-     * @return mixed|void
+     * @return array|null
      */
-    public function getContact(string $jid)
+    public function getContact(string $jid): ?array
     {
         if ($this->isContact($jid)) {
             return $this->roster_array[$jid]['contact'];
         }
-        return;
+        return null;
     }
 
     /**
@@ -128,12 +128,16 @@ class Roster
      * Add given contact to roster
      *
      * @param string $jid
-     * @param string $subscription
+     * @param string $subscription (optional)
      * @param string $name (optional)
      * @param array $groups (optional)
      */
-    public function addContact(string $jid, string $subscription, string $name = '', array $groups = []): void
+    public function addContact(
-    {
+        string $jid,
+        string $subscription = 'none',
+        string $name = '',
+        array  $groups = []
+    ): void {
         $contact = ['jid' => $jid, 'subscription' => $subscription, 'name' => $name, 'groups' => $groups];
         if ($this->isContact($jid)) {
             $this->roster_array[$jid]['contact'] = $contact;

vendor/diogocomposer/xmpphp/XMPPHP/XMLObj.php

@@ -158,15 +158,15 @@ class XMLObj
      * Return a sub
      *
      * @param string $name
-     * @param array|null $attrs (optional)
      * @param string|null $ns (optional)
+     * @param array|null $attrs (optional)
      * @return mixed
      */
-    public function sub(string $name, ?array $attrs = null, ?string $ns = null)
+    public function sub(string $name, ?string $ns = null, ?array $attrs = null)
     {
-        #TODO attrs is ignored
+        // @todo attrs is ignored
         foreach ($this->subs as $sub) {
-            if ($sub->name == $name and ($ns == null or $sub->ns == $ns)) {
+            if ($sub->name === $name && (is_null($ns) || $sub->ns === $ns)) {
                 return $sub;
             }
         }

vendor/diogocomposer/xmpphp/XMPPHP/XMLStream.php

@@ -374,13 +374,10 @@ class XMLStream
      */
     public function event(string $name, ?array $payload = null): void
     {
-        $this->log->log("EVENT: $name", Log::LEVEL_DEBUG);
+        $this->log->log("EVENT: {$name}", Log::LEVEL_DEBUG);
         foreach ($this->eventhandlers as $handler) {
-            if ($name == $handler[0]) {
+            if ($handler[0] === $name) {
-                if ($handler[2] === null) {
+                call_user_func_array($handler[1], [&$payload]);
-                    $handler[2] = $this;
-                }
-                $handler[2]->{$handler[1]}($payload);
             }
         }
         foreach ($this->until as $key => $until) {
@@ -505,12 +502,12 @@ class XMLStream
     /**
      * Get next ID
      *
-     * @return int
+     * @return string
      */
-    public function getId(): int
+    public function getId(): string
     {
-        $this->lastid++;
+        ++$this->lastid;
-        return $this->lastid;
+        return (string) $this->lastid;
     }
 
     /**
@@ -522,16 +519,45 @@ class XMLStream
         $this->use_ssl = $use;
     }
 
+    /**
+     * Compose a proper callable if given legacy syntax
+     *
+     * @param callable|string $pointer
+     * @param object|null|bool $obj
+     * @return callable
+     * @throws InvalidArgumentException
+     */
+    protected function ensureHandler($pointer, $obj = false): callable
+    {
+        $handler = $pointer;
+
+        if (is_string($pointer)) {
+            if (is_object($obj)) {
+                $handler = [$obj, $pointer];
+            } elseif (is_null($obj)) {
+                // Questionable behaviour for backwards compatibility
+                $handler = [$this, $pointer];
+            }
+        }
+
+        if (!is_callable($handler)) {
+            throw new \InvalidArgumentException(
+                'Cannot compose a proper callable'
+            );
+        }
+        return $handler;
+    }
+
     /**
      * Add ID Handler
      *
      * @param int $id
-     * @param string $pointer
+     * @param callable|string $pointer
-     * @param string|null $obj
+     * @param object|bool|null $obj
      */
-    public function addIdHandler(int $id, string $pointer, ?string $obj = null): void
+    public function addIdHandler(string $id, $pointer, $obj = null): void
     {
-        $this->idhandlers[$id] = [$pointer, $obj];
+        $this->idhandlers[$id] = [$this->ensureHandler($pointer, $obj)];
     }
 
     /**
@@ -540,52 +566,49 @@ class XMLStream
      * @param string $name
      * @param string $ns
      * @param string $pointer
-     * @param string|null $obj
+     * @param object|bool|null $obj
      * @param int $depth
      *
-     * public function addHandler(string $name, string $ns, string $pointer, ?string $obj = null, int $depth = 1): void
+     * public function addHandler(string $name, string $ns, $pointer, $obj = null, int $depth = 1): void
      * {
-     * #TODO deprication warning
+     *     // TODO deprecation warning
-     * $this->nshandlers[] = [$name, $ns, $pointer, $obj, $depth];
+     *     $this->nshandlers[] = [$name, $ns, $this->ensureHandler($pointer, $obj), $depth];
      * }*/
 
     /**
      * Add XPath Handler
      *
      * @param string $xpath
-     * @param string $pointer
+     * @param callable|string $pointer
-     * @param string|null $obj
+     * @param object|bool|null $obj
      */
-    public function addXPathHandler(string $xpath, string $pointer, ?string $obj = null): void
+    public function addXPathHandler(string $xpath, $pointer, $obj = null): void
     {
-        if (preg_match_all("/\(?{[^\}]+}\)?(\/?)[^\/]+/", $xpath, $regs)) {
+        if (preg_match_all('/\/?(\{[^\}]+\})?[^\/]+/', $xpath, $regs)) {
-            $ns_tags = $regs[0];
+            $tag = $regs[0];
         } else {
-            $ns_tags = [$xpath];
+            $tag = [$xpath];
         }
         $xpath_array = [];
-        foreach ($ns_tags as $ns_tag) {
+        foreach ($tag as $t) {
-            list($l, $r) = explode("}", $ns_tag);
+            $t = ltrim($t, '/');
-            if ($r != null) {
+            preg_match('/(\{([^\}]+)\})?(.*)/', $t, $regs);
-                $xpart = [substr($l, 1), $r];
+            $xpath_array[] = [$regs[2], $regs[3]];
-            } else {
-                $xpart = [null, $l];
-            }
-            $xpath_array[] = $xpart;
         }
-        $this->xpathhandlers[] = [$xpath_array, $pointer, $obj];
+
+        $this->xpathhandlers[] = [$xpath_array, $this->ensureHandler($pointer, $obj)];
     }
 
     /**
      * Add Event Handler
      *
      * @param string $name
-     * @param string $pointer
+     * @param callable|string $pointer
-     * @param object $obj
+     * @param object|bool|null $obj
      */
-    public function addEventHandler(string $name, string $pointer, object $obj)
+    public function addEventHandler(string $name, $pointer, $obj = null): void
     {
-        $this->eventhandlers[] = [$name, $pointer, $obj];
+        $this->eventhandlers[] = [$name, $this->ensureHandler($pointer, $obj)];
     }
 
     /**
@@ -719,12 +742,8 @@ class XMLStream
                                 break;
                             }
                         }
-                        if ($searchxml !== null) {
+                        if (!is_null($searchxml)) {
-                            if ($handler[2] === null) {
+                            call_user_func_array($handler[1], [&$this->xmlobj[2]]);
-                                $handler[2] = $this;
-                            }
-                            $this->log->log("Calling {$handler[1]}", Log::LEVEL_DEBUG);
-                            $handler[2]->{$handler[1]}($this->xmlobj[2]);
                         }
                     }
                 }
@@ -735,21 +754,25 @@ class XMLStream
                 } elseif (is_array($this->xmlobj) and array_key_exists(2, $this->xmlobj)) {
                     $searchxml = $this->xmlobj[2];
                 }
-                if ($searchxml !== null and $searchxml->name == $handler[0] and ($searchxml->ns == $handler[1] or (!$handler[1] and $searchxml->ns == $this->default_ns))) {
+                if (
-                    if ($handler[3] === null) {
+                    !is_null($searchxml)
-                        $handler[3] = $this;
+                    && $searchxml->name === $handler[0]
-                    }
+                    && (
-                    $this->log->log("Calling {$handler[2]}", Log::LEVEL_DEBUG);
+                        (!$handler[1] && $searchxml->ns === $this->default_ns)
-                    $handler[3]->{$handler[2]}($this->xmlobj[2]);
+                        || $searchxml->ns === $handler[1]
+                    )
+                ) {
+                    call_user_func_array($handler[2], [&$this->xmlobj[2]]);
                 }
             }
             foreach ($this->idhandlers as $id => $handler) {
-                if (array_key_exists('id', $this->xmlobj[2]->attrs) and $this->xmlobj[2]->attrs['id'] == $id) {
+                if (
-                    if ($handler[1] === null) {
+                    array_key_exists(2, $this->xmlobj)
-                        $handler[1] = $this;
+                    && array_key_exists('id', $this->xmlobj[2]->attrs)
-                    }
+                    && $this->xmlobj[2]->attrs['id'] === (string) $id
-                    $handler[1]->{$handler[0]}($this->xmlobj[2]);
+                ) {
-                    #id handlers are only used once
+                    call_user_func_array($handler[0], [&$this->xmlobj[2]]);
+                    // id handlers are only used once
                     unset($this->idhandlers[$id]);
                     break;
                 }

vendor/diogocomposer/xmpphp/XMPPHP/XMPP.php

@@ -135,13 +135,34 @@ class XMPP extends XMLStream
         $this->stream_end = '</stream:stream>';
         $this->default_ns = 'jabber:client';
 
-        $this->addXPathHandler('{http://etherx.jabber.org/streams}features', 'features_handler');
+        $this->addXPathHandler(
-        $this->addXPathHandler('{urn:ietf:params:xml:ns:xmpp-sasl}success', 'sasl_success_handler');
+            '{http://etherx.jabber.org/streams}features',
-        $this->addXPathHandler('{urn:ietf:params:xml:ns:xmpp-sasl}failure', 'sasl_failure_handler');
+            [$this, 'features_handler']
-        $this->addXPathHandler('{urn:ietf:params:xml:ns:xmpp-tls}proceed', 'tls_proceed_handler');
+        );
-        $this->addXPathHandler('{jabber:client}message', 'message_handler');
+        $this->addXPathHandler(
-        $this->addXPathHandler('{jabber:client}presence', 'presence_handler');
+            '{urn:ietf:params:xml:ns:xmpp-sasl}success',
-        $this->addXPathHandler('iq/{jabber:iq:roster}query', 'roster_iq_handler');
+            [$this, 'sasl_success_handler']
+        );
+        $this->addXPathHandler(
+            '{urn:ietf:params:xml:ns:xmpp-sasl}failure',
+            [$this, 'sasl_failure_handler']
+        );
+        $this->addXPathHandler(
+            '{urn:ietf:params:xml:ns:xmpp-tls}proceed',
+            [$this, 'tls_proceed_handler']
+        );
+        $this->addXPathHandler(
+            '{jabber:client}message',
+            [$this, 'message_handler']
+        );
+        $this->addXPathHandler(
+            '{jabber:client}presence',
+            [$this, 'presence_handler']
+        );
+        $this->addXPathHandler(
+            'iq/{jabber:iq:roster}query',
+            [$this, 'roster_iq_handler']
+        );
     }
 
     /**
@@ -326,8 +347,8 @@ class XMPP extends XMLStream
      */
     public function getVCard(?string $jid = null): void
     {
-        $id = $this->getID();
+        $id = $this->getId();
-        $this->addIdHandler($id, 'vcard_get_handler');
+        $this->addIdHandler($id, [$this, 'vcard_get_handler']);
         if ($jid) {
             $this->send("<iq type='get' id='$id' to='$jid'><vCard xmlns='vcard-temp' /></iq>");
         } else {
@@ -347,7 +368,7 @@ class XMPP extends XMLStream
             $this->send("<starttls xmlns='urn:ietf:params:xml:ns:xmpp-tls'><required /></starttls>");
         } elseif ($xml->hasSub('bind') and $this->authed) {
             $id = $this->getId();
-            $this->addIdHandler($id, 'resource_bind_handler');
+            $this->addIdHandler($id, [$this, 'resource_bind_handler']);
             $this->send("<iq xmlns=\"jabber:client\" type=\"set\" id=\"$id\"><bind xmlns=\"urn:ietf:params:xml:ns:xmpp-bind\"><resource>{$this->resource}</resource></bind></iq>");
         } else {
             $this->log->log("Attempting Auth...");
@@ -401,7 +422,7 @@ class XMPP extends XMLStream
             $this->jid = $jidarray[0];
         }
         $id = $this->getId();
-        $this->addIdHandler($id, 'session_start_handler');
+        $this->addIdHandler($id, [$this, 'session_start_handler']);
         $this->send("<iq xmlns='jabber:client' type='set' id='$id'><session xmlns='urn:ietf:params:xml:ns:xmpp-session' /></iq>");
     }
 
@@ -414,28 +435,30 @@ class XMPP extends XMLStream
      */
     protected function roster_iq_handler(XMLObj $xml): void
     {
-        $status = "result";
+        $status = 'result';
         $xmlroster = $xml->sub('query');
         $contacts = [];
         foreach ($xmlroster->subs as $item) {
             $groups = [];
-            if ($item->name == 'item') {
+            if ($item->name === 'item') {
-                $jid = $item->attrs['jid']; //REQUIRED
+                $jid = $item->attrs['jid'];  // REQUIRED
-                $name = $item->attrs['name']; //MAY
+                $name = $item->attrs['name'] ?? '';
-                $subscription = $item->attrs['subscription'];
+                $subscription = $item->attrs['subscription'] ?? 'none';
                 foreach ($item->subs as $subitem) {
-                    if ($subitem->name == 'group') {
+                    if ($subitem->name === 'group') {
                         $groups[] = $subitem->data;
                     }
                 }
-                $contacts[] = [$jid, $subscription, $name, $groups]; //Store for action if no errors happen
+                // Store for action if no errors happen
+                $contacts[] = [$jid, $subscription, $name, $groups];
             } else {
-                $status = "error";
+                $status = 'error';
             }
         }
-        if ($status == "result") { //No errors, add contacts
+        // No errors, add contacts
+        if ($status === 'result') {
             foreach ($contacts as $contact) {
-                $this->roster->addContact($contact[0], $contact[1], $contact[2], $contact[3]);
+                $this->roster->addContact(...$contact);
             }
         }
         if ($xml->attrs['type'] == 'set') {