Merge branch 'detect-shallow-clone' into 'master'

Suppress upload-pack Wait() error on shallow clone

Closes https://gitlab.com/gitlab-org/gitlab-workhorse/issues/36

It is a [known
bug](https://gitlab.com/gitlab-org/gitlab-workhorse/issues/36#note_4761298)
in git-upload-pack that it exits unsuccessfully when the client does a
'shallow clone'. We don't want this non-zero process exit status to
clutter our logs. In this change we buffer the first 4kB of the request body for
git upload-pack and scan for a 'deepen' message (which indicates the
clients wants to do a shallow clone). If deepen is found we suppress
error reporting for a non-zero exit status of git upload-pack.


See merge request !71

internal/git/git-http.go

@@ -5,11 +5,13 @@ In this file we handle the Git 'smart HTTP' protocol
 package git
 
 import (
+	"bytes"
 	"fmt"
 	"io"
 	"log"
 	"net/http"
 	"os"
+	"os/exec"
 	"path"
 	"path/filepath"
 	"strings"
@@ -101,6 +103,8 @@ func handleGetInfoRefs(w http.ResponseWriter, r *http.Request, a *api.Response)
 
 func handlePostRPC(w http.ResponseWriter, r *http.Request, a *api.Response) {
 	var err error
+	var body io.Reader
+	var isShallowClone bool
 
 	// Get Git action from URL
 	action := filepath.Base(r.URL.Path)
@@ -110,6 +114,29 @@ func handlePostRPC(w http.ResponseWriter, r *http.Request, a *api.Response) {
 		return
 	}
 
+	if action == "git-upload-pack" {
+		buffer := &bytes.Buffer{}
+		// Only sniff on the first 4096 bytes: we assume that if we find no
+		// 'deepen' message in the first 4096 bytes there won't be one later
+		// either.
+		_, err = io.Copy(buffer, io.LimitReader(r.Body, 4096))
+		if err != nil {
+			helper.Fail500(w, r, &copyError{fmt.Errorf("handlePostRPC: buffer git-upload-pack body: %v", err)})
+			return
+		}
+
+		isShallowClone, err = scanDeepen(bytes.NewReader(buffer.Bytes()))
+		body = io.MultiReader(buffer, r.Body)
+		if err != nil {
+			// Do not pass on the error: our failure to parse the
+			// request body should not abort the request.
+			helper.LogError(r, fmt.Errorf("parseBody (non-fatal): %v", err))
+		}
+
+	} else {
+		body = r.Body
+	}
+
 	// Prepare our Git subprocess
 	cmd := gitCommand(a.GL_ID, "git", subCommand(action), "--stateless-rpc", a.RepoPath)
 	stdout, err := cmd.StdoutPipe()
@@ -131,7 +158,7 @@ func handlePostRPC(w http.ResponseWriter, r *http.Request, a *api.Response) {
 	defer helper.CleanUpProcessGroup(cmd) // Ensure brute force subprocess clean-up
 
 	// Write the client request body to Git's standard input
-	if _, err := io.Copy(stdin, r.Body); err != nil {
+	if _, err := io.Copy(stdin, body); err != nil {
 		helper.Fail500(w, r, fmt.Errorf("handlePostRPC: write to %v: %v", cmd.Args, err))
 		return
 	}
@@ -155,22 +182,17 @@ func handlePostRPC(w http.ResponseWriter, r *http.Request, a *api.Response) {
 		)
 		return
 	}
-	if err := cmd.Wait(); err != nil {
+	if err := cmd.Wait(); err != nil && !(isExitError(err) && isShallowClone) {
 		helper.LogError(r, fmt.Errorf("handlePostRPC: wait for %v: %v", cmd.Args, err))
 		return
 	}
 }
 
-func subCommand(rpc string) string {
-	return strings.TrimPrefix(rpc, "git-")
+func isExitError(err error) bool {
+	_, ok := err.(*exec.ExitError)
+	return ok
 }
 
-func pktLine(w io.Writer, s string) error {
-	_, err := fmt.Fprintf(w, "%04x%s", len(s)+4, s)
-	return err
-}
-
-func pktFlush(w io.Writer) error {
-	_, err := fmt.Fprint(w, "0000")
-	return err
+func subCommand(rpc string) string {
+	return strings.TrimPrefix(rpc, "git-")
 }

internal/git/pktline.go

+package git
+
+import (
+	"bufio"
+	"bytes"
+	"fmt"
+	"io"
+	"strconv"
+)
+
+func pktLine(w io.Writer, s string) error {
+	_, err := fmt.Fprintf(w, "%04x%s", len(s)+4, s)
+	return err
+}
+
+func pktFlush(w io.Writer) error {
+	_, err := fmt.Fprint(w, "0000")
+	return err
+}
+
+func scanDeepen(body io.Reader) (bool, error) {
+	hasDeepen := false
+
+	scanner := bufio.NewScanner(body)
+	scanner.Split(pktLineSplitter)
+	for scanner.Scan() {
+		if bytes.HasPrefix(scanner.Bytes(), []byte("deepen")) {
+			hasDeepen = true
+			break
+		}
+	}
+
+	return hasDeepen, scanner.Err()
+}
+
+func pktLineSplitter(data []byte, atEOF bool) (advance int, token []byte, err error) {
+	if len(data) < 4 {
+		if atEOF && len(data) > 0 {
+			return 0, nil, fmt.Errorf("pktLineSplitter: incomplete length prefix on %q", data)
+		}
+		return 0, nil, nil // want more data
+	}
+
+	if bytes.HasPrefix(data, []byte("0000")) {
+		// special case: "0000" terminator packet: return empty token
+		return 4, data[:0], nil
+	}
+
+	// We have at least 4 bytes available so we can decode the 4-hex digit
+	// length prefix of the packet line.
+	pktLength64, err := strconv.ParseInt(string(data[:4]), 16, 0)
+	if err != nil {
+		return 0, nil, fmt.Errorf("pktLineSplitter: decode length: %v", err)
+	}
+
+	// Cast is safe because we requested an int-size number from strconv.ParseInt
+	pktLength := int(pktLength64)
+
+	if pktLength < 0 {
+		return 0, nil, fmt.Errorf("pktLineSplitter: invalid length: %d", pktLength)
+	}
+
+	if len(data) < pktLength {
+		if atEOF {
+			return 0, nil, fmt.Errorf("pktLineSplitter: less than %d bytes in input %q", pktLength, data)
+		}
+		return 0, nil, nil // want more data
+	}
+
+	// return "pkt" token without length prefix
+	return pktLength, data[4:pktLength], nil
+}

internal/git/pktline_test.go

+package git
+
+import (
+	"bytes"
+	"testing"
+)
+
+func TestSuccessfulScanDeepen(t *testing.T) {
+	examples := []struct {
+		input  string
+		output bool
+	}{
+		{"000dsomething000cdeepen 10000", true},
+		{"000dsomething0000000cdeepen 1", true},
+		{"000dsomething0000", false},
+	}
+
+	for _, example := range examples {
+		hasDeepen, err := scanDeepen(bytes.NewReader([]byte(example.input)))
+		if err != nil {
+			t.Fatalf("error scanning %q: %v", example.input, err)
+		}
+
+		if hasDeepen != example.output {
+			t.Fatalf("scanDeepen %q: expected %v, got %v", example.input, example.output, hasDeepen)
+		}
+	}
+}
+
+func TestFailedScanDeepen(t *testing.T) {
+	examples := []string{
+		"invalid data",
+		"deepen",
+		"000cdeepen",
+	}
+
+	for _, example := range examples {
+		hasDeepen, err := scanDeepen(bytes.NewReader([]byte(example)))
+		if err == nil {
+			t.Fatalf("expected error scanning %q", example)
+		}
+
+		t.Log(err)
+
+		if hasDeepen == true {
+			t.Fatalf("scanDeepen %q: expected result to be false, got true", example)
+		}
+	}
+}