Bugs item #311595, was changed at 2009-04-11 04:30 by Freddy Vulto You can respond by visiting: https://alioth.debian.org/tracker/?func=detail&atid=413095&aid=311595&group_id=100114
>Status: Closed Priority: 3 Submitted By: Olivier Crête (tester-guest) Assigned to: Freddy Vulto (fvu-guest) Summary: tries to execute ssh UserKnownHostsFile Distribution: Gentoo Originally reported in: None Milestone: None Status: None Original bug number: Initial Comment: The _known_hosts function tries to execute the UserKnownHostsFile file instead of just getting its name, there is an extra "eval", if you just remove it, it works fine. ---------------------------------------------------------------------- Comment By: Freddy Vulto (fvu-guest) Date: 2009-04-16 23:04 Message: Patch looks good, I'll apply. Since the patch already uses parameter expansion to remove double quotes, the `eval' can be removed as well. Single quotes aren't allowed around a UserKnownHostsFile parameter (tested on OpenSSH_5.1p1 Debian-3ubuntu1, OpenSSL 0.9.8g 19 Oct 2007) so removing the double quotes should be enough. Theoretically, a specified UserKnownHostsFile could contain double quotes. So an improvement still would be to really dequote instead of search-replacing all quotes. An improved `dequote()' should do without `eval' and instead accept an additional parameter $2: ---8<---- (pseudocode) ------------------------------------------------- # $1 mixed Param to dequote # $2 integer Quotes to remove: # 0 (default) Remove single or double quotes # 1 Remove single quotes # 2 Remove double quotes dequote() { if (($2 == 0/1) && ($1 =~ "[\s]'$1'[\s]")) { trim whitespace & remove single quotes } elseif (($2 == 0/2) && ($1 =~ '[\s]"$1"[\s]')) { trim whitespace & remove double quotes fi } ---8<------------------------------------------------------------------- Freddy ---------------------------------------------------------------------- Comment By: David Paleino (hanska-guest) Date: 2009-04-15 14:40 Message: A Debian user reported this bug too, and he attached a patch. Freddy, since you've reassigned the bug to yourself, would you mind taking a look? http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=5;filename=support_multiple_hosts_files.patch;att=1;bug=524190 David ---------------------------------------------------------------------- Comment By: David Paleino (hanska-guest) Date: 2009-04-13 23:45 Message: Aha, maybe re-reading the bugreport I didn't fully understand it. Yes, the quotes are really needed. However, what's the probability of having parameter expansion inside ~/.ssh/config (or whatever)? Freddy, feel free to reassign this bug to yourself if you wish. In the meanwhile, I'm reopening it. ---------------------------------------------------------------------- Comment By: Freddy Vulto (fvu-guest) Date: 2009-04-13 23:35 Message: The `eval' is/was used - and is still necessary - to remove possible quotes around the UserKnownHostsFiles... ("known hosts{1,2}" in my previous message). Maybe we should remove these quotes with parameter expansion instead of 'eval' then? ---------------------------------------------------------------------- Comment By: Freddy Vulto (fvu-guest) Date: 2009-04-13 23:17 Message: The quote around the sed output is still necessary if we want to support the `known_hosts' files containing whitespace, for example with this ssh config: UserKnownHostsFile "known hosts1" # Contains TWO spaces UserKnownHostsFile "known hosts2" # Contains TWO spaces So I'm adding the quotes again around the sed output since removing 'eval' alone already fixes the execution(!!!) :-( $ echo $( sed -ne 's/^[ \t]*UserKnownHostsFile['"$'\t '"']*\(.*\)$/\1/p' config ) "known hosts1" "known hosts2" $ echo "$( sed -ne 's/^[ \t]*UserKnownHostsFile['"$'\t '"']*\(.*\)$/\1/p' config )" "known hosts1" "known hosts2" A TODO is to make ssh completion actually handle the case of more than one specified UserKnownHostsFile (didn't even know it was supported by ssh): `user_kh' and `global_kh' have to become arrays. So we have to watch for all `eval echo'-s then? There's an "eval echo" in the `dequote()' function too. We have to make sure all parameters to `dequote' are quoted (as is done in _ssh()), otherwise there's potentially the same problem(?): $ dequote "$'foo\nbar'" foo bar $ dequote $'foo\nbar' foo bash: bar: command not found Regards, Freddy Vulto ---------------------------------------------------------------------- Comment By: David Paleino (hanska-guest) Date: 2009-04-13 17:11 Message: commit a9994ac15f29892a27ebfff6ded63930a43f1187 Author: David Paleino <d.pale...@gmail.com> Date: Mon Apr 13 17:10:41 2009 +0200 Remove eval() and sed quoting in _known_hosts() (Alioth: #311595) Fixes execution (!!!) of hosts specified by {Global,User}KnownHosts in SSH config files. ---------------------------------------------------------------------- Comment By: David Paleino (hanska-guest) Date: 2009-04-13 17:07 Message: Hello, I clearly remember being able to reproduce the bug at the time. However, I'm not able anymore to reproduce it, and Debian #504650 doesn't show up anymore, both with and without quotes. I've been able to reproduce *this* bug (I tried a mixture of this and Debian #504650, i.e. multiple-spaced hosts on more lines) and seems to work without quotes. Also, I believe those two eval's are not needed at all -- and they cause "execution" of specified hostnames. Thus, I'm fixing this bug -- Freddy, if you have more to say, please reopen this bug and feel free to revert my commit (will arrive in the next minutes). Ciao, David ---------------------------------------------------------------------- Comment By: Ville Skyttä (scop-guest) Date: 2009-04-13 11:10 Message: Ok, I can reproduce now. It also appears to work with the eval if the quotes around "$( sed ... )" are removed, but those were added in http://git.debian.org/?p=bash-completion/bash-completion.git;a=commitdiff;h=fec084a69be9383776090df6facfea1a790c5c66 to fix http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=504650 I was not able to reproduce the issue in Debian bug 504650 (works for me both with and without the quotes) but then again it is missing an exact, concrete reproducer so it could be that I'm not testing the right thing. Freddy, David? I'm not sure if the eval is needed either, but someone who understands Debian bug 504650 should check how removing the eval affects the scenario in it. ---------------------------------------------------------------------- Comment By: Olivier Crête (tester-guest) Date: 2009-04-13 00:07 Message: The difference is that I have two UserKnownHost files in different "Host" sections... So there is a newline in between... Simple example: aa=$(eval echo "$(echo /dev/null; echo /dev/null)") So bashcomp has to be able to deal with multiple answers here, just removing the eval seems to work.. I don't think its required anyway, but maybe I'm missing something (I'm not a bash expert). ---------------------------------------------------------------------- Comment By: Ville Skyttä (scop-guest) Date: 2009-04-12 23:40 Message: I can't reproduce - the two eval lines in _known_hosts are both like foo=$( eval echo bar ) which for me does not try to execute "bar". ---------------------------------------------------------------------- Comment By: Olivier Crête (tester-guest) Date: 2009-04-12 23:03 Message: Its 1.0.. and I also checked the git master (but didn't test it). ---------------------------------------------------------------------- Comment By: Ville Skyttä (scop-guest) Date: 2009-04-12 18:47 Message: Which version of bash_completion is this? If not 1.0, please test with it. ---------------------------------------------------------------------- You can respond by visiting: https://alioth.debian.org/tracker/?func=detail&atid=413095&aid=311595&group_id=100114 _______________________________________________ Bash-completion-devel mailing list Bash-completion-devel@lists.alioth.debian.org http://lists.alioth.debian.org/mailman/listinfo/bash-completion-devel