From a1e7ea0837ead8eb96c5e6b9a0f9b28051e48d97 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Thu, 27 Aug 2026 14:22:17 -0700 Subject: [PATCH 1/2] scripts: harden sftp.test The ready-file counter is shared by every create_port call and was never reset, so the twenty iterations are a budget for the whole run rather than per server start. A slow first start leaves later ones with no wait at all, and the test fails with "NO ready file" instead of waiting. The guard after the loop then disagreed with the loop as well: the wait is for a non-empty file, but the guard only asked whether the file existed, so a wait that ran out still went on to read an empty port. - reset the counter in create_port, so each server start gets the full wait and a new call site cannot forget it - test the guard on -s, matching what the loop waited for - drop "echo -e", undefined for POSIX sh and printed literally by shells that do not take the flag; the escapes were only ever blank lines - quote the expansions, use $(...) instead of backticks, and grep -q - exit 1 rather than exit -1 from the trap handler - drop the redundant PWD assignment before the set-directory test, and the second copy of the wolfsftp executable check Leaves shellcheck -s sh clean apart from SC2329 on the trap handler. --- scripts/sftp.test | 93 +++++++++++++++++++++++++---------------------- 1 file changed, 50 insertions(+), 43 deletions(-) diff --git a/scripts/sftp.test b/scripts/sftp.test index 4e6a5763b..3d5304cfe 100755 --- a/scripts/sftp.test +++ b/scripts/sftp.test @@ -3,12 +3,11 @@ # sftp local test no_pid=-1 -server_pid=$no_pid -ready_file=`pwd`/wolfssh_sftp_ready$$ -counter=0 +server_pid="$no_pid" +ready_file="$PWD/wolfssh_sftp_ready$$" nonblockingOnly=0 -[ ! -x ./examples/sftpclient/wolfsftp ] && echo -e "\n\nwolfSFTP client doesn't exist" && exit 1 +[ ! -x ./examples/sftpclient/wolfsftp ] && echo && echo "wolfSFTP client doesn't exist" && exit 1 WOLFSSH_OPTIONS=$(./apps/wolfssh-options) || { echo "fail: could not run ./apps/wolfssh-options" @@ -33,38 +32,43 @@ fi #echo "ready file $ready_file" create_port() { + # counted per server start, so a slow first start cannot eat the wait + # the later ones need + counter=0 while [ ! -s "$ready_file" ] && [ "$counter" -lt 20 ]; do - echo -e "waiting for ready file..." + echo "waiting for ready file..." sleep 0.1 counter=$((counter+ 1)) done - if test -e $ready_file; then - echo -e "found ready file, starting client..." + # -s, not -e: the loop waits for the port to be written, and an empty + # file here means the wait ran out + if [ -s "$ready_file" ]; then + echo "found ready file, starting client..." # get created port 0 ephemeral port - port=`cat $ready_file` + port=$(cat "$ready_file") else - echo -e "NO ready file ending test..." + echo "NO ready file ending test..." do_cleanup exit 1 fi } remove_ready_file() { - if test -e $ready_file; then - echo -e "removing existing ready file" - rm $ready_file + if test -e "$ready_file"; then + echo "removing existing ready file" + rm "$ready_file" fi } do_cleanup() { echo "in cleanup" - if [ $server_pid != $no_pid ] + if [ "$server_pid" != "$no_pid" ] then echo "killing server" - kill -9 $server_pid + kill -9 "$server_pid" fi remove_ready_file } @@ -72,23 +76,22 @@ do_cleanup() { do_trap() { echo "got trap" do_cleanup - exit -1 + exit 1 } trap do_trap INT TERM -[ ! -x ./examples/sftpclient/wolfsftp ] && echo -e "\n\nClient doesn't exist" && exit 1 - -if [ $nonblockingOnly = 0 ]; then +if [ "$nonblockingOnly" = 0 ]; then echo "Test basic connection" - ./examples/echoserver/echoserver -1 -R $ready_file & + ./examples/echoserver/echoserver -1 -R "$ready_file" & server_pid=$! create_port - echo "exit" | ./examples/sftpclient/wolfsftp -u jill -P upthehill -p $port + echo "exit" | ./examples/sftpclient/wolfsftp -u jill -P upthehill -p "$port" RESULT=$? remove_ready_file if [ $RESULT -ne 0 ]; then - echo -e "\n\nfailed to connect" + echo + echo "failed to connect" do_cleanup exit 1 fi @@ -96,79 +99,83 @@ fi # Test non blocking connection echo "Test non blocking connection" -./examples/echoserver/echoserver -N -1 -R $ready_file & +./examples/echoserver/echoserver -N -1 -R "$ready_file" & server_pid=$! create_port -echo "exit" | ./examples/sftpclient/wolfsftp -N -u jill -P upthehill -p $port +echo "exit" | ./examples/sftpclient/wolfsftp -N -u jill -P upthehill -p "$port" RESULT=$? remove_ready_file if [ $RESULT -ne 0 ]; then - echo -e "\n\nfailed to connect" + echo + echo "failed to connect" do_cleanup exit 1 fi # Test want write return from highwater callback -if [ $nonblockingOnly = 0 ]; then +if [ "$nonblockingOnly" = 0 ]; then echo "Test want write return from highwater callback" - ./examples/echoserver/echoserver -H -N -1 -R $ready_file & + ./examples/echoserver/echoserver -H -N -1 -R "$ready_file" & server_pid=$! create_port - ./examples/sftpclient/wolfsftp -N -u jill -P upthehill -p $port -g -r $PWD/README.md-2 -l $PWD/README.md + ./examples/sftpclient/wolfsftp -N -u jill -P upthehill -p "$port" -g -r "$PWD/README.md-2" -l "$PWD/README.md" RESULT=$? remove_ready_file - rm -f $PWD/README.md-2 + rm -f "$PWD/README.md-2" if [ $RESULT -ne 0 ]; then - echo -e "\n\nfailed to connect" + echo + echo "failed to connect" do_cleanup exit 1 fi fi # Test of setting directory -if [ $nonblockingOnly = 0 ]; then +if [ "$nonblockingOnly" = 0 ]; then echo "Test of setting directory" - PWD=`pwd` - ./examples/echoserver/echoserver -d $PWD/examples -1 -R $ready_file & + ./examples/echoserver/echoserver -d "$PWD/examples" -1 -R "$ready_file" & server_pid=$! create_port - echo "exit" | ./examples/sftpclient/wolfsftp -N -u jill -P upthehill -p $port + echo "exit" | ./examples/sftpclient/wolfsftp -N -u jill -P upthehill -p "$port" RESULT=$? remove_ready_file if [ $RESULT -ne 0 ]; then - echo -e "\n\nfailed to connect" + echo + echo "failed to connect" do_cleanup exit 1 fi fi # Test SFTP status failure for non-existing file -if [ $nonblockingOnly = 0 ]; then +if [ "$nonblockingOnly" = 0 ]; then echo "Test SFTP status failure for non-existing file" - ./examples/echoserver/echoserver -N -1 -R $ready_file & + ./examples/echoserver/echoserver -N -1 -R "$ready_file" & server_pid=$! create_port - ./examples/sftpclient/wolfsftp -N -u jill -P upthehill -p $port -G -r $PWD/this_file_does_not_exist_12345.txt -l $PWD/test_output.txt > sftp_status_failure.log 2>&1 + ./examples/sftpclient/wolfsftp -N -u jill -P upthehill -p "$port" -G -r "$PWD/this_file_does_not_exist_12345.txt" -l "$PWD/test_output.txt" > sftp_status_failure.log 2>&1 RESULT=$? - grep "Unable to copy remote file" sftp_status_failure.log > /dev/null + grep -q "Unable to copy remote file" sftp_status_failure.log RESULT_MSG=$? remove_ready_file rm -f sftp_status_failure.log - rm -f $PWD/test_output.txt + rm -f "$PWD/test_output.txt" if [ $RESULT -eq 0 ]; then - echo -e "\n\nERROR: Should have failed for non-existing file" + echo + echo "ERROR: Should have failed for non-existing file" do_cleanup exit 1 fi if [ $RESULT_MSG -ne 0 ]; then - echo -e "\n\nERROR: Unexpected failure path while testing missing remote file" + echo + echo "ERROR: Unexpected failure path while testing missing remote file" do_cleanup exit 1 fi echo "Successfully received failure status packet" fi -echo -e "\nALL Tests Passed" +echo +echo "ALL Tests Passed" exit 0 - From 98685230feae6b6fd2ced4c224409c89bb8098de Mon Sep 17 00:00:00 2001 From: John Safranek Date: Fri, 28 Aug 2026 16:53:13 -0700 Subject: [PATCH 2/2] scripts: remove the external test external.test connected the client and SFTP client to a host named by WOLFSSH_EXTERNAL_HOST. It only ran when WOLFSSH_EXTERNAL_TEST was set, which nothing in the tree or in CI does, so it always exited 77. - delete scripts/external.test - drop it from scripts/include.am --- scripts/external.test | 73 ------------------------------------------- scripts/include.am | 2 +- 2 files changed, 1 insertion(+), 74 deletions(-) delete mode 100755 scripts/external.test diff --git a/scripts/external.test b/scripts/external.test deleted file mode 100755 index 7dbc2e789..000000000 --- a/scripts/external.test +++ /dev/null @@ -1,73 +0,0 @@ -#!/bin/sh - -# external tests -host="$WOLFSSH_EXTERNAL_HOST" -user="$WOLFSSH_EXTERNAL_USER" -password="$WOLFSSH_EXTERNAL_PASSWORD" - -if test -n "$WOLFSSH_EXTERNAL_TEST"; then - echo "WOLFSSH_EXTERNAL_TEST set, running test..." -else - echo "WOLFSSH_EXTERNAL_TEST NOT set, won't run" - exit 77 -fi - -# test for nonblocking only -WOLFSSH_OPTIONS=$(./apps/wolfssh-options) || { - echo "fail: could not run ./apps/wolfssh-options" - exit 1 -} - -if echo "$WOLFSSH_OPTIONS" | grep -qx "TEST_BLOCK" -then - echo "macro WOLFSSH_TEST_BLOCK was used" - echo "skipping for now" - exit 77 -fi - -do_cleanup() { - echo "in cleanup" -} - -do_trap() { - echo "got trap" - do_cleanup - exit -1 -} - -trap do_trap INT TERM - -[ ! -x ./examples/client/client ] && echo -e "\n\nClient doesn't exist" && exit 1 - -echo "Testing client connection to $host : " -./examples/client/client -u $user -P $password -h $host -p 22 -x -RESULT=$? -if [ $RESULT -ne 0 ]; then - echo -e "failed to connect\n" - do_cleanup - exit 1 -fi -echo -e "Success\n" - - -# not having the sftp client built in is not a failure case -./examples/sftpclient/wolfsftp -h -if [ $? -eq 0 ]; then - echo "Testing wolfsftp connection to $host : " - echo "exit" | ./examples/sftpclient/wolfsftp -u $user -P $password -h $host -p 22 - RESULT=$? - if [ $RESULT -ne 0 ]; then - echo -e "failed to connect\n" - do_cleanup - exit 1 - else - echo -e "Success\n" - fi -else - echo -e "\n\nwolfSFTP client doesn't exist" -fi - -echo -e "\nALL Tests Passed" - -exit 0 - diff --git a/scripts/include.am b/scripts/include.am index 4fbfe39ad..d1fa5d8c0 100644 --- a/scripts/include.am +++ b/scripts/include.am @@ -15,5 +15,5 @@ endif # app wasn't built. dist_noinst_SCRIPTS+= scripts/sshclient.test -dist_noinst_SCRIPTS+= scripts/external.test scripts/fwd.test +dist_noinst_SCRIPTS+= scripts/fwd.test EXTRA_DIST += scripts/fwd.test.expect