csaf-tools / CVRF-CSAF-Converter

A CVRF CSAF Converter, taking care about OASIS specification.
https://www.telekom.com/security
MIT License
10 stars 4 forks source link

Error: undefined name in handle_boolean_config_values (runtime error) #74

Closed sthagen closed 2 years ago

sthagen commented 2 years ago

Smelling copy-pasta-search-replace (CPSR) with a failed R step :wink:

def handle_boolean_config_values(key, val):
    try:
        if isinstance(val, bool):
            return val
        if val.strip().lower() in {'true', 'yes', '1', 'y'}:
            return True
        if val.strip().lower() in {'false', 'no', '0', 'n'}:
            return False

        critical_exit(f"Reading config.yaml failed. "
                      f"Invalid value for config key {key}: {val}. {e}.")

    except Exception as e:
        critical_exit(f"Reading config.yaml failed. "
                      f"Invalid value for config key {key}: {val} {e}.")

Problem:

>>> import cvrf2csaf.common.utils as bomb
>>> bomb.handle_boolean_config_values('whatever', 'bomb')
2022-03-12 02:14:48,332 - utils - CRITICAL - Reading config.yaml failed. Invalid value for config key whatever: bomb local variable 'e' referenced before assignment.

Suggest to handle the finding flake8 → cvrf2csaf/common/utils.py:29:69: F821 undefined name 'e'

Minimal fix:

f"Invalid value for config key {key}: {val}. {e}."

with:

f"Invalid value for config key {key}: {val}."

Real (assuming here) fix would be to not care if a type was wrong (where the .strip() would cause an exception) but simply raise a subsequently handled exception after the if-elif-elif dispatch for any "survivors" and reuse the logging call in the exception handler.

Example implementation: Narrowing the Exception down to an AttributeError and injecting a ValueErrorif we fall off the normal handling in a soft way should just work fine. So what about:

def handle_boolean_config_values(key, val):
    """Translate boolean like values to boolean or initiate exit."""
    try:
        if isinstance(val, bool):
            return val
        if val.strip().lower() in {'true', 'yes', '1', 'y'}:
            return True
        if val.strip().lower() in {'false', 'no', '0', 'n'}:
            return False

        raise ValueError("unexpected value")

    except (AttributeError, ValueError) as err:
        critical_exit(f"Reading config.yaml failed.  Invalid value for config key {key}: {val} {err}.")

Short test:

>>> import spike  # Sorry, the above function as is in spike.py plus critical_exit = print
>>> spike.handle_boolean_config_values(42, "12")
Reading config.yaml failed.  Invalid value for config key 42: 12 unexpected value.