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

Also fix some redundant type declarations. Change-Id: Iad2950b67b1ec2e2590c59393b8ad15421ed3add GitHub-Last-Rev: 41cf552 GitHub-Pull-Request: #263 Reviewed-on: https://go-review.googlesource.com/c/crypto/+/505798 Reviewed-by: Dmitri Shuralyov <[email protected]> Reviewed-by: David Chase <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Auto-Submit: Dmitri Shuralyov <[email protected]> Reviewed-by: Filippo Valsorda <[email protected]> Run-TryBot: Filippo Valsorda <[email protected]>
golang · Jul 31, 2023 · edc325d · edc325d
1 parent eab9315
commit edc325d
Show file tree

Hide file tree

Showing 8 changed files with 140 additions and 73 deletions.
diff --git a/ssh/agent/client_test.go b/ssh/agent/client_test.go
@@ -369,7 +369,8 @@ func TestAuth(t *testing.T) {
 	go func() {
 		conn, _, _, err := ssh.NewServerConn(a, &serverConf)
 		if err != nil {
-			t.Fatalf("Server: %v", err)
+			t.Errorf("NewServerConn error: %v", err)
+			return
 		}
 		conn.Close()
 	}()

diff --git a/ssh/agent/server_test.go b/ssh/agent/server_test.go
@@ -53,10 +53,11 @@ func TestSetupForwardAgent(t *testing.T) {
 	incoming := make(chan *ssh.ServerConn, 1)
 	go func() {
 		conn, _, _, err := ssh.NewServerConn(a, &serverConf)
+		incoming <- conn
 		if err != nil {
-			t.Fatalf("Server: %v", err)
+			t.Errorf("NewServerConn error: %v", err)
+			return
 		}
-		incoming <- conn
 	}()
 
 	conf := ssh.ClientConfig{
@@ -71,8 +72,10 @@ func TestSetupForwardAgent(t *testing.T) {
 	if err := ForwardToRemote(client, socket); err != nil {
 		t.Fatalf("SetupForwardAgent: %v", err)
 	}
-
 	server := <-incoming
+	if server == nil {
+		t.Fatal("Unable to get server")
+	}
 	ch, reqs, err := server.OpenChannel(channelType, nil)
 	if err != nil {
 		t.Fatalf("OpenChannel(%q): %v", channelType, err)

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
 }