From cbc346b279b5c08b281c9901b994a58eb037a60d Mon Sep 17 00:00:00 2001 From: Edd Salkield Date: Sat, 18 Jan 2020 18:40:45 +0000 Subject: Fix shell globbing bugs There are several problems leading to the unintentional globbing issue: Firstly, within `rcup` and `rcdn`, when constructing arguments to pass to `lsrc`, the _for_ loops over the arguments do not have quoted variables, leading to globbing. I have quoted these accordingly. Secondly, `lsrc` is invoked as follows: ```sh dests_and_srcs="$(lsrc $LS_ARGS)" ``` When shells use command substitution like this, they go through two stages: - Word expansion. This is useful because it splits `LS_ARGS` back up into its constituent strings. - File name expansion. The side effect of this is to introduce globbing. You can read more about how this works [here](https://www.tldp.org/LDP/Bash-Beginners-Guide/html/sect_03_04.html#sect_03_04_07). To fix this, I have passed `lsrc` and its arguments to `eval`. This involves quoting the relevant arguments, so: ```sh for dotfiles_dir in "$DOTFILES_DIRS"; do LS_ARGS="$LS_ARGS -d $dotfiles_dir" done ``` becomes ```sh for dotfiles_dir in "$DOTFILES_DIRS"; do LS_ARGS="$LS_ARGS -d \"$dotfiles_dir\"" done ``` Then `lsrc` is invoked as follows: ```sh dests_and_srcs="$(eval "lsrc $LS_ARGS")" ``` There is one final non-globbing issue: the parsing of arguments can introduce extra spaces in the variables, which then trip up the `dotfiles_dir_excludes` function. For example: ```sh I) includes="$includes $OPTARG";; ``` introduces a space if `includes` is empty or null. I have introduced the function `append_variable`, which allows two variables to be appended without introducing unnecessary whitespace. Then the additional whitespace is never added in the first place. Fixes #256. --- bin/lsrc.in | 18 +++++++++--------- bin/mkrc.in | 2 +- bin/rcdn.in | 54 +++++++++++++++++++++++++++--------------------------- bin/rcup.in | 50 +++++++++++++++++++++++++------------------------- 4 files changed, 62 insertions(+), 62 deletions(-) (limited to 'bin') diff --git a/bin/lsrc.in b/bin/lsrc.in index 6fb6162..eed4c0a 100755 --- a/bin/lsrc.in +++ b/bin/lsrc.in @@ -166,7 +166,7 @@ dotfiles_dir_excludes() { $DEBUG "dotfiles_dir_excludes $dotfiles_dir" $DEBUG " with excludes: $excludes" - for exclude in $excludes; do + for exclude in "$excludes"; do if echo "$exclude" | grep ':' >/dev/null; then dotfiles_dir_pat="$(echo "$exclude" | sed 's/:.*//')" file_glob="$(echo "$exclude" | sed 's/.*://')" @@ -259,17 +259,17 @@ handle_command_line() { case "$opt" in F) show_sigils=1;; h) show_help ;; - I) includes="$includes $OPTARG";; - t) arg_tags="$arg_tags $OPTARG";; + I) includes="$(append_variable "$includes" "$OPTARG")" ;; + t) arg_tags="$(append_variable "$arg_tags" "$OPTARG")" ;; v) verbosity=$(($verbosity + 1));; q) verbosity=$(($verbosity - 1));; - d) dotfiles_dirs="$dotfiles_dirs $OPTARG";; + d) dotfiles_dirs="$(append_variable "$dotfiles_dirs" "$OPTARG")" ;; V) version=1;; - x) excludes="$excludes $OPTARG";; - S) symlink_dirs="$symlink_dirs $OPTARG";; - s) never_symlink_dirs="$never_symlink_dirs $OPTARG";; - U) undotted="$undotted $OPTARG";; - u) never_undotted="$never_undotted $OPTARG";; + x) excludes="$(append_variable "$excludes" "$OPTARG")" ;; + S) symlink_dirs="$(append_variable "$symlink_dirs" "$OPTARG")" ;; + s) never_symlink_dirs="$(append_variable "$never_symlink_dirs" "$OPTARG")" ;; + U) undotted="$(append_variable "$undotted" "$OPTARG")" ;; + u) never_undotted="$(append_variable "$never_undotted" "$OPTARG")" ;; B) hostname="$OPTARG";; ?) show_help 64 ;; esac diff --git a/bin/mkrc.in b/bin/mkrc.in index d246c33..5628f14 100755 --- a/bin/mkrc.in +++ b/bin/mkrc.in @@ -86,7 +86,7 @@ while getopts :ChSsUuVvqot:d:B: opt; do B) in_host=1 hostname="$OPTARG" - install_args="-B $hostname" + install_args=$(append_variable "$install_args" "-B $hostname") ;; ?) show_help 64 ;; esac diff --git a/bin/rcdn.in b/bin/rcdn.in index a2a8472..3ebbd27 100755 --- a/bin/rcdn.in +++ b/bin/rcdn.in @@ -48,19 +48,19 @@ handle_command_line() { case "$opt" in h) show_help ;; B) hostname="$OPTARG" ;; - I) includes="$includes $OPTARG";; + I) includes="$(append_variable "$includes" "$OPTARG")" ;; k) run_hooks=1 ;; K) run_hooks=0 ;; - t) arg_tags="$arg_tags $OPTARG" ;; - S) symlink_dirs="$symlink_dirs $OPTARG";; - s) never_symlink_dirs="$never_symlink_dirs $OPTARG";; - U) undotted="$undotted $OPTARG";; - u) never_undotted="$never_undotted $OPTARG";; + t) arg_tags="$(append_variable "$arg_tags" "$OPTARG")" ;; + S) symlink_dirs="$(append_variable "$symlink_dirs" "$OPTARG")" ;; + s) never_symlink_dirs="$(append_variable "$never_symlink_dirs" "$OPTARG")" ;; + U) undotted="$(append_variable "$undotted" "$OPTARG")" ;; + u) never_undotted="$(append_variable "$never_undotted" "$OPTARG")";; v) verbosity=$(($verbosity + 1));; q) verbosity=$(($verbosity - 1));; - d) dotfiles_dirs="$dotfiles_dirs $OPTARG" ;; + d) dotfiles_dirs="$(append_variable "$dotfiles_dirs" "$OPTARG")" ;; V) version=1 ;; - x) excludes="$excludes $OPTARG" ;; + x) excludes="$(append_variable "$excludes" "$OPTARG")" ;; ?) show_help 64 ;; esac done @@ -72,34 +72,34 @@ handle_command_line() { tags="${arg_tags:-$TAGS}" dotfiles_dirs="${dotfiles_dirs:-$DOTFILES_DIRS}" files="$@" - RUN_HOOKS=$run_hooks + RUN_HOOKS="$run_hooks" - for tag in $tags; do - LS_ARGS="$LS_ARGS -t $tag" + for tag in "$tags"; do + LS_ARGS="$LS_ARGS -t \"$tag\"" done - for dotfiles_dir in $dotfiles_dirs; do - LS_ARGS="$LS_ARGS -d $dotfiles_dir" + for dotfiles_dir in "$dotfiles_dirs"; do + LS_ARGS="$LS_ARGS -d \"$dotfiles_dir\"" done - for exclude in $excludes; do - LS_ARGS="$LS_ARGS -x $exclude" + for exclude in "$excludes"; do + LS_ARGS="$LS_ARGS -x \"$exclude\"" done - for include in $includes; do - LS_ARGS="$LS_ARGS -I $include" + for include in "$includes"; do + LS_ARGS="$LS_ARGS -I \"$include\"" done - for symlink_dir in $symlink_dirs; do - LS_ARGS="$LS_ARGS -S $symlink_dir" + for symlink_dir in "$symlink_dirs"; do + LS_ARGS="$LS_ARGS -S \"$symlink_dir\"" done - for never_symlink_dir in $symlink_dirs; do - LS_ARGS="$LS_ARGS -s $never_symlink_dir" + for never_symlink_dir in "$symlink_dirs"; do + LS_ARGS="$LS_ARGS -s \"$never_symlink_dir\"" done - for undot in $undotted; do - LS_ARGS="$LS_ARGS -U $undot" + for undot in "$undotted"; do + LS_ARGS="$LS_ARGS -U \"$undot\"" done - for never_undot in $never_undotted; do - LS_ARGS="$LS_ARGS -u $never_undot" + for never_undot in "$never_undotted"; do + LS_ARGS="$LS_ARGS -u \"$never_undot\"" done - LS_ARGS="$LS_ARGS -B $hostname $files" + LS_ARGS="$LS_ARGS -B \"$hostname\" $files" $DEBUG "LS_ARGS: $LS_ARGS" } @@ -111,7 +111,7 @@ handle_command_line "$@" run_hooks pre down -dests_and_srcs="$(lsrc $LS_ARGS)" +dests_and_srcs="$(eval "lsrc $LS_ARGS")" saved_ifs="$IFS" IFS=' diff --git a/bin/rcup.in b/bin/rcup.in index 78f5faa..65f0ab8 100755 --- a/bin/rcup.in +++ b/bin/rcup.in @@ -213,23 +213,23 @@ handle_command_line() { case "$opt" in B) hostname="$OPTARG" ;; C) always_copy=1 ;; - d) dotfiles_dirs="$dotfiles_dirs $OPTARG" ;; + d) dotfiles_dirs="$(append_variable "$dotfiles_dirs" "$OPTARG")" ;; f) REPLACE_ALL=1 ;; g) generate=1 ;; h) show_help ;; i) REPLACE_ALL=0 ;; - I) includes="$includes $OPTARG" ;; + I) includes="$(append_variable "$includes" "$OPTARG")" ;; k) run_hooks=1 ;; K) run_hooks=0 ;; q) verbosity=$(($verbosity - 1)) ;; - t) arg_tags="$arg_tags $OPTARG" ;; - S) symlink_dirs="$symlink_dirs $OPTARG";; - s) never_symlink_dirs="$never_symlink_dirs $OPTARG";; - U) undotted="$undotted $OPTARG";; - u) never_undotted="$never_undotted $OPTARG";; + t) arg_tags="$(append_variable "$arg_tags" "$OPTARG")" ;; + S) symlink_dirs="$(append_variable "$symlink_dirs" "$OPTARG")" ;; + s) never_symlink_dirs="$(append_variable "$never_symlink_dirs" "$OPTARG")";; + U) undotted="$(append_variable "$undotted" "$OPTARG")" ;; + u) never_undotted="$(append_variable "$never_undotted" "$OPTARG")" ;; v) verbosity=$(($verbosity + 1)) ;; V) version=1 ;; - x) excludes="$excludes $OPTARG" ;; + x) excludes="$(append_variable "$excludes" "$OPTARG")" ;; ?) show_help 64 ;; esac done @@ -261,31 +261,31 @@ handle_command_line() { done for tag in $tags; do - LS_ARGS="$LS_ARGS -t $tag" + LS_ARGS="$LS_ARGS -t \"$tag\"" done - for dotfiles_dir in $DOTFILES_DIRS; do - LS_ARGS="$LS_ARGS -d $dotfiles_dir" + for dotfiles_dir in "$DOTFILES_DIRS"; do + LS_ARGS="$LS_ARGS -d \"$dotfiles_dir\"" done - for exclude in $excludes; do - LS_ARGS="$LS_ARGS -x $exclude" + for exclude in "$excludes"; do + LS_ARGS="$LS_ARGS -x \"$exclude\"" done - for include in $includes; do - LS_ARGS="$LS_ARGS -I $include" + for include in "$includes"; do + LS_ARGS="$LS_ARGS -I \"$include\"" done - for symlink_dir in $symlink_dirs; do - LS_ARGS="$LS_ARGS -S $symlink_dir" + for symlink_dir in "$symlink_dirs"; do + LS_ARGS="$LS_ARGS -S \"$symlink_dir\"" done - for never_symlink_dir in $never_symlink_dirs; do - LS_ARGS="$LS_ARGS -s $never_symlink_dir" + for never_symlink_dir in "$never_symlink_dirs"; do + LS_ARGS="$LS_ARGS -s \"$never_symlink_dir\"" done - for undot in $undotted; do - LS_ARGS="$LS_ARGS -U $undot" + for undot in "$undotted"; do + LS_ARGS="$LS_ARGS -U \"$undot\"" done - for never_undot in $never_undotted; do - LS_ARGS="$LS_ARGS -u $never_undot" + for never_undot in "$never_undotted"; do + LS_ARGS="$LS_ARGS -u \"$never_undot\"" done - LS_ARGS="$LS_ARGS -B $hostname $files" + LS_ARGS="$LS_ARGS -B \"$hostname\" $files" $DEBUG "LS_ARGS: $LS_ARGS" } @@ -297,7 +297,7 @@ handle_command_line "$@" run_hooks pre up -dests_and_srcs="$(lsrc $LS_ARGS)" +dests_and_srcs="$(eval "lsrc $LS_ARGS")" saved_ifs="$IFS" IFS=' -- cgit v1.2.3