[PATCH 0/1] tools/bash-completion: fix variable leaks of "IFS" and "word"
Description: The bash-completion function for virsh, which is defined in /usr/share/bash-completion/completions/virsh (generated from libvirt tools/bash-completion/vsh.in), overwrites the shell variables "IFS" and "word" on query for completions. Repeat-By: I am testing it with Fedora 35 Workstation where the completion script is provided by the package: libvirt-client-7.6.0-3.fc35.x86_64 With a Bash session with bash-completion loaded, one can observe the change of Bash behavior after the attempt of the completion for "virsh" command as follows: $ declare -p IFS | cat -A declare -- IFS=" ^I$ "$ $ virsh a[TAB][TAB] allocpages attach-device attach-disk attach-interface autostart $ virsh a[C-c] $ declare -p IFS | cat -A declare -- IFS="$ "$ Originally the value of IFS is IFS=$' \t\n', but it becomes IFS=$'\n' after the attempt of the completion. After that, word splitting of the shell will not be processed as normal: $ a='a b c' $ printf '<%s>\n' $a <a> <b> <c> $ virsh a[TAB][TAB][C-c] $ printf '<%s>\n' $a <a b c> The shell variable "word" is also affected by the virsh completion. Fix: The completion function "_virsh_complete" should declare "IFS" and "word" as local variables before changing them. Reference: I initially received the related issue in my project at https://github.com/akinomyoga/ble.sh/issues/147 -- Koichi Koichi Murase (1): bash-completion: fix variable leaks of "IFS" and "word" tools/bash-completion/vsh.in | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) -- 2.21.3
--- tools/bash-completion/vsh.in | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/bash-completion/vsh.in b/tools/bash-completion/vsh.in index 70ade50a02..4399ff0a64 100644 --- a/tools/bash-completion/vsh.in +++ b/tools/bash-completion/vsh.in @@ -4,7 +4,7 @@ _@command@_complete() { - local words cword c=0 i=0 cur RO URI CMDLINE INPUT A + local words cword c=0 i=0 cur word RO URI CMDLINE INPUT A # Here, $COMP_WORDS is an array of words on the bash # command line that user wants to complete. However, when @@ -56,7 +56,8 @@ _@command@_complete() # the name of the command whose arguments are being # completed. # Therefore, we might just run $1. - IFS=$'\n' A=($($1 ${CMDLINE[@]} complete -- "${INPUT[@]}" 2>/dev/null)) + local IFS=$'\n' + A=($($1 ${CMDLINE[@]} complete -- "${INPUT[@]}" 2>/dev/null)) COMPREPLY=($(compgen -W "${A[*]%--}" -- ${cur})) __ltrim_colon_completions "$cur" -- 2.21.3
On 11/14/21 07:06, Koichi Murase wrote:
--- tools/bash-completion/vsh.in | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/tools/bash-completion/vsh.in b/tools/bash-completion/vsh.in index 70ade50a02..4399ff0a64 100644 --- a/tools/bash-completion/vsh.in +++ b/tools/bash-completion/vsh.in @@ -4,7 +4,7 @@
_@command@_complete() { - local words cword c=0 i=0 cur RO URI CMDLINE INPUT A + local words cword c=0 i=0 cur word RO URI CMDLINE INPUT A
The word variable is also used only inside a while loop (not visible in the context). So it can be declared local there instead of here. I'll fix that before pushing. However, we do require developers to provide signoff for their patches to signal their compliance with DCO: https://libvirt.org/hacking.html#developer-certificate-of-origin No need to resend the whole patch, it's sufficient if you reply with your SoB line. Michal
Thank you for reviewing the patch. 2021年11月23日(火) 21:37 Michal Prívozník <mprivozn@redhat.com>:
[...]
The word variable is also used only inside a while loop (not visible in the context). So it can be declared local there instead of here. I'll fix that before pushing.
OK.
However, we do require developers to provide signoff for their patches to signal their compliance with DCO:
https://libvirt.org/hacking.html#developer-certificate-of-origin
No need to resend the whole patch, it's sufficient if you reply with your SoB line.
OK. Are just the following lines sufficient? -- [PATCH 1/1] bash-completion: fix variable leaks of "IFS" and "word" Signed-off-by: Koichi Murase <myoga.murase@gmail.com> -- Thank you, Koichi
On 11/23/21 13:50, Koichi Murase wrote:
Thank you for reviewing the patch.
2021年11月23日(火) 21:37 Michal Prívozník <mprivozn@redhat.com>:
[...]
The word variable is also used only inside a while loop (not visible in the context). So it can be declared local there instead of here. I'll fix that before pushing.
OK.
However, we do require developers to provide signoff for their patches to signal their compliance with DCO:
https://libvirt.org/hacking.html#developer-certificate-of-origin
No need to resend the whole patch, it's sufficient if you reply with your SoB line.
OK. Are just the following lines sufficient? -- [PATCH 1/1] bash-completion: fix variable leaks of "IFS" and "word"
Signed-off-by: Koichi Murase <myoga.murase@gmail.com>
Perfect. Reviewed-by: Michal Privoznik <mprivozn@redhat.com> and pushed. Congratulations on your first libvirt contribution! Michal
participants (2)
-
Koichi Murase -
Michal Prívozník