Broken regex for default value

[heiner0815]

Hi,
the regex pattern is broken. One needs to exclude the default_sep character from first matching group, otherwise the environment variable is not set. It should read:

pattern = re.compile(r'.*?\$\{([^}{' + default_sep + r']+)' + default_sep_pattern + r'\}.*?')

If one changes the default value of the separator from None to "", one could do:

    default_sep_pattern = r'(' + default_sep + '[^}^{]+)?'\
        if default_sep != "" else ''
    pattern = re.compile(r'.*?\$\{([^}{' + default_sep + r']+)' + default_sep_pattern + r'\}.*?')

A test like this will fail with current version:

    def test_parse_config_default_separator_strong_password_overwritten_by_env_var(self):
        os.environ["ENV_TAG1"] = "myWeakPassword"

        test_data = '''
        test1:
            data0: !TEST ${ENV_TAG1:NoHtnnmEuluGp2boPvGQkGrXqTAtBvIVz9VRmV65}/somethingelse/${ENV_TAG2}
            data1:  !TEST ${ENV_TAG2:0.0.0.0}
        '''
        config = parse_config(data=test_data, tag='!TEST', default_sep=':')

        expected_config = {
            'test1': {
                'data0': 'myWeakPassword/somethingelse/N/A',
                'data1': '0.0.0.0'
            }
        }

        self.assertDictEqual(
            config,
            expected_config
        )

Thanks a lot

[mkaranasou]

Hi, thanks for the detailed issue, unfortunately the pattern got a bit changed in the previous release and I didn't catch this. v1.0.3 should cover your example. I included your test also, along with other two variations.
Let me know if this is ok for you :)

[heiner0815]

Hi,
thanks for the quick reply. Reverting back to \w in the first matching group limits the allowed characters of the environment variables. For me this is also fine. The previous regex, with the suggested fix would have also allowed special characters as, to my understanding, this was the intention by the change before.

[mkaranasou]

Hmm, I see, I will revisit this soon with more tests. Thanks!

[mkaranasou]

@heiner0815 the current pattern allows for alphanumeric, so a-z, A-Z, _ and 0-9 ([a-zA-Z_]{1,}[a-zA-Z0-9_]{0,}). I think other characters are usually not allowed, zsh for example will give an unknown command or a not valid in this context error. Could you please provide an example of what the current pattern might be missing?

[heiner0815]

Hi,
from shell point of view this is certainly true. However, for example from within python you could use also different characters, e.g.:
os.environ["env%2"]="123" works. Also a name starting with a digit works.
From this thread basically everything except "=" is allowed as characters https://stackoverflow.com/questions/2821043/allowed-characters-in-linux-environment-variable-names#:~:text=Environment%20variable%20names%20used%20by,not%20begin%20with%20a%20digit.

In the end it's a question what you want to support. I would prefer the least strict :-)