Store config data serialized (#10651)

* Store config data serialized
This way we can store null, booleans, and more reliably.

* Use model to get the mutated output.

* fix whitespace and unused function

* use json_encode/decode and casts
migration to transfer

* json_encode JSON_UNESCAPED_SLASHES

* Use JSON_UNESCAPED_SLASHES.  That is only relevant if you are printing into an HTML page.

* pre-encode the seed...

* filter other fields besides config_value

LibreNMS/Config.php

@@ -44,7 +44,7 @@ class Config
         self::loadFiles();
 
         // Make sure the database is connected
-        if (Eloquent::isConnected() || (function_exists('dbIsConnected') && dbIsConnected())) {
+        if (Eloquent::isConnected()) {
             // pull in the database config settings
             self::mergeDb();
 
@@ -244,39 +244,23 @@ class Config
         global $config;
 
         if ($persist) {
-            if (Eloquent::isConnected()) {
-                try {
-                    $config_array = collect([
-                        'config_name' => $key,
-                        'config_value' => $value,
-                        'config_default' => $default,
-                        'config_descr' => $descr,
-                        'config_group' => $group,
-                        'config_sub_group' => $sub_group,
-                    ])->filter(function ($value) {
-                        return !is_null($value);
-                    })->toArray();
-
-                    \App\Models\Config::updateOrCreate(['config_name' => $key], $config_array);
-                } catch (QueryException $e) {
-                    // possibly table config doesn't exist yet
-                    global $debug;
-                    if ($debug) {
-                        echo $e;
-                    }
+            try {
+                \App\Models\Config::updateOrCreate(['config_name' => $key], collect([
+                    'config_name' => $key,
+                    'config_default' => $default,
+                    'config_descr' => $descr,
+                    'config_group' => $group,
+                    'config_sub_group' => $sub_group,
+                ])->filter(function ($value, $field) {
+                    return !is_null($value);
+                })->put('config_value', $value)->toArray());
+            } catch (QueryException $e) {
+                if (class_exists(\Log::class)) {
+                    \Log::error($e);
                 }
-            } else {
-                $res = dbUpdate(array('config_value' => $value), 'config', '`config_name`=?', array($key));
-                if (!$res && !dbFetchCell('SELECT 1 FROM `config` WHERE `config_name`=?', array($key))) {
-                    $insert = array(
-                        'config_name' => $key,
-                        'config_value' => $value,
-                        'config_default' => $default,
-                        'config_descr' => $descr,
-                        'config_group' => $group,
-                        'config_sub_group' => $sub_group,
-                    );
-                    dbInsert($insert, 'config');
+                global $debug;
+                if ($debug) {
+                    echo $e;
                 }
             }
         }
@@ -356,68 +340,27 @@ class Config
 
         $db_config = [];
 
-        if (Eloquent::isConnected()) {
-            try {
-                \App\Models\Config::get(['config_name', 'config_value'])
-                    ->each(function ($item) use (&$db_config) {
-                        array_set($db_config, $item->config_name, $item->config_value);
-                    });
-            } catch (QueryException $e) {
-                // possibly table config doesn't exist yet
-            }
-
-        } else {
-            foreach (dbFetchRows('SELECT `config_name`,`config_value` FROM `config`') as $obj) {
-                self::assignArrayByPath($db_config, $obj['config_name'], $obj['config_value']);
-            }
+        try {
+            \App\Models\Config::get(['config_name', 'config_value'])
+                ->each(function ($item) use (&$db_config) {
+                    Arr::set($db_config, $item->config_name, $item->config_value);
+                });
+        } catch (QueryException $e) {
+            // possibly table config doesn't exist yet
         }
 
         $config = array_replace_recursive($db_config, $config);
     }
 
-    /**
-     * Assign a value into the passed array by a path
-     * 'snmp.version' = 'v1' becomes $arr['snmp']['version'] = 'v1'
-     *
-     * @param array $arr the array to insert the value into, will be modified in place
-     * @param string $path the path to insert the value at
-     * @param mixed $value the value to insert, will be type cast
-     * @param string $separator path separator
-     */
-    private static function assignArrayByPath(&$arr, $path, $value, $separator = '.')
-    {
-        // type cast value. Is this needed here?
-        if (filter_var($value, FILTER_VALIDATE_INT)) {
-            $value = (int)$value;
-        } elseif (filter_var($value, FILTER_VALIDATE_FLOAT)) {
-            $value = (float)$value;
-        } elseif (filter_var($value, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE) !== null) {
-            $value = filter_var($value, FILTER_VALIDATE_BOOLEAN);
-        }
-
-        $keys = explode($separator, $path);
-
-        // walk the array creating keys if they don't exist
-        foreach ($keys as $key) {
-            $arr = &$arr[$key];
-        }
-        // assign the variable
-        $arr = $value;
-    }
-
     private static function loadGraphsFromDb()
     {
         global $config;
 
-        if (Eloquent::isConnected()) {
-            try {
-                $graph_types = GraphType::all()->toArray();
-            } catch (QueryException $e) {
-                // possibly table config doesn't exist yet
-                $graph_types = [];
-            }
-        } else {
-            $graph_types = dbFetchRows('SELECT * FROM graph_types');
+        try {
+            $graph_types = GraphType::all()->toArray();
+        } catch (QueryException $e) {
+            // possibly table config doesn't exist yet
+            $graph_types = [];
         }
 
         // load graph types from the database

app/Models/Config.php

@@ -45,32 +45,17 @@ class Config extends BaseModel
         'config_group' => '',
         'config_sub_group' => '',
     ];
+    protected $casts = [
+        'config_default' => 'array'
+    ];
 
-    /**
-     * Get the config_value (type cast)
-     *
-     * @param string $value
-     * @return mixed
-     */
     public function getConfigValueAttribute($value)
     {
-        if (filter_var($value, FILTER_VALIDATE_INT)) {
-            return (int)$value;
-        } elseif (filter_var($value, FILTER_VALIDATE_FLOAT)) {
-            return (float)$value;
-        } elseif (filter_var($value, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE) !== null) {
-            return filter_var($value, FILTER_VALIDATE_BOOLEAN);
-        }
-
-        return $value;
+        return json_decode($value);
     }
 
     public function setConfigValueAttribute($value)
     {
-        if (is_bool($value)) {
-            $this->attributes['config_value'] = $value ? 'true' : 'false';
-        } else {
-            $this->attributes['config_value'] = $value;
-        }
+        $this->attributes['config_value'] = json_encode($value, JSON_UNESCAPED_SLASHES);
     }
 }

database/migrations/2019_10_03_211702_serialize_config.php

@@ -0,0 +1,49 @@
+<?php
+
+use Illuminate\Support\Facades\Schema;
+use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Database\Migrations\Migration;
+
+class SerializeConfig extends Migration
+{
+    /**
+     * Run the migrations.
+     *
+     * @return void
+     */
+    public function up()
+    {
+        DB::table('config')->get()->each(function ($config) {
+            $value = $config->config_value;
+
+            if (filter_var($value, FILTER_VALIDATE_INT)) {
+                $value = (int)$value;
+            } elseif (filter_var($value, FILTER_VALIDATE_FLOAT)) {
+                $value = (float)$value;
+            } elseif (filter_var($value, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE) !== null) {
+                $value = filter_var($value, FILTER_VALIDATE_BOOLEAN);
+            }
+
+            DB::table('config')
+                ->where('config_id', $config->config_id)
+                ->update(['config_value' => json_encode($value)]);
+        });
+    }
+
+    /**
+     * Reverse the migrations.
+     *
+     * @return void
+     */
+    public function down()
+    {
+        DB::table('config')->get()->each(function ($config) {
+            $value = json_decode($config->config_value);
+            $value = is_bool($value) ? var_export($value, true) : (string)$value;
+
+            DB::table('config')
+                ->where('config_id', $config->config_id)
+                ->update(['config_value' => $value]);
+        });
+    }
+}

database/seeds/DefaultConfigSeeder.php

@@ -62,8 +62,8 @@ class DefaultConfigSeeder extends Seeder
             ],
             [
                 "config_name" => "alert.default_mail",
-                "config_value" => "",
-                "config_default" => "",
+                "config_value" => '""',
+                "config_default" => '""',
                 "config_descr" => "The default mail contact",
                 "config_group" => "alerting",
                 "config_group_order" => "0",
@@ -99,7 +99,7 @@ class DefaultConfigSeeder extends Seeder
             [
                 "config_name" => "alert.fixed-contacts",
                 "config_value" => "true",
-                "config_default" => "TRUE",
+                "config_default" => "true",
                 "config_descr" => "If TRUE any changes to sysContact or users emails will not be honoured whilst alert is active",
                 "config_group" => "alerting",
                 "config_group_order" => "0",
@@ -111,7 +111,7 @@ class DefaultConfigSeeder extends Seeder
             [
                 "config_name" => "alert.globals",
                 "config_value" => "true",
-                "config_default" => "TRUE",
+                "config_default" => "true",
                 "config_descr" => "Alert read only administrators",
                 "config_group" => "alerting",
                 "config_group_order" => "0",
@@ -123,7 +123,7 @@ class DefaultConfigSeeder extends Seeder
             [
                 "config_name" => "alert.syscontact",
                 "config_value" => "true",
-                "config_default" => "TRUE",
+                "config_default" => "true",
                 "config_descr" => "Issue alerts to sysContact",
                 "config_group" => "alerting",
                 "config_group_order" => "0",
@@ -182,8 +182,8 @@ class DefaultConfigSeeder extends Seeder
             ],
             [
                 "config_name" => "email_backend",
-                "config_value" => "mail",
-                "config_default" => "mail",
+                "config_value" => '"mail"',
+                "config_default" => '"mail"',
                 "config_descr" => "The backend to use for sending email, can be mail, sendmail or smtp",
                 "config_group" => "alerting",
                 "config_group_order" => "0",
@@ -194,8 +194,8 @@ class DefaultConfigSeeder extends Seeder
             ],
             [
                 "config_name" => "email_from",
-                "config_value" => "NULL",
-                "config_default" => "NULL",
+                "config_value" => "null",
+                "config_default" => "null",
                 "config_descr" => "Email address used for sending emails (from)",
                 "config_group" => "alerting",
                 "config_group_order" => "0",
@@ -218,8 +218,8 @@ class DefaultConfigSeeder extends Seeder
             ],
             [
                 "config_name" => "email_sendmail_path",
-                "config_value" => "/usr/sbin/sendmail",
-                "config_default" => "/usr/sbin/sendmail",
+                "config_value" => '"/usr/sbin/sendmail"',
+                "config_default" => '"/usr/sbin/sendmail"',
                 "config_descr" => "Location of sendmail if using this option",
                 "config_group" => "alerting",
                 "config_group_order" => "0",
@@ -231,7 +231,7 @@ class DefaultConfigSeeder extends Seeder
             [
                 "config_name" => "email_smtp_auth",
                 "config_value" => "false",
-                "config_default" => "FALSE",
+                "config_default" => "false",
                 "config_descr" => "Enable / disable smtp authentication",
                 "config_group" => "alerting",
                 "config_group_order" => "0",
@@ -242,8 +242,8 @@ class DefaultConfigSeeder extends Seeder
             ],
             [
                 "config_name" => "email_smtp_host",
-                "config_value" => "localhost",
-                "config_default" => "localhost",
+                "config_value" => '"localhost"',
+                "config_default" => '"localhost"',
                 "config_descr" => "SMTP Host for sending email if using this option",
                 "config_group" => "alerting",
                 "config_group_order" => "0",
@@ -254,8 +254,8 @@ class DefaultConfigSeeder extends Seeder
             ],
             [
                 "config_name" => "email_smtp_password",
-                "config_value" => "NULL",
-                "config_default" => "NULL",
+                "config_value" => "null",
+                "config_default" => "null",
                 "config_descr" => "SMTP Auth password",
                 "config_group" => "alerting",
                 "config_group_order" => "0",
@@ -278,8 +278,8 @@ class DefaultConfigSeeder extends Seeder
             ],
             [
                 "config_name" => "email_smtp_secure",
-                "config_value" => "",
-                "config_default" => "",
+                "config_value" => '""',
+                "config_default" => '""',
                 "config_descr" => "Enable / disable encryption (use tls or ssl)",
                 "config_group" => "alerting",
                 "config_group_order" => "0",
@@ -302,8 +302,8 @@ class DefaultConfigSeeder extends Seeder
             ],
             [
                 "config_name" => "email_smtp_username",
-                "config_value" => "NULL",
-                "config_default" => "NULL",
+                "config_value" => "null",
+                "config_default" => "null",
                 "config_descr" => "SMTP Auth username",
                 "config_group" => "alerting",
                 "config_group_order" => "0",
@@ -314,8 +314,8 @@ class DefaultConfigSeeder extends Seeder
             ],
             [
                 "config_name" => "email_user",
-                "config_value" => "LibreNMS",
-                "config_default" => "LibreNMS",
+                "config_value" => '"LibreNMS"',
+                "config_default" => '"LibreNMS"',
                 "config_descr" => "Name used as part of the from address",
                 "config_group" => "alerting",
                 "config_group_order" => "0",
@@ -326,8 +326,8 @@ class DefaultConfigSeeder extends Seeder
             ],
             [
                 "config_name" => "fping",
-                "config_value" => "/usr/sbin/fping",
-                "config_default" => "fping",
+                "config_value" => '"/usr/sbin/fping"',
+                "config_default" => '"fping"',
                 "config_descr" => "Path to fping",
                 "config_group" => "external",
                 "config_group_order" => "0",
@@ -338,8 +338,8 @@ class DefaultConfigSeeder extends Seeder
             ],
             [
                 "config_name" => "fping6",
-                "config_value" => "/usr/sbin/fping6",
-                "config_default" => "fping6",
+                "config_value" => '"/usr/sbin/fping6"',
+                "config_default" => '"fping6"',
                 "config_descr" => "Path to fping6",
                 "config_group" => "external",
                 "config_group_order" => "0",
@@ -350,8 +350,8 @@ class DefaultConfigSeeder extends Seeder
             ],
             [
                 "config_name" => "geoloc.api_key",
-                "config_value" => "",
-                "config_default" => "",
+                "config_value" => '""',
+                "config_default" => '""',
                 "config_descr" => "Geocoding API Key (Required to function)",
                 "config_group" => "external",
                 "config_group_order" => "0",
@@ -362,8 +362,8 @@ class DefaultConfigSeeder extends Seeder
             ],
             [
                 "config_name" => "geoloc.engine",
-                "config_value" => "",
-                "config_default" => "",
+                "config_value" => '""',
+                "config_default" => '""',
                 "config_descr" => "Geocoding Engine",
                 "config_group" => "external",
                 "config_group_order" => "0",
@@ -374,8 +374,8 @@ class DefaultConfigSeeder extends Seeder
             ],
             [
                 "config_name" => "oxidized.default_group",
-                "config_value" => "",
-                "config_default" => "",
+                "config_value" => '""',
+                "config_default" => '""',
                 "config_descr" => "Set the default group returned",
                 "config_group" => "external",
                 "config_group_order" => "0",
@@ -434,8 +434,8 @@ class DefaultConfigSeeder extends Seeder
             ],
             [
                 "config_name" => "oxidized.url",
-                "config_value" => "",
-                "config_default" => "",
+                "config_value" => '""',
+                "config_default" => '""',
                 "config_descr" => "Oxidized API url",
                 "config_group" => "external",
                 "config_group_order" => "0",
@@ -482,8 +482,8 @@ class DefaultConfigSeeder extends Seeder
             ],
             [
                 "config_name" => "rrdtool",
-                "config_value" => "/usr/bin/rrdtool",
-                "config_default" => "/usr/bin/rrdtool",
+                "config_value" => '"/usr/bin/rrdtool"',
+                "config_default" => '"/usr/bin/rrdtool"',
                 "config_descr" => "Path to rrdtool",
                 "config_group" => "external",
                 "config_group_order" => "0",
@@ -506,8 +506,8 @@ class DefaultConfigSeeder extends Seeder
             ],
             [
                 "config_name" => "snmpgetnext",
-                "config_value" => "/usr/bin/snmpgetnext",
-                "config_default" => "snmpgetnext",
+                "config_value" => '"/usr/bin/snmpgetnext"',
+                "config_default" => '"snmpgetnext"',
                 "config_descr" => "Path to snmpgetnext",
                 "config_group" => "external",
                 "config_group_order" => "0",
@@ -650,8 +650,8 @@ class DefaultConfigSeeder extends Seeder
             ],
             [
                 "config_name" => "webui.graph_type",
-                "config_value" => "png",
-                "config_default" => "png",
+                "config_value" => '"png"',
+                "config_default" => '"png"',
                 "config_descr" => "Set the default graph type",
                 "config_group" => "webui",
                 "config_group_order" => "0",

includes/html/forms/config-item-update.inc.php

@@ -25,7 +25,7 @@ if (!is_numeric($_POST['config_id']) || empty($_POST['data'])) {
     exit;
 } else {
     $data   = mres($_POST['data']);
-    $update = dbUpdate(array('config_value' => "$data"), 'config', '`config_id` = ?', array($_POST['config_id']));
+    $update = dbUpdate(array('config_value' => json_encode($data, JSON_UNESCAPED_SLASHES)), 'config', '`config_id` = ?', array($_POST['config_id']));
     if (!empty($update) || $update == '0') {
         echo 'success';
         exit;

includes/html/forms/update-config-item.inc.php

@@ -83,8 +83,16 @@ if (!is_numeric($config_id)) {
     $message = 'Config item has been updated:';
     $status  = 'ok';
 } else {
-    $state  = mres($_POST['config_value']);
-    $update = dbUpdate(array('config_value' => $state), 'config', '`config_id`=?', array($config_id));
+    $state = $_POST['config_value'];
+    if (filter_var($value, FILTER_VALIDATE_INT)) {
+        $state = (int)$value;
+    } elseif (filter_var($value, FILTER_VALIDATE_FLOAT)) {
+        $state = (float)$value;
+    } elseif (filter_var($value, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE) !== null) {
+        $state = filter_var($value, FILTER_VALIDATE_BOOLEAN);
+    }
+
+    $update = dbUpdate(['config_value' => json_encode($state, JSON_UNESCAPED_SLASHES)], 'config', '`config_id`=?', array($config_id));
     if (!empty($update) || $update == '0') {
         $message = 'Alert rule has been updated.';
         $status  = 'ok';

includes/html/functions.inc.php

@@ -917,52 +917,15 @@ function clean_bootgrid($string)
 
 function get_config_by_group($group)
 {
-    $items = array();
-    foreach (dbFetchRows("SELECT * FROM `config` WHERE `config_group` = ?", array($group)) as $config_item) {
-        $val = $config_item['config_value'];
-        if (filter_var($val, FILTER_VALIDATE_INT)) {
-            $val = (int)$val;
-        } elseif (filter_var($val, FILTER_VALIDATE_FLOAT)) {
-            $val = (float)$val;
-        } elseif (filter_var($val, FILTER_VALIDATE_BOOLEAN)) {
-            $val = (boolean)$val;
+    return \App\Models\Config::query()->where('config_group', $group)->get()->map(function ($config_item) {
+        if ($config_item['config_value'] === true) {
+            $config_item['config_checked']  = 'checked';
         }
 
-        if ($val === true) {
-            $config_item += array('config_checked' => 'checked');
-        }
-
-        $items[$config_item['config_name']] = $config_item;
-    }
-
-    return $items;
+        return $config_item;
+    })->keyBy('config_name')->toArray();
 }//end get_config_by_group()
 
-
-function get_config_like_name($name)
-{
-    $items = array();
-    foreach (dbFetchRows("SELECT * FROM `config` WHERE `config_name` LIKE ?", array("%$name%")) as $config_item) {
-        $items[$config_item['config_id']] = $config_item;
-    }
-
-    return $items;
-}//end get_config_like_name()
-
-
-function get_config_by_name($name)
-{
-    $config_item = dbFetchRow('SELECT * FROM `config` WHERE `config_name` = ?', array($name));
-    return $config_item;
-}//end get_config_by_name()
-
-
-function set_config_name($name, $config_value)
-{
-    return dbUpdate(array('config_value' => $config_value), 'config', '`config_name`=?', array($name));
-}//end set_config_name()
-
-
 function get_url()
 {
     // http://stackoverflow.com/questions/2820723/how-to-get-base-url-with-php

tests/ConfigTest.php

@@ -126,7 +126,7 @@ class ConfigTest extends LaravelTestCase
 
         $key = 'testing.persist';
 
-        $query = Eloquent::DB()->table('config')->where('config_name', $key);
+        $query = \App\Models\Config::query()->where('config_name', $key);
 
         $query->delete();
         $this->assertFalse($query->exists(), "$key should not be set, clean database");