From 36fcdeeb5edef3ecd5fbbbf8c461f852be39826a Mon Sep 17 00:00:00 2001
From: Paulo Gomes <paulo.gomes@weave.works>
Date: Wed, 30 Mar 2022 14:32:15 +0100
Subject: [PATCH] libgit2: fix access to nil t.stdin and improve observability
 All errors that were previously not handled are now logged through traceLog,
 to further help during transport investigations.

Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
---
 pkg/git/libgit2/managed/http.go |  9 +++++++--
 pkg/git/libgit2/managed/ssh.go  | 24 +++++++++++++++++-------
 2 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/pkg/git/libgit2/managed/http.go b/pkg/git/libgit2/managed/http.go
index 804657564..04e1c54b1 100644
--- a/pkg/git/libgit2/managed/http.go
+++ b/pkg/git/libgit2/managed/http.go
@@ -292,8 +292,13 @@ func (self *httpSmartSubtransportStream) Free() {
 			// ensure body is fully processed and closed
 			// for increased likelihood of transport reuse in HTTP/1.x.
 			// it should not be a problem to do this more than once.
-			_, _ = io.Copy(io.Discard, self.resp.Body) // errors can be safely ignored
-			_ = self.resp.Body.Close()                 // errors can be safely ignored
+			if _, err := io.Copy(io.Discard, self.resp.Body); err != nil {
+				traceLog.Error(err, "[http]: cannot discard response body")
+			}
+
+			if err := self.resp.Body.Close(); err != nil {
+				traceLog.Error(err, "[http]: cannot close response body")
+			}
 		}
 	}
 }
diff --git a/pkg/git/libgit2/managed/ssh.go b/pkg/git/libgit2/managed/ssh.go
index a6d417052..0c7f916de 100644
--- a/pkg/git/libgit2/managed/ssh.go
+++ b/pkg/git/libgit2/managed/ssh.go
@@ -137,7 +137,9 @@ func (t *sshSmartSubtransport) Action(urlString string, action git2go.SmartServi
 			if t.lastAction == git2go.SmartServiceActionUploadpackLs {
 				return t.currentStream, nil
 			}
-			t.Close()
+			if err := t.Close(); err != nil {
+				traceLog.Error(err, "[ssh]: error cleaning up previous stream")
+			}
 		}
 		cmd = fmt.Sprintf("git-upload-pack '%s'", uPath)
 
@@ -146,7 +148,9 @@ func (t *sshSmartSubtransport) Action(urlString string, action git2go.SmartServi
 			if t.lastAction == git2go.SmartServiceActionReceivepackLs {
 				return t.currentStream, nil
 			}
-			t.Close()
+			if err := t.Close(); err != nil {
+				traceLog.Error(err, "[ssh]: error cleaning up previous stream")
+			}
 		}
 		cmd = fmt.Sprintf("git-receive-pack '%s'", uPath)
 
@@ -161,11 +165,11 @@ func (t *sshSmartSubtransport) Action(urlString string, action git2go.SmartServi
 	defer cred.Free()
 
 	var addr string
+	port := "22"
 	if u.Port() != "" {
-		addr = fmt.Sprintf("%s:%s", u.Hostname(), u.Port())
-	} else {
-		addr = fmt.Sprintf("%s:22", u.Hostname())
+		port = u.Port()
 	}
+	addr = fmt.Sprintf("%s:%s", u.Hostname(), port)
 
 	ckey, sshConfig, err := cacheKeyAndConfig(addr, cred)
 	if err != nil {
@@ -264,12 +268,13 @@ func (t *sshSmartSubtransport) Close() error {
 
 	traceLog.Info("[ssh]: sshSmartSubtransport.Close()")
 	t.currentStream = nil
-	if t.client != nil {
+	if t.client != nil && t.stdin != nil {
 		if err := t.stdin.Close(); err != nil {
 			returnErr = fmt.Errorf("cannot close stdin: %w", err)
 		}
-		t.client = nil
 	}
+	t.client = nil
+
 	if t.session != nil {
 		traceLog.Info("[ssh]: skipping session.wait")
 		traceLog.Info("[ssh]: session.Close()")
@@ -277,6 +282,7 @@ func (t *sshSmartSubtransport) Close() error {
 			returnErr = fmt.Errorf("cannot close session: %w", err)
 		}
 	}
+	t.session = nil
 
 	return returnErr
 }
@@ -302,6 +308,10 @@ func (stream *sshSmartSubtransportStream) Free() {
 }
 
 func cacheKeyAndConfig(remoteAddress string, cred *git2go.Credential) (string, *ssh.ClientConfig, error) {
+	if cred == nil {
+		return "", nil, fmt.Errorf("cannot create cache key from a nil credential")
+	}
+
 	username, _, privatekey, passphrase, err := cred.GetSSHKey()
 	if err != nil {
 		return "", nil, err