Re: [PATCH 18/21] initscripts fkt: readhash should only parse lines with a =

[Michael Tremer <michael.tremer@ipfire.org>]

[-- Attachment #1: Type: text/plain, Size: 3144 bytes --]

Hello,

> On 2 Jun 2024, at 19:09, Jonatan Schlag <jonatan.schlag(a)ipfire.org> wrote:
> 
> Hi,
> 
> 
> Am Freitag, dem 31.05.2024 um 10:56 +0100 schrieb Michael Tremer:
>> 
>> 
>>> On 20 May 2024, at 10:06, Jonatan Schlag
>>> <jonatan.schlag(a)ipfire.org> wrote:
>>> 
>>> A line without a = is clearly invalid.
>>> 
>>> Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
>>> ---
>>> src/initscripts/system/functions                            | 6
>>> ++++++
>>> tests/src/initscripts/system/functions/data/2               | 1 +
>>> tests/src/initscripts/system/functions/data/2_output_stderr | 1 +
>>> 3 files changed, 8 insertions(+)
>>> 
>>> diff --git a/src/initscripts/system/functions
>>> b/src/initscripts/system/functions
>>> index bbcfab95d..38a1316a8 100644
>>> --- a/src/initscripts/system/functions
>>> +++ b/src/initscripts/system/functions
>>> @@ -911,6 +911,12 @@ readhash() {
>>> continue
>>> fi
>>> 
>>> + # Skip lines without a =
>>> + if ! [[ ${line} =~ [^=]*=[^=]*$ ]]; then
>>> + echo "Invalid line '${line}'" >&2
>>> + continue
>>> + fi
>>> +
>>> local key="${line%=*}"
>>> local val="${line#*=}"
>> 
>> You can simply check if key and value have content. That should be a
>> lot easier than whatever this is.
> 
> Hi,
> 
> Unfortunately not. A line like
> 
> Line_without_a_equal_sign_is_also_invalid

Okay, let’s say this is indeed an empty line (and not just a key without a value).

> result in an entry like this:
> 
> Key = Line_without_a_equal_sign_is_also_invalid
> Value = Line_without_a_equal_sign_is_also_invalid

Yes, you can’t use the parameter expansions then and need to check first if you have an = character at all (you can simply do that with a case statement instead of a complicated regular expression.

> We simply do not split without an equal sign, so key and value are
> equal. And maybe are also valid keys and values.

That isn’t a very good check. It could theoretically happen.

-Michael

> 
> Jonatan
>> 
>>> 
>>> diff --git a/tests/src/initscripts/system/functions/data/2
>>> b/tests/src/initscripts/system/functions/data/2
>>> index 3e1a7028b..d1c4d327d 100644
>>> --- a/tests/src/initscripts/system/functions/data/2
>>> +++ b/tests/src/initscripts/system/functions/data/2
>>> @@ -15,6 +15,7 @@ RED_ADDRESS=0.0.0.0
>>> RED_NETMASK=0.0.0.0
>>> RED_TYPE=PPPOE
>>> RED_NETADDRESS=0.0.0.0
>>> +Line_without_a_equal_sign_is_also_invalid
>>> 
>>> # Comment for testing  
>>>     # Comment for testing Comments with spaces before
>>> diff --git
>>> a/tests/src/initscripts/system/functions/data/2_output_stderr
>>> b/tests/src/initscripts/system/functions/data/2_output_stderr
>>> index dfcf2154b..cccd19af6 100644
>>> --- a/tests/src/initscripts/system/functions/data/2_output_stderr
>>> +++ b/tests/src/initscripts/system/functions/data/2_output_stderr
>>> @@ -2,3 +2,4 @@ Invalid key '-RED_DEV'
>>> Invalid key 'RE??D_MACADDR'
>>> Invalid key 'RED&&_DRIVER'
>>> Invalid key '0BLUE_DEV'
>>> +Invalid line 'Line_without_a_equal_sign_is_also_invalid'
>>> -- 
>>> 2.39.2
>>> 
>> 
>