From 5fc76ee3a9383bdf00cbf664d9e33af2278fda55 Mon Sep 17 00:00:00 2001
From: Anirudh Oppiliappan <x@icyphox.sh>
Date: Sun, 30 Mar 2025 09:28:11 +0000
Subject: [PATCH] knotserver: git: fix upload-pack handling

Fix stdin piping to git-upload-pack. I think we were closing the pipe
too soon previously, resulting in the 'fatal: remote end hung up
unexpectedly' error. Probably went unnoticed for smaller repos.

Also fix a bunch of misc. bugs in with superfluous http header writes
and the gzip reader being shadowed.
---
 knotserver/git.go | 39 ++++++++++++++++++++++-----------------
 knotserver/git/service/service.go | 55 +++++++++++++++++++++++++++++++------------------------
 2 file(s) changed, 53 insertion(s)(+), 41 deletion(s)(-)

diff --git a/knotserver/git.go b/knotserver/git.go
--- a/knotserver/git.go
+++ b/knotserver/git.go
@@ -34,34 +34,39 @@
 func (d *Handle) UploadPack(w http.ResponseWriter, r *http.Request) {
 	did := chi.URLParam(r, "did")
 	name := chi.URLParam(r, "name")
-	repo, _ := securejoin.SecureJoin(d.c.Repo.ScanPath, filepath.Join(did, name))
-
-	w.Header().Set("content-type", "application/x-git-upload-pack-result")
-	w.Header().Set("Connection", "Keep-Alive")
-	w.Header().Set("Transfer-Encoding", "chunked")
-	w.WriteHeader(http.StatusOK)
-
-	cmd := service.ServiceCommand{
-		Dir:    repo,
-		Stdout: w,
+	repo, err := securejoin.SecureJoin(d.c.Repo.ScanPath, filepath.Join(did, name))
+	if err != nil {
+		writeError(w, err.Error(), 500)
+		d.l.Error("git: failed to secure join repo path", "handler", "UploadPack", "error", err)
+		return
 	}
 
-	var reader io.ReadCloser
-	reader = r.Body
-
+	var bodyReader io.ReadCloser = r.Body
 	if r.Header.Get("Content-Encoding") == "gzip" {
-		reader, err := gzip.NewReader(r.Body)
+		gzipReader, err := gzip.NewReader(r.Body)
 		if err != nil {
 			writeError(w, err.Error(), 500)
 			d.l.Error("git: failed to create gzip reader", "handler", "UploadPack", "error", err)
 			return
 		}
-		defer reader.Close()
+		defer gzipReader.Close()
+		bodyReader = gzipReader
+	}
+
+	w.Header().Set("Content-Type", "application/x-git-upload-pack-result")
+	w.Header().Set("Connection", "Keep-Alive")
+
+	d.l.Info("git: executing git-upload-pack", "handler", "UploadPack", "repo", repo)
+
+	cmd := service.ServiceCommand{
+		Dir:    repo,
+		Stdout: w,
+		Stdin:  bodyReader,
 	}
 
-	cmd.Stdin = reader
+	w.WriteHeader(http.StatusOK)
+
 	if err := cmd.UploadPack(); err != nil {
-		writeError(w, err.Error(), 500)
 		d.l.Error("git: failed to execute git-upload-pack", "handler", "UploadPack", "error", err)
 		return
 	}
diff --git a/knotserver/git/service/service.go b/knotserver/git/service/service.go
--- a/knotserver/git/service/service.go
+++ b/knotserver/git/service/service.go
@@ -8,6 +8,7 @@ "log"
 	"net/http"
 	"os/exec"
 	"strings"
+	"sync"
 	"syscall"
 )
 
@@ -68,43 +69,49 @@ return nil
 }
 
 func (c *ServiceCommand) UploadPack() error {
-	cmd := exec.Command("git", []string{
-		"-c", "uploadpack.allowFilter=true",
-		"upload-pack",
-		"--stateless-rpc",
-		".",
-	}...)
+	var stderr bytes.Buffer
+
+	cmd := exec.Command("git", "-c", "uploadpack.allowFilter=true",
+		"upload-pack", "--stateless-rpc", ".")
 	cmd.Dir = c.Dir
 	cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
 
-	stdoutPipe, _ := cmd.StdoutPipe()
-	cmd.Stderr = cmd.Stdout
-	defer stdoutPipe.Close()
+	stdoutPipe, err := cmd.StdoutPipe()
+	if err != nil {
+		return fmt.Errorf("failed to create stdout pipe: %w", err)
+	}
+
+	cmd.Stderr = &stderr
 
 	stdinPipe, err := cmd.StdinPipe()
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to create stdin pipe: %w", err)
 	}
-	defer stdinPipe.Close()
 
 	if err := cmd.Start(); err != nil {
-		log.Printf("git: failed to start git-upload-pack: %s", err)
-		return err
+		return fmt.Errorf("failed to start git-upload-pack: %w", err)
 	}
 
-	if _, err := io.Copy(stdinPipe, c.Stdin); err != nil {
-		log.Printf("git: failed to copy stdin: %s", err)
-		return err
-	}
-	stdinPipe.Close()
+	var wg sync.WaitGroup
+
+	wg.Add(1)
+	go func() {
+		defer wg.Done()
+		defer stdinPipe.Close()
+		io.Copy(stdinPipe, c.Stdin)
+	}()
+
+	wg.Add(1)
+	go func() {
+		defer wg.Done()
+		io.Copy(newWriteFlusher(c.Stdout), stdoutPipe)
+		stdoutPipe.Close()
+	}()
+
+	wg.Wait()
 
-	if _, err := io.Copy(newWriteFlusher(c.Stdout), stdoutPipe); err != nil {
-		log.Printf("git: failed to copy stdout: %s", err)
-		return err
-	}
 	if err := cmd.Wait(); err != nil {
-		log.Printf("git: failed to wait for git-upload-pack: %s", err)
-		return err
+		return fmt.Errorf("git-upload-pack failed: %w, stderr: %s", err, stderr.String())
 	}
 
 	return nil

--
tangled.sh