Skip to content

Commit a7aabbc

Browse files
Seamus Connorakinomyoga
Seamus Connor
authored andcommitted
fix: Properly handle environments with unset IFS
Prior to this commit, the current state of `IFS` was stored into a local variable `ifs`. Later, IFS is restored by setting `IFS=$ifs`. If the user starts with either a set `IFS` or an empty `IFS`, then this behavior is correct. If the user starts with an unset `IFS`, then `IFS=$ifs` results in an empty `IFS`, which is not the original state. Most of the time an empty `IFS` is not desired, and indeed it breaks some things inside bash-completions. eg ``` unset IFS local ifs=$IFS IFS=$' \t\n' restore=$(shopt -po noglob) IFS=$ifs $restore ... -bash: set -o noglob: command not found ``` With this commit, all instances of `local ifs=$IFS` have been removed. Instead, the `local IFS` is dynamically unset.
1 parent a66ae99 commit a7aabbc

File tree

5 files changed

+55
-24
lines changed

5 files changed

+55
-24
lines changed

bash_completion

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,23 @@ dequote()
149149
eval printf %s "$1" 2>/dev/null
150150
}
151151

152+
# Unset the given variables across a scope boundary. Useful for unshadowing
153+
# global scoped variables. Note that simply calling unset on a local variable
154+
# will not unshadow the global variable. Rather, the result will be a local
155+
# variable in an unset state.
156+
# Usage: local IFS='|'; _comp_unlocal IFS
157+
# Param: $* Variable names to be unset
158+
_comp_unlocal()
159+
{
160+
if ((BASH_VERSINFO[0] >= 5)) && shopt -q localvar_unset; then
161+
shopt -u localvar_unset
162+
unset -v "$@"
163+
shopt -s localvar_unset
164+
else
165+
unset -v "$@"
166+
fi
167+
}
168+
152169
# Assign variable one scope above the caller
153170
# Usage: local "$1" && _upvar $1 "value(s)"
154171
# Param: $1 Variable name to assign value to
@@ -721,12 +738,12 @@ _comp_delimited()
721738
# glob char escaping issues to deal with. Do removals by hand instead.
722739
COMPREPLY=($(compgen "$@"))
723740
local -a existing
724-
local x i ifs=$IFS IFS=$delimiter
741+
local x i IFS=$delimiter
725742
existing=($cur)
726743
# Do not remove the last from existing if it's not followed by the
727744
# delimiter so we get space appended.
728745
[[ ! $cur || $cur == *"$delimiter" ]] || unset -v "existing[${#existing[@]}-1]"
729-
IFS=$ifs
746+
_comp_unlocal IFS
730747
if ((${#COMPREPLY[@]})); then
731748
for x in ${existing+"${existing[@]}"}; do
732749
for i in "${!COMPREPLY[@]}"; do
@@ -1253,14 +1270,13 @@ else
12531270
if [[ ${1-} == -s ]]; then
12541271
procs=($(command ps ax -o comm | command sed -e 1d))
12551272
else
1256-
local line i=-1 ifs=$IFS
1257-
IFS=$'\n'
1273+
local line i=-1 IFS=$'\n'
12581274
# Some versions of ps don't support "command", but do "comm", e.g.
12591275
# some busybox ones. Fall back
12601276
local -a psout=($({
12611277
command ps ax -o command= || command ps ax -o comm=
12621278
} 2>/dev/null))
1263-
IFS=$ifs
1279+
_comp_unlocal IFS
12641280
for line in "${psout[@]}"; do
12651281
if ((i == -1)); then
12661282
# First line, see if it has COMMAND column header. For
@@ -1751,7 +1767,7 @@ _included_ssh_config_files()
17511767
# Return: Completions, starting with CWORD, are added to COMPREPLY[]
17521768
_known_hosts_real()
17531769
{
1754-
local configfile flag prefix="" ifs=$IFS
1770+
local configfile flag prefix=""
17551771
local cur suffix="" aliases i host ipv4 ipv6
17561772
local -a kh tmpkh=() khd=() config=()
17571773

@@ -1817,7 +1833,7 @@ _known_hosts_real()
18171833
# spaces in their name work (watch out for ~ expansion
18181834
# breakage! Alioth#311595)
18191835
tmpkh=($(awk 'sub("^[ \t]*([Gg][Ll][Oo][Bb][Aa][Ll]|[Uu][Ss][Ee][Rr])[Kk][Nn][Oo][Ww][Nn][Hh][Oo][Ss][Tt][Ss][Ff][Ii][Ll][Ee][ \t=]+", "") { print $0 }' "${config[@]}" | sort -u))
1820-
IFS=$ifs
1836+
_comp_unlocal IFS
18211837
fi
18221838
if ((${#tmpkh[@]} != 0)); then
18231839
local j
@@ -1873,7 +1889,7 @@ _known_hosts_real()
18731889
# Add host to candidates
18741890
COMPREPLY+=($host)
18751891
done
1876-
IFS=$ifs
1892+
_comp_unlocal IFS
18771893
done <"$i"
18781894
done
18791895
((${#COMPREPLY[@]})) &&
@@ -1935,7 +1951,6 @@ _known_hosts_real()
19351951
$(compgen -A hostname -P "$prefix" -S "$suffix" -- "$cur"))
19361952
fi
19371953

1938-
IFS=$' \t\n'
19391954
$reset
19401955

19411956
if ((${#COMPREPLY[@]})); then
@@ -2349,12 +2364,12 @@ complete -F _minimal ''
23492364
__load_completion()
23502365
{
23512366
local -a dirs=(${BASH_COMPLETION_USER_DIR:-${XDG_DATA_HOME:-$HOME/.local/share}/bash-completion}/completions)
2352-
local ifs=$IFS IFS=: dir cmd="${1##*/}" compfile
2367+
local IFS=: dir cmd="${1##*/}" compfile
23532368
[[ -n $cmd ]] || return 1
23542369
for dir in ${XDG_DATA_DIRS:-/usr/local/share:/usr/share}; do
23552370
dirs+=($dir/bash-completion/completions)
23562371
done
2357-
IFS=$ifs
2372+
_comp_unlocal IFS
23582373
23592374
if [[ $BASH_SOURCE == */* ]]; then
23602375
dirs+=("${BASH_SOURCE%/*}/completions")

completions/7z

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,9 @@ _7z()
4949
local reset=$(shopt -po noglob)
5050
set -o noglob
5151
compopt -o filenames
52-
local ifs=$IFS IFS=$'\n'
52+
local IFS=$'\n'
5353
COMPREPLY=($(compgen -d -P${cur:0:2} -S/ -- "${cur:2}"))
54-
IFS=$ifs
54+
_comp_unlocal IFS
5555
$reset
5656
compopt -o nospace
5757
return

completions/_umount.linux

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,9 @@ _reply_compgen_array()
2525
ecur=${ecur//\'/\\\'}
2626

2727
# Actually generate completions.
28-
local ifs=$IFS
29-
IFS=$'\n' eval 'COMPREPLY=(`compgen -W "$wlist" -- "${ecur}"`)'
30-
IFS=$ifs
28+
local IFS=$'\n'
29+
COMPREPLY=($(compgen -W "$wlist" -- "${ecur}"))
30+
_comp_unlocal IFS
3131
}
3232

3333
# Unescape strings in the linux fstab(5) format (with octal escapes).
@@ -49,24 +49,23 @@ _linux_fstab()
4949

5050
# Read and unescape values into COMPREPLY
5151
local fs_spec fs_file fs_other
52-
local ifs="$IFS"
5352
while read -r fs_spec fs_file fs_other; do
5453
if [[ $fs_spec == [#]* ]]; then continue; fi
5554
if [[ ${1-} == -L ]]; then
5655
local fs_label=${fs_spec/#LABEL=/}
5756
if [[ $fs_label != "$fs_spec" ]]; then
5857
__linux_fstab_unescape fs_label
59-
IFS=$'\0'
58+
local IFS=$'\0'
6059
COMPREPLY+=("$fs_label")
61-
IFS=$ifs
60+
_comp_unlocal IFS
6261
fi
6362
else
6463
__linux_fstab_unescape fs_spec
6564
__linux_fstab_unescape fs_file
66-
IFS=$'\0'
65+
local IFS=$'\0'
6766
[[ $fs_spec == */* ]] && COMPREPLY+=("$fs_spec")
6867
[[ $fs_file == */* ]] && COMPREPLY+=("$fs_file")
69-
IFS=$ifs
68+
_comp_unlocal IFS
7069
fi
7170
done
7271

completions/svcadm

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,9 @@ _smf_complete_fmri()
4444
# we generate all possibles abbreviations for the FMRI
4545
# no need to have a generic loop as we will have a finite
4646
# number of components
47-
local ifs="$IFS"
48-
IFS="/"
47+
local IFS="/"
4948
set -- $fmri
50-
IFS=$ifs
49+
_comp_unlocal IFS
5150
case $# in
5251
1) fmri_part_list=" $1" ;;
5352
2) fmri_part_list=" $2 $1/$2" ;;

test/t/unit/test_unit_unlocal.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import pytest
2+
3+
from conftest import assert_bash_exec
4+
5+
6+
@pytest.mark.bashcomp(cmd=None, ignore_env=r"^\+(VAR=|declare -f foo)")
7+
class TestUnlocal:
8+
def test_1(self, bash):
9+
cmd = (
10+
"foo() { "
11+
"local VAR=inner; "
12+
"_comp_unlocal VAR; "
13+
"echo $VAR; "
14+
"}; "
15+
"VAR=outer; foo; "
16+
)
17+
res = assert_bash_exec(bash, cmd, want_output=True).strip()
18+
assert res == "outer"

0 commit comments

Comments
 (0)