Cacti / plugin_syslog

Syslog Plugin for Cacti
GNU General Public License v2.0
21 stars 16 forks source link

syslog plugin and PHP 8 #160

Closed ARC1450 closed 2 years ago

ARC1450 commented 2 years ago

On my Cacti 1.2.17 install, when Gentoo decided to upgrade to PHP 8.0.11, I started getting a lot of these deprecation errors in my logs:

2021/10/18 10:59:15 - ERROR PHP DEPRECATED in Plugin 'syslog': Required parameter $db_ssl_ca follows optional parameter $port in file: /var/www/localhost/htdocs/cacti/plugins/syslog/database.php on line: 38

When checking database.php, $port was in fact ahead of $db_ssl_ca in the argument list.

/* syslog_db_connect_real - makes a connection to the database server
   @arg $host - the hostname of the database server, 'localhost' if the database server is running
      on this machine
   @arg $user - the username to connect to the database server as
   @arg $pass - the password to connect to the database server with
   @arg $db_name - the name of the database to connect to
   @arg $db_type - the type of database server to connect to, only 'mysql' is currently supported
   @arg $retries - the number a time the server should attempt to connect before failing
   @arg $db_ssl - true or false, is the database using ssl
   @arg $db_ssl_key - the path to the ssl key file
   @arg $db_ssl_cert - the path to the ssl cert file
   @arg $db_ssl_ca - the path to the ssl ca file
   @returns - (object) connection_id for success, (bool) '0' for error */
function syslog_db_connect_real($host, $user, $pass, $db_name, $db_type, $port = '3306', $retries = 20, $db_ssl,
        $db_ssl_key, $db_ssl_cert, $db_ssl_ca) {
        return db_connect_real($host, $user, $pass, $db_name, $db_type, $port, $retries, $db_ssl, $db_ssl_key, $db_ssl_cert, $db_ssl_ca);
}

$port also is a statically defined argument which is a little weird considering the config file for the plugin has an variable that defines the DB port.

global $config, $database_type, $database_default, $database_hostname;
global $database_username, $database_password, $database_port;
global $database_ssl, $database_ssl_key, $database_ssl_cert, $database_ssl_ca;

/* revert if you dont use the Cacti database */
$use_cacti_db = false;

if (!$use_cacti_db) {
        $syslogdb_type     = 'mysql';
        $syslogdb_default  = 'syslog';
        $syslogdb_hostname = 'name.db.comnetorglocal';
        $syslogdb_username = 's3kr1t_lu53r';
        $syslogdb_password = 's3kr1t_p@ssw0rd';
        $syslogdb_port     = 3306;
        $syslogdb_retries  = 5;
        $syslogdb_ssl      = false;
        $syslogdb_ssl_key  = '';
        $syslogdb_ssl_cert = '';
        $syslogdb_ssl_ca   = '';
} else {
        $syslogdb_type     = $database_type;
        $syslogdb_default  = $database_default;
        $syslogdb_hostname = $database_hostname;
        $syslogdb_username = $database_username;
        $syslogdb_password = $database_password;
        $syslogdb_port     = $database_port;
        $syslogdb_retries  = 5;
        $syslogdb_ssl      = $database_ssl;
        $syslogdb_ssl_key  = $database_ssl_key;
        $syslogdb_ssl_cert = $database_ssl_cert;
        $syslogdb_ssl_ca   = $database_ssl_ca;
}
netniV commented 2 years ago

The certificate should be optionally since no everyone is using it. There's a mistake there. But the the reason for allowing the parameters for any of the parameters is to make the function generic and allow flexibility.

If you add an = '' I presume your PHP 8 error is removed?

ARC1450 commented 2 years ago

No clue. I forced a downgrade back to PHP 7.4 as I wasn't in the mood for the headache of troubleshooting and there were a couple of other deprecation errors (one was for flowview and I think another was for Cacti in general). :(

I would imagine just adding "@arg $port" to this list would help since it would make "$port" mandatory. I'm no PHP wiz, though.

   @arg $host - the hostname of the database server, 'localhost' if the database server is running
      on this machine
   @arg $user - the username to connect to the database server as
   @arg $pass - the password to connect to the database server with
   @arg $db_name - the name of the database to connect to
   @arg $db_type - the type of database server to connect to, only 'mysql' is currently supported
   @arg $retries - the number a time the server should attempt to connect before failing
   @arg $db_ssl - true or false, is the database using ssl
   @arg $db_ssl_key - the path to the ssl key file
   @arg $db_ssl_cert - the path to the ssl cert file
   @arg $db_ssl_ca - the path to the ssl ca file
netniV commented 2 years ago

That is just the documentation of the function, not the actual parameters.

TheWitness commented 2 years ago

This is resolved from my perspective. I'll be making a change shortly for the $database_retries setting.