#1969 PR merged: check for carriage return in {local|site|rescue}.conf files

Labels: enhancement, fixed / solved / done

gdha opened issue at 2018-11-19 11:45:

  • Type: Enhancement

  • Impact: Low

  • Reference to related issue (URL): #1965

  • How was this pull request tested? locally

$ sudo usr/sbin/rear -v mkrescue
Relax-and-Recover 2.4 / Git
Running rear mkrescue (PID 15205)
Using log file: /projects/rear/myforks/rear/var/log/rear/rear-okido.log
ERROR: Carriage return character in /projects/rear/myforks/rear/etc/rear/local.conf (perhaps DOS or Mac format)
  • Brief description of the changes in this pull request: If configuration files contain hidden carriage returns this often runs into an error (which is sometimes not obvious to understand where the issue came from). With this patch we will bail out and tell the user to fix his config file

jsmeix commented at 2018-11-19 12:37:

Currently it tests only site.conf local.conf and rescue.conf
but sbin/rear sources several other config files too (excerpts):

source $SHARE_DIR/conf/default.conf

perhaps we can assume default.conf is never modified by the user
but I think there have been already issues where users had
messed around with default.conf ?

test -r "$CONFIG_DIR/os.conf" && Source "$CONFIG_DIR/os.conf" || true
test -r "$CONFIG_DIR/$WORKFLOW.conf" && Source "$CONFIG_DIR/$WORKFLOW.conf" || true

I think all *.conf files in $CONFIG_DIR could have been broken by the user
so that all *.conf files in $CONFIG_DIR should be tested.

jsmeix commented at 2018-11-19 12:42:

@schlomo
I think your Nice trick with 'tr' comment means
that you don't know a plain bash-only method and
https://github.com/rear/rear/issues/1965#issuecomment-439432400
is answered.

jsmeix commented at 2018-11-19 14:17:

FYI:
With https://github.com/rear/rear/pull/1970
it would be possible to use the Source function return code
to actually check in commands like Source "$CONFIG_DIR/os.conf"
whether or not that was actually successful - but still note
https://github.com/rear/rear/issues/1965#issuecomment-439330017
i.e. as long as it is allowed that the last command in config files could fail
we cannot use the Source function return code to check if config files
are valid as I did in
https://github.com/rear/rear/issues/1965#issuecomment-439325290

gdha commented at 2018-11-20 07:17:

@jsmeix Glad to use your code and as the issue was already closed I decided to add it to rear before we all forgot about it. Anyhow, a big thank you.
IMHO I think these 3 scripts are in 99% of the cases where end-users play with - if they touches other scripts or config files then we may consider them as experts, no?

jsmeix commented at 2018-11-20 10:06:

@gdha
yes, as it is now it should detect 99.9% of such cases.

[Export of Github issue for rear/rear.]