#2932 PR `merged`: check for wrong array assignment in user configs¶

Labels: fixed / solved / done

schlomo opened issue at 2023-02-15 00:03:¶

fixes #2930

I wrote something that has this effect:

rear-sle15sp4:/src/rear # cat /etc/rear/local.conf 
BACKUP_PROG_INCLUDE=( $(findmnt -n -r -o TARGET -t btrfs | grep -v '^/$' | egrep -v 'snapshots|crash') )
COPY_AS_IS=foobar
rear-sle15sp4:/src/rear # rear help
ERROR: Missing array assignment like +=(...) for COPY_AS_IS in /etc/rear/local.conf:
COPY_AS_IS=foobar

Use debug mode '-d' for some debug messages or debugscript mode '-D' for full debug messages with 'set -x' output
Aborting due to an error, check /var/log/rear/rear-rear-sle15sp4.log.lockless for details
Terminated

What do you think?

schlomo commented at 2023-02-15 09:15:¶

@rear/contributors what do you think? As usual, no comment means I will merge tomorrow.

schlomo commented at 2023-02-16 18:40:¶

Now it looks like this:

# usr/sbin/rear -C /tmp/bad help
ERROR: Syntax error: Variable BACKUP_PROG_INCLUDE not assigned as Bash array in /tmp/bad.conf:
BACKUP_PROG_INCLUDE="( $(findmnt -n -r -o TARGET -t btrfs | grep -v '^/$' | egrep -v 'snapshots|crash') )"

Use debug mode '-d' for some debug messages or debugscript mode '-D' for full debug messages with 'set -x' output
Aborting due to an error, check /src/rear/var/log/rear/rear-rear-sle15sp4.log.lockless for details
Terminated

Thanks for the feedback!

schlomo commented at 2023-02-16 18:56:¶

And now we catch even these cases:

variables that contain other variable names
multiple assignment on the same line

# cat /tmp/bad.conf 
F_BACKUP_PROG_INCLUDE="( $(findmnt -n -r -o TARGET -t btrfs | grep -v '^/$' | egrep -v 'snapshots|crash') )"
COPY_AS_IS=() BACKUP_PROG_INCLUDE=foobar

# usr/sbin/rear -D -C /tmp/bad help
Sourcing additional configuration file '/tmp/bad.conf'
Running 'init' stage ======================
ERROR: Syntax error: Variable BACKUP_PROG_INCLUDE not assigned as Bash array in /tmp/bad.conf:
COPY_AS_IS=() BACKUP_PROG_INCLUDE=foobar

jsmeix commented at 2023-02-17 08:29:¶

It was merged too fast
because mapfile is not availabe in older bash,
at least not in GNU bash version 3.1.17 on SLES10
(SLE10 is the oldest one I have and I use that
to check what there is on rather old systems):

# type -a mapfile
-bash: type: mapfile: not found

# bash --version
GNU bash, version 3.1.17 ...

mapfile is a bash 4.x thing according to
https://tldp.org/LDP/abs/html/bashver4.html

The new mapfile builtin ...

I suggest to add at the beginning of
init/default/001_verify_config_arrays.sh
a simple fail safe check like

# Skip this test when 'mapfile' (a bash 4.x builtin) is not available:
type -a mapfile 1>/dev/null || return 0

because that script is optional.

That '1>/dev/null' is only there to suppress needless

mapfile is a shell builtin

output which would appear in the log file in debug modes
but on the other hand it does not hurt in the log file in debug modes
so perhaps the simpler the better without needless '1>/dev/null'.

schlomo commented at 2023-02-17 08:46:¶

Ah, good catch. I was wondering about that and will add the check.

BTW, sed -E also doesn't work on ancient systems, I checked this on CentOS 5 which has Bash 3.x (couldn't find a SLES10/SUSE10 Docker image quickly)

[Export of Github issue for rear/rear.]

#2932 PR merged: check for wrong array assignment in user configs¶