From 883ef5b965bb7b02a701037748436f8daa9d0643 Mon Sep 17 00:00:00 2001

From: Rob Crittenden <rcritten@redhat.com>

Date: Fri Apr 29 11:42:53 2022 -0400

Subject: Convert configuration option strings into native data

 types

When loading options from the configuration file they will all

be strings. This breaks existing boolean checks (if <something>)

and some assumptions about integer types (e.g. timeout, indent).

So try to detect the data type, defaulting to remain as a string.

Also hardcode some type validation for known keys to prevent

things like debug=foo (which would evaluate to True).

https://bugzilla.redhat.com/show_bug.cgi?id=2079861

Signed-off-by: Rob Crittenden <rcritten@redhat.com>

---

 src/ipahealthcheck/core/config.py | 47 ++++++++++++++++++++++++++++++-

 src/ipahealthcheck/core/output.py |  2 +-

 tests/test_config.py              | 16 ++++++++++-

 tests/test_options.py             |  2 +-

 4 files changed, 63 insertions(+), 4 deletions(-)

diff --git a/src/ipahealthcheck/core/config.py b/src/ipahealthcheck/core/config.py

index 14ae27f..3cd4e8e 100644

--- a/src/ipahealthcheck/core/config.py

+++ b/src/ipahealthcheck/core/config.py

@@ -71,6 +71,27 @@ class Config:

             self.__d[key] = d[key]

+def convert_string(value):

+    """

+    Reading options from the configuration file will leave them as

+    strings. This breaks boolean values so attempt to convert them.

+    """

+    if not isinstance(value, str):

+        return value

+

+    if value.lower() in (

+        "true",

+        "false",

+    ):

+        return value.lower() == 'true'

+    else:

+        try:

+            value = int(value)

+        except ValueError:

+            pass

+    return value

+

+

 def read_config(config_file):

     """

     Simple configuration file reader

@@ -100,6 +121,30 @@ def read_config(config_file):

     items = parser.items(CONFIG_SECTION)

     for (key, value) in items:

-        config[key] = value

+        if len(value) == 0 or value is None:

+            logging.error(

+                "Empty value for %s in %s [%s]",

+                key, config_file, CONFIG_SECTION

+            )

+            return None

+        else:

+            # Try to do some basic validation. This is unfortunately

+            # hardcoded.

+            if key in ('all', 'debug', 'failures_only', 'verbose'):

+                if value.lower() not in ('true', 'false'):

+                    logging.error(

+                        "%s is not a valid boolean in %s [%s]",

+                        key, config_file, CONFIG_SECTION

+                    )

+                    return None

+            elif key in ('indent',):

+                if not isinstance(convert_string(value), int):

+                    logging.error(

+                        "%s is not a valid integer in %s [%s]",

+                        key, config_file, CONFIG_SECTION

+                    )

+                    return None

+            # Some rough type translation from strings

+            config[key] = convert_string(value)

     return config

diff --git a/src/ipahealthcheck/core/output.py b/src/ipahealthcheck/core/output.py

index 61dffbe..945969f 100644

--- a/src/ipahealthcheck/core/output.py

+++ b/src/ipahealthcheck/core/output.py

@@ -110,7 +110,7 @@ class JSON(Output):

     def __init__(self, options):

         super(JSON, self).__init__(options)

-        self.indent = int(options.indent)

+        self.indent = options.indent

     def generate(self, data):

         output = json.dumps(data, indent=self.indent)

diff --git a/tests/test_config.py b/tests/test_config.py

index 09c2188..655233c 100644

--- a/tests/test_config.py

+++ b/tests/test_config.py

@@ -2,7 +2,7 @@

 # Copyright (C) 2019 FreeIPA Contributors see COPYING for license

 #

-from ipahealthcheck.core.config import read_config

+from ipahealthcheck.core.config import read_config, convert_string

 import tempfile

@@ -60,3 +60,17 @@ def test_config_recursion():

         config._Config__d['_Config__d']

     except KeyError:

         pass

+

+

+def test_convert_string():

+    for value in ("s", "string", "BiggerString"):

+        assert convert_string(value) == value

+

+    for value in ("True", "true", True):

+        assert convert_string(value) is True

+

+    for value in ("False", "false", False):

+        assert convert_string(value) is False

+

+    for value in ("10", "99999", 807):

+        assert convert_string(value) == int(value)

diff --git a/tests/test_options.py b/tests/test_options.py

index da1866f..00cdb7c 100644

--- a/tests/test_options.py

+++ b/tests/test_options.py

@@ -40,6 +40,6 @@ def test_options_merge(mock_parse, mock_run, mock_service):

         # verify two valus that have defaults with our overriden values

         assert run.options.output_type == 'human'

-        assert run.options.indent == '5'

+        assert run.options.indent == 5

     finally:

         os.remove(config_path)

-- 

2.31.1