Icinga / icingaweb2-module-elasticsearch

This module will not be updated by Icinga anymore. Please don't attempt to use it.
GNU General Public License v2.0
29 stars 9 forks source link

count(): Parameter must be an array or an object that implements Countable #38

Closed klippo closed 5 years ago

klippo commented 6 years ago

There is a problem getting results when using PHP 7.2

count(): Parameter must be an array or an object that implements Countable

#0 [internal function]: Icinga\Application\ApplicationBootstrap->Icinga\Application\{closure}(Integer, String, String, Integer, Array)
#1 /usr/share/icingaweb2/modules/elasticsearch/library/vendor/iplx/Http/Client.php(180): count(NULL)
#2 /usr/share/icingaweb2/modules/elasticsearch/library/vendor/iplx/Http/Client.php(195): iplx\Http\Client->executeHandle(Object(iplx\Http\Handle))
#3 /usr/share/icingaweb2/modules/elasticsearch/library/Elasticsearch/Query.php(147): iplx\Http\Client->send(Object(iplx\Http\Request), Array)
#4 /usr/share/icingaweb2/modules/elasticsearch/library/Elasticsearch/Query.php(178): Icinga\Module\Elasticsearch\Query->execute()
#5 /usr/share/icingaweb2/modules/elasticsearch/application/controllers/EventsController.php(106): Icinga\Module\Elasticsearch\Query->fetchAll()
#6 /usr/share/icingaweb2/library/vendor/Zend/Controller/Action.php(507): Icinga\Module\Elasticsearch\Controllers\EventsController->indexAction()
#7 /usr/share/php/Icinga/Web/Controller/Dispatcher.php(76): Zend_Controller_Action->dispatch(String)
#8 /usr/share/icingaweb2/library/vendor/Zend/Controller/Front.php(937): Icinga\Web\Controller\Dispatcher->dispatch(Object(Icinga\Web\Request), Object(Icinga\Web\Response))
#9 /usr/share/php/Icinga/Application/Web.php(300): Zend_Controller_Front->dispatch(Object(Icinga\Web\Request), Object(Icinga\Web\Response))
#10 /usr/share/php/Icinga/Application/webrouter.php(104): Icinga\Application\Web->dispatch()
#11 /usr/share/icingaweb2/public/index.php(4): require_once(String)
#12 {main}

After doing a quick google search I tested the following change with success, with limited knowledge of the code I don't want to submit a PR.

diff --git a/library/vendor/iplx/Http/Client.php b/library/vendor/iplx/Http/Client.php
index 5beb60f..129dddb 100644
--- a/library/vendor/iplx/Http/Client.php
+++ b/library/vendor/iplx/Http/Client.php
@@ -177,7 +177,7 @@ class Client implements ClientInterface
             curl_getinfo($ch, CURLINFO_HTTP_CODE), $handle->responseHeaders, $handle->responseBody
         );

-        if (count($this->handles) >= self::MAX_HANDLES) {
+        if ($this->handles && count($this->handles) >= self::MAX_HANDLES) {
             curl_close($ch);
         } else {
             curl_reset($ch);

Versions:

\# cat /etc/os-release 
NAME="Ubuntu"
VERSION="18.04.1 LTS (Bionic Beaver)"
ID=ubuntu
ID_LIKE=debian
PRETTY_NAME="Ubuntu 18.04.1 LTS"

\# dpkg -l | grep php

ii  libapache2-mod-php7.2                7.2.7-0ubuntu0.18.04.2             amd64        server-side, HTML-embedded scripting language (Apache 2 module)
ii  php                                  1:7.2+60ubuntu1                    all          server-side, HTML-embedded scripting language (default)
ii  php-common                           1:60ubuntu1                        all          Common files for PHP packages
ii  php-curl                             1:7.2+60ubuntu1                    all          CURL module for PHP [default]
ii  php-htmlpurifier                     4.7.0-2                            all          Standards-compliant HTML filter
ii  php-icinga                           2.6.0-1.bionic                     all          PHP library to communicate with and use Icinga
ii  php-imagick                          3.4.3~rc2-2ubuntu4                 amd64        Provides a wrapper to the ImageMagick library
ii  php-intl                             1:7.2+60ubuntu1                    all          Internationalisation module for PHP [default]
ii  php-ldap                             1:7.2+60ubuntu1                    all          LDAP module for PHP [default]
ii  php-mysql                            1:7.2+60ubuntu1                    all          MySQL module for PHP [default]
ii  php-xml                              1:7.2+60ubuntu1                    all          DOM, SimpleXML, WDDX, XML, and XSL module for PHP [default]
ii  php7.2                               7.2.7-0ubuntu0.18.04.2             all          server-side, HTML-embedded scripting language (metapackage)
ii  php7.2-cli                           7.2.7-0ubuntu0.18.04.2             amd64        command-line interpreter for the PHP scripting language
ii  php7.2-common                        7.2.7-0ubuntu0.18.04.2             amd64        documentation, examples and common module for PHP
ii  php7.2-curl                          7.2.7-0ubuntu0.18.04.2             amd64        CURL module for PHP
ii  php7.2-intl                          7.2.7-0ubuntu0.18.04.2             amd64        Internationalisation module for PHP
ii  php7.2-json                          7.2.7-0ubuntu0.18.04.2             amd64        JSON module for PHP
ii  php7.2-ldap                          7.2.7-0ubuntu0.18.04.2             amd64        LDAP module for PHP
ii  php7.2-mysql                         7.2.7-0ubuntu0.18.04.2             amd64        MySQL module for PHP
ii  php7.2-opcache                       7.2.7-0ubuntu0.18.04.2             amd64        Zend OpCache module for PHP
ii  php7.2-readline                      7.2.7-0ubuntu0.18.04.2             amd64        readline module for PHP
ii  php7.2-xml                           7.2.7-0ubuntu0.18.04.2             amd64        DOM, SimpleXML, WDDX, XML, and XSL module for PHP
SkUrRiEr commented 6 years ago

A better solution is to initialise the $handles property as an array.

This appears to be a class pulled from some other vendor, maybe it needs to be updated?

mdetrano commented 5 years ago

Casting 'handles' to array also seems to work.

--- Client.php.orig     2019-06-13 11:45:35.361486290 -0600
+++ Client.php  2019-06-13 11:44:46.978109913 -0600
@@ -177,7 +177,7 @@
             curl_getinfo($ch, CURLINFO_HTTP_CODE), $handle->responseHeaders, $handle->responseBody
         );

-        if (count($this->handles) >= self::MAX_HANDLES) {
+        if (count((array) $this->handles) >= self::MAX_HANDLES) {
             curl_close($ch);
         } else {
             curl_reset($ch);
dolito commented 5 years ago

Initialize with $handles = []; would be indeed better - without checking the references.

But, it doesn't matter until someone is maintaining this repo. There are 2 or 3 issues for this and at least 1 pull request.

This extension deserves better, since it's working quite nice. But this minor error destroys a lot of the user experience.