ssh: fix call to Fatalf from a non-test goroutine

also fix some redundant type declarations
golang · Jun 24, 2023 · 9d84158 · 9d84158
1 parent 183630a
commit 9d84158
Show file tree

Hide file tree

Showing 8 changed files with 165 additions and 80 deletions.
diff --git a/ssh/agent/client_test.go b/ssh/agent/client_test.go
@@ -363,10 +363,12 @@ func TestAuth(t *testing.T) {
 		return nil, errors.New("pubkey rejected")
 	}
 
+	errorCh := make(chan error, 1)
 	go func() {
 		conn, _, _, err := ssh.NewServerConn(a, &serverConf)
+		errorCh <- err
 		if err != nil {
-			t.Fatalf("Server: %v", err)
+			return
 		}
 		conn.Close()
 	}()
@@ -380,6 +382,9 @@ func TestAuth(t *testing.T) {
 		t.Fatalf("NewClientConn: %v", err)
 	}
 	conn.Close()
+	if err := <-errorCh; err != nil {
+		t.Fatalf("Server: %v", err)
+	}
 }
 
 func TestLockOpenSSHAgent(t *testing.T) {

diff --git a/ssh/agent/server_test.go b/ssh/agent/server_test.go
@@ -51,10 +51,12 @@ func TestSetupForwardAgent(t *testing.T) {
 	}
 	serverConf.AddHostKey(testSigners["rsa"])
 	incoming := make(chan *ssh.ServerConn, 1)
+	errorCh := make(chan error, 1)
 	go func() {
 		conn, _, _, err := ssh.NewServerConn(a, &serverConf)
+		errorCh <- err
 		if err != nil {
-			t.Fatalf("Server: %v", err)
+			return
 		}
 		incoming <- conn
 	}()
@@ -71,7 +73,9 @@ func TestSetupForwardAgent(t *testing.T) {
 	if err := ForwardToRemote(client, socket); err != nil {
 		t.Fatalf("SetupForwardAgent: %v", err)
 	}
-
+	if err := <-errorCh; err != nil {
+		t.Fatalf("Server: %v", err)
+	}
 	server := <-incoming
 	ch, reqs, err := server.OpenChannel(channelType, nil)
 	if err != nil {

diff --git a/ssh/benchmark_test.go b/ssh/benchmark_test.go
@@ -6,6 +6,7 @@ package ssh
 
 import (
 	"errors"
+	"fmt"
 	"io"
 	"net"
 	"testing"
@@ -90,16 +91,16 @@ func BenchmarkEndToEnd(b *testing.B) {
 	go func() {
 		newCh, err := server.Accept()
 		if err != nil {
-			b.Fatalf("Client: %v", err)
+			panic(fmt.Sprintf("Client: %v", err))
 		}
 		ch, incoming, err := newCh.Accept()
 		if err != nil {
-			b.Fatalf("Accept: %v", err)
+			panic(fmt.Sprintf("Accept: %v", err))
 		}
 		go DiscardRequests(incoming)
 		for i := 0; i < b.N; i++ {
 			if _, err := io.ReadFull(ch, output); err != nil {
-				b.Fatalf("ReadFull: %v", err)
+				panic(fmt.Sprintf("ReadFull: %v", err))
 			}
 		}
 		ch.Close()

diff --git a/ssh/common_test.go b/ssh/common_test.go
@@ -82,35 +82,35 @@ func TestFindAgreedAlgorithms(t *testing.T) {
 	}
 
 	cases := []testcase{
-		testcase{
+		{
 			name: "standard",
 		},
 
-		testcase{
+		{
 			name: "no common hostkey",
 			serverIn: kexInitMsg{
 				ServerHostKeyAlgos: []string{"hostkey2"},
 			},
 			wantErr: true,
 		},
 
-		testcase{
+		{
 			name: "no common kex",
 			serverIn: kexInitMsg{
 				KexAlgos: []string{"kex2"},
 			},
 			wantErr: true,
 		},
 
-		testcase{
+		{
 			name: "no common cipher",
 			serverIn: kexInitMsg{
 				CiphersClientServer: []string{"cipher2"},
 			},
 			wantErr: true,
 		},
 
-		testcase{
+		{
 			name: "client decides cipher",
 			serverIn: kexInitMsg{
 				CiphersClientServer: []string{"cipher1", "cipher2"},

diff --git a/ssh/handshake_test.go b/ssh/handshake_test.go
@@ -148,6 +148,7 @@ func TestHandshakeBasic(t *testing.T) {
 	clientDone := make(chan int, 0)
 	gotHalf := make(chan int, 0)
 	const N = 20
+	errorCh := make(chan error, 1)
 
 	go func() {
 		defer close(clientDone)
@@ -158,7 +159,9 @@ func TestHandshakeBasic(t *testing.T) {
 		for i := 0; i < N; i++ {
 			p := []byte{msgRequestSuccess, byte(i)}
 			if err := trC.writePacket(p); err != nil {
-				t.Fatalf("sendPacket: %v", err)
+				errorCh <- err
+				trC.Close()
+				return
 			}
 			if (i % 10) == 5 {
 				<-gotHalf
@@ -177,16 +180,15 @@ func TestHandshakeBasic(t *testing.T) {
 				checker.waitCall <- 1
 			}
 		}
+		errorCh <- nil
 	}()
 
 	// Server checks that client messages come in cleanly
 	i := 0
-	err = nil
 	for ; i < N; i++ {
-		var p []byte
-		p, err = trS.readPacket()
-		if err != nil {
-			break
+		p, err := trS.readPacket()
+		if err != nil && err != io.EOF {
+			t.Fatalf("server error: %v", err)
 		}
 		if (i % 10) == 5 {
 			gotHalf <- 1
@@ -198,8 +200,8 @@ func TestHandshakeBasic(t *testing.T) {
 		}
 	}
 	<-clientDone
-	if err != nil && err != io.EOF {
-		t.Fatalf("server error: %v", err)
+	if err := <-errorCh; err != nil {
+		t.Fatalf("sendPacket: %v", err)
 	}
 	if i != N {
 		t.Errorf("received %d messages, want 10.", i)
@@ -345,16 +347,16 @@ func TestHandshakeAutoRekeyRead(t *testing.T) {
 
 	// While we read out the packet, a key change will be
 	// initiated.
-	done := make(chan int, 1)
+	errorCh := make(chan error, 1)
 	go func() {
-		defer close(done)
-		if _, err := trC.readPacket(); err != nil {
-			t.Fatalf("readPacket(client): %v", err)
-		}
-
+		_, err := trC.readPacket()
+		errorCh <- err
 	}()
 
-	<-done
+	if err := <-errorCh; err != nil {
+		t.Fatalf("readPacket(client): %v", err)
+	}
+
 	<-sync.called
 }