Mailing List Archive

scp3.sh regression test fails if scp not already installed
Hey all,

I know this is a corner case but it turns out that the scp3.sh
regression test fails if you don't already have scp installed in your
path. I know a lot of people won't run into that but it's sort of a
chicken or egg problem.

The following patch checks to see if it's in the path or if it's only in
the openssh-portable build directory. I have a strong suspicion that
this test will fail in debuild or rpmbuild environments but it will fail
in a way that the test is run.

Chris


diff --git a/regress/scp3.sh b/regress/scp3.sh
index f71b15677..7555e4e4d 100644
--- a/regress/scp3.sh
+++ b/regress/scp3.sh
@@ -14,6 +14,15 @@ cp ${SRC}/scp-ssh-wrapper.sh ${OBJ}/scp-ssh-wrapper.scp
chmod 755 ${OBJ}/scp-ssh-wrapper.scp
export SCP # used in scp-ssh-wrapper.scp

+
+scp_path=$(which -a scp)
+if [ -x "$scp_path" ] || [ $scp_path == "*openssh-portable*" ]; then
+ echo "*****************"
+ echo "scp not found in path. Skipping scp3 test."
+ echo "*****************"
+ exit
+fi
+
scpclean() {
rm -rf ${COPY} ${COPY2} ${DIR} ${DIR2}
mkdir ${DIR} ${DIR2}
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
Re: scp3.sh regression test fails if scp not already installed [ In reply to ]
Hi.

On Sat, 23 Jul 2022 at 05:34, rapier <rapier@psc.edu> wrote:
[...]
> The following patch checks to see if it's in the path or if it's only in
> the openssh-portable build directory. I have a strong suspicion that
> this test will fail in debuild or rpmbuild environments but it will fail
> in a way that the test is run.

Thanks for the report. I am fine with doing this in principle but
there's a few problems with the specifics of this patch:

> +scp_path=$(which -a scp)
> +if [ -x "$scp_path" ]

"which" is not POSIX, its semantics vary wildly and not all of them
have a "-a". This test is probably also going to do the wrong thing
in the case where there are two scps on the path. There's a
"have_prog" function in test-exec.sh in portable for this specific
purpose.

Also, the $() subshell syntax is not in the earlier POSIX standards
and doesn't work on older shells. Solaris is particularly annoying in
this regard: /bin/sh on Solaris <=10 doesn't understand $() while on
Solaris 11 it's ksh in disguise and does but will complain about
backticks (under at least some conditions).

> || [ $scp_path == "*openssh-portable*" ]; then

What's the purpose of this? In case the build dir is in the build
shell's path but not the login shell's? There's no guarantee the
build directory is going to be named in a way that matches. Also,
pattern matching with "==" is not POSIX (we would use a "case" if we
needed to do this, but I think there's a better way here).

Instead, I think what we would need to do is check for scp in the path
over the ssh connection in a similar way to how have_prog does it.

> + echo "scp not found in path. Skipping scp3 test."
> + echo "*****************"
> + exit

test-exec.sh has a "skip" function to do this in a consistent (and shorter) way.

After a bit of fiddling, this seems to work, which I'll commit shortly:

$SSH -F $OBJ/ssh_proxy somehost \
'IFS=":"; for i in $PATH;do [ -x "$i/scp" ] && exit 0; done; exit 1'
if [ $? -eq 1 ]; then
skip "No scp on remote path."
fi

--
Darren Tucker (dtucker at dtucker.net)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860 37F4 9357 ECEF 11EA A6FA (new)
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
Re: scp3.sh regression test fails if scp not already installed [ In reply to ]
On Sat, 23 Jul 2022, Darren Tucker wrote:

> Solaris 11 it's ksh in disguise and does but will complain about
> backticks (under at least some conditions).

The accent-gravis form of command substitution is n?n-portable in
at least one important regard: the handling of double quotes.

In general, you can have double quotes *either* outside *or* inside
the accent-gravis command substitution…

echo "foo `echo bar` baz"

echo foo `echo "$bar"` baz

… but NOT both:

echo "foo `echo "$bar"` baz" # IS AN ERROR

This is because some shells priorise the accent-gravis over
the double quote, some shells priorise the double quote higher.

In general, it’s unsafe to use. I do use it in the build script
for mksh, because that one has to be portable, but for most uses
it might be sensible to require the shell to be somewhat capable.

(mksh/Build.sh requires the shell to have functions; some very
old ash variants lacked them, but this is where I draw the line…
GNU autoconf is generated so they can expand inline all the time,
but for hand-written scripts, this isn’t tenable.)

bye,
//mirabilos (mksh developer)
--
Infrastrukturexperte • tarent solutions GmbH
Am Dickobskreuz 10, D-53121 Bonn • http://www.tarent.de/
Telephon +49 228 54881-393 • Fax: +49 228 54881-235
HRB AG Bonn 5168 • USt-ID (VAT): DE122264941
Geschäftsführer: Dr. Stefan Barth, Kai Ebenrett, Boris Esser, Alexander Steeg

****************************************************
/?\ The UTF-8 Ribbon
? ? Campaign against Mit dem tarent-Newsletter nichts mehr verpassen:
 ?  HTML eMail! Also, https://www.tarent.de/newsletter
? ? header encryption!
****************************************************
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
Re: scp3.sh regression test fails if scp not already installed [ In reply to ]
On Mon, Jul 25, 2022 at 11:58:30PM +0200, Thorsten Glaser wrote:
> (mksh/Build.sh requires the shell to have functions; some very
> old ash variants lacked them, but this is where I draw the line…
> GNU autoconf is generated so they can expand inline all the time,
> but for hand-written scripts, this isn’t tenable.)

This was true of Autoconf many years ago, but it hasn't been the case
since Autoconf 2.63b, released in 2009 - even that notoriously
conservative project draws the line there nowadays.

--
Colin Watson (he/him) [cjwatson@debian.org]
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
Re: scp3.sh regression test fails if scp not already installed [ In reply to ]
On Tue, 26 Jul 2022, Jim Knoble wrote:

> The idiom I've seen work most portably around backticks and double quotes is:
>
> x="`echo \"words with spaces\"`"

Nah, it’s not clear how many backspaces you need with this.
Avoid using double quotes with accent-gravis style comsubs.

(In the shown example, you don’t need them outside, because
this is a scalar assignment and thus not subject to word
splitting.)

bye,
//mirabilos
--
Infrastrukturexperte • tarent solutions GmbH
Am Dickobskreuz 10, D-53121 Bonn • http://www.tarent.de/
Telephon +49 228 54881-393 • Fax: +49 228 54881-235
HRB AG Bonn 5168 • USt-ID (VAT): DE122264941
Geschäftsführer: Dr. Stefan Barth, Kai Ebenrett, Boris Esser, Alexander Steeg

****************************************************
/?\ The UTF-8 Ribbon
? ? Campaign against Mit dem tarent-Newsletter nichts mehr verpassen:
 ?  HTML eMail! Also, https://www.tarent.de/newsletter
? ? header encryption!
****************************************************
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
Re: scp3.sh regression test fails if scp not already installed [ In reply to ]
Hey, sorry for the late reply. I was at a conference and then got food
poisoning so I've been spending way too long catching up.

On 7/23/22 12:33 AM, Darren Tucker wrote:
>
> Thanks for the report. I am fine with doing this in principle but
> there's a few problems with the specifics of this patch:

That's cool. I'm not really very good at shell scripting (at all) and I
just went with the first thing I found that worked in my context. I
submitted the patch as an example rather than anything I expected to
have included.

>> || [ $scp_path == "*openssh-portable*" ]; then
>
> What's the purpose of this? In case the build dir is in the build
> shell's path but not the login shell's?

Yup. I knew it was fragile but I didn't know of a better way of doing it.

> After a bit of fiddling, this seems to work, which I'll commit shortly:
>
> $SSH -F $OBJ/ssh_proxy somehost \
> 'IFS=":"; for i in $PATH;do [ -x "$i/scp" ] && exit 0; done; exit 1'
> if [ $? -eq 1 ]; then
> skip "No scp on remote path."
> fi

Sweet! I'll get that into my code base as well. Thanks for looking at this.

Chris
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev