diff mbox series

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

Message ID 20240520090611.10406-19-jonatan.schlag@ipfire.org
State New
Headers
Series [01/21] test: Add bash lib for colors |

Commit Message

Jonatan Schlag May 20, 2024, 9:06 a.m. UTC 
  A line without a = is clearly invalid.

Signed-off-by: Jonatan Schlag <jonatan.schlag@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(+)

Comments

Michael Tremer May 31, 2024, 9:56 a.m. UTC | #1 
> On 20 May 2024, at 10:06, Jonatan Schlag <jonatan.schlag@ipfire.org> wrote:
> 
> A line without a = is clearly invalid.
> 
> Signed-off-by: Jonatan Schlag <jonatan.schlag@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.

> 
> 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
>
Jonatan Schlag June 2, 2024, 6:09 p.m. UTC | #2 
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@ipfire.org> wrote:
> > 
> > A line without a = is clearly invalid.
> > 
> > Signed-off-by: Jonatan Schlag <jonatan.schlag@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


result in an entry like this:

Key = Line_without_a_equal_sign_is_also_invalid
Value = Line_without_a_equal_sign_is_also_invalid

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

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
> > 
>
Michael Tremer June 3, 2024, 9:25 a.m. UTC | #3 
Hello,

> On 2 Jun 2024, at 19:09, Jonatan Schlag <jonatan.schlag@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@ipfire.org> wrote:
>>> 
>>> A line without a = is clearly invalid.
>>> 
>>> Signed-off-by: Jonatan Schlag <jonatan.schlag@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
>>> 
>> 
>
diff mbox series

Patch

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#*=}"
 
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'