First version of "git archive" via headers

internal/api/api.go

@@ -38,15 +38,6 @@ type Response struct {
 	// RepoPath is the full path on disk to the Git repository the request is
 	// about
 	RepoPath string
-	// ArchivePath is the full path where we should find/create a cached copy
-	// of a requested archive
-	ArchivePath string
-	// ArchivePrefix is used to put extracted archive contents in a
-	// subdirectory
-	ArchivePrefix string
-	// CommitId is used do prevent race conditions between the 'time of check'
-	// in the GitLab Rails app and the 'time of use' in gitlab-workhorse.
-	CommitId string
 	// StoreLFSPath is provided by the GitLab Rails application
 	// to mark where the tmp file should be placed
 	StoreLFSPath string

internal/git/archive.go

@@ -5,7 +5,6 @@ In this file we handle 'git archive' downloads
 package git
 
 import (
-	"../api"
 	"../helper"
 	"fmt"
 	"io"
@@ -20,11 +19,20 @@ import (
 	"time"
 )
 
-func GetArchive(a *api.API) http.Handler {
-	return repoPreAuthorizeHandler(a, handleGetArchive)
-}
+const SendArchivePrefix = "git-archive:"
+
+func SendArchive(w http.ResponseWriter, r *http.Request, sendData string) {
+	var params struct {
+		RepoPath      string
+		ArchivePath   string
+		ArchivePrefix string
+		CommitId      string
+	}
+	if err := unpackSendData(&params, sendData, SendArchivePrefix); err != nil {
+		helper.Fail500(w, fmt.Errorf("SendArchive: unpack sendData: %v", err))
+		return
+	}
 
-func handleGetArchive(w http.ResponseWriter, r *http.Request, a *api.Response) {
 	var format string
 	urlPath := r.URL.Path
 	switch filepath.Base(urlPath) {
@@ -41,11 +49,11 @@ func handleGetArchive(w http.ResponseWriter, r *http.Request, a *api.Response) {
 		return
 	}
 
-	archiveFilename := path.Base(a.ArchivePath)
+	archiveFilename := path.Base(params.ArchivePath)
 
-	if cachedArchive, err := os.Open(a.ArchivePath); err == nil {
+	if cachedArchive, err := os.Open(params.ArchivePath); err == nil {
 		defer cachedArchive.Close()
-		log.Printf("Serving cached file %q", a.ArchivePath)
+		log.Printf("Serving cached file %q", params.ArchivePath)
 		setArchiveHeaders(w, format, archiveFilename)
 		// Even if somebody deleted the cachedArchive from disk since we opened
 		// the file, Unix file semantics guarantee we can still read from the
@@ -58,7 +66,7 @@ func handleGetArchive(w http.ResponseWriter, r *http.Request, a *api.Response) {
 	// safe. We create the tempfile in the same directory as the final cached
 	// archive we want to create so that we can use an atomic link(2) operation
 	// to finalize the cached archive.
-	tempFile, err := prepareArchiveTempfile(path.Dir(a.ArchivePath), archiveFilename)
+	tempFile, err := prepareArchiveTempfile(path.Dir(params.ArchivePath), archiveFilename)
 	if err != nil {
 		helper.Fail500(w, fmt.Errorf("handleGetArchive: create tempfile: %v", err))
 		return
@@ -68,7 +76,7 @@ func handleGetArchive(w http.ResponseWriter, r *http.Request, a *api.Response) {
 
 	compressCmd, archiveFormat := parseArchiveFormat(format)
 
-	archiveCmd := gitCommand("", "git", "--git-dir="+a.RepoPath, "archive", "--format="+archiveFormat, "--prefix="+a.ArchivePrefix+"/", a.CommitId)
+	archiveCmd := gitCommand("", "git", "--git-dir="+params.RepoPath, "archive", "--format="+archiveFormat, "--prefix="+params.ArchivePrefix+"/", params.CommitId)
 	archiveStdout, err := archiveCmd.StdoutPipe()
 	if err != nil {
 		helper.Fail500(w, fmt.Errorf("handleGetArchive: archive stdout: %v", err))
@@ -125,13 +133,14 @@ func handleGetArchive(w http.ResponseWriter, r *http.Request, a *api.Response) {
 		}
 	}
 
-	if err := finalizeCachedArchive(tempFile, a.ArchivePath); err != nil {
+	if err := finalizeCachedArchive(tempFile, params.ArchivePath); err != nil {
 		helper.LogError(fmt.Errorf("handleGetArchive: finalize cached archive: %v", err))
 		return
 	}
 }
 
 func setArchiveHeaders(w http.ResponseWriter, format string, archiveFilename string) {
+	w.Header().Del("Content-Length")
 	w.Header().Add("Content-Disposition", fmt.Sprintf(`attachment; filename="%s"`, archiveFilename))
 	if format == "zip" {
 		w.Header().Add("Content-Type", "application/zip")

internal/git/blob.go

@@ -11,19 +11,15 @@ import (
 	"strings"
 )
 
-type blobParams struct {
-	RepoPath string
-	BlobId   string
-}
-
 const SendBlobPrefix = "git-blob:"
 
 func SendBlob(w http.ResponseWriter, r *http.Request, sendData string) {
-	params, err := unpackSendData(sendData)
-	if err != nil {
+	var params struct{ RepoPath, BlobId string }
+	if err := unpackSendData(&params, sendData, SendBlobPrefix); err != nil {
 		helper.Fail500(w, fmt.Errorf("SendBlob: unpack sendData: %v", err))
 		return
 	}
+
 	log.Printf("SendBlob: sending %q for %q", params.BlobId, r.URL.Path)
 
 	gitShowCmd := gitCommand("", "git", "--git-dir="+params.RepoPath, "cat-file", "blob", params.BlobId)
@@ -48,14 +44,13 @@ func SendBlob(w http.ResponseWriter, r *http.Request, sendData string) {
 	}
 }
 
-func unpackSendData(sendData string) (*blobParams, error) {
-	jsonBytes, err := base64.URLEncoding.DecodeString(strings.TrimPrefix(sendData, SendBlobPrefix))
+func unpackSendData(result interface{}, sendData string, prefix string) error {
+	jsonBytes, err := base64.URLEncoding.DecodeString(strings.TrimPrefix(sendData, prefix))
 	if err != nil {
-		return nil, err
+		return err
 	}
-	result := &blobParams{}
 	if err := json.Unmarshal([]byte(jsonBytes), result); err != nil {
-		return nil, err
+		return err
 	}
-	return result, nil
+	return nil
 }

internal/senddata/sendfile.go

@@ -70,11 +70,21 @@ func (s *sendFileResponseWriter) WriteHeader(status int) {
 		sendFileFromDisk(s.rw, s.req, file)
 		return
 	}
-	if sendData := s.Header().Get(sendDataResponseHeader); strings.HasPrefix(sendData, git.SendBlobPrefix) {
-		s.Header().Del(sendDataResponseHeader)
-		s.hijacked = true
-		git.SendBlob(s.rw, s.req, sendData)
-		return
+
+	sendData := s.Header().Get(sendDataResponseHeader)
+	s.Header().Del(sendDataResponseHeader)
+	for _, handler := range []struct {
+		prefix string
+		f      func(http.ResponseWriter, *http.Request, string)
+	}{
+		{git.SendBlobPrefix, git.SendBlob},
+		{git.SendArchivePrefix, git.SendArchive},
+	} {
+		if strings.HasPrefix(sendData, handler.prefix) {
+			s.hijacked = true
+			handler.f(s.rw, s.req, sendData)
+			return
+		}
 	}
 
 	s.rw.WriteHeader(s.status)

internal/upstream/routes.go

@@ -50,20 +50,6 @@ func (u *Upstream) configureRoutes() {
 		route{"POST", regexp.MustCompile(gitProjectPattern + `git-receive-pack\z`), contentEncodingHandler(git.PostRPC(api))},
 		route{"PUT", regexp.MustCompile(gitProjectPattern + `gitlab-lfs/objects/([0-9a-f]{64})/([0-9]+)\z`), lfs.PutStore(api, proxy)},
 
-		// Repository Archive
-		route{"GET", regexp.MustCompile(projectPattern + `repository/archive\z`), git.GetArchive(api)},
-		route{"GET", regexp.MustCompile(projectPattern + `repository/archive.zip\z`), git.GetArchive(api)},
-		route{"GET", regexp.MustCompile(projectPattern + `repository/archive.tar\z`), git.GetArchive(api)},
-		route{"GET", regexp.MustCompile(projectPattern + `repository/archive.tar.gz\z`), git.GetArchive(api)},
-		route{"GET", regexp.MustCompile(projectPattern + `repository/archive.tar.bz2\z`), git.GetArchive(api)},
-
-		// Repository Archive API
-		route{"GET", regexp.MustCompile(projectsAPIPattern + `repository/archive\z`), git.GetArchive(api)},
-		route{"GET", regexp.MustCompile(projectsAPIPattern + `repository/archive.zip\z`), git.GetArchive(api)},
-		route{"GET", regexp.MustCompile(projectsAPIPattern + `repository/archive.tar\z`), git.GetArchive(api)},
-		route{"GET", regexp.MustCompile(projectsAPIPattern + `repository/archive.tar.gz\z`), git.GetArchive(api)},
-		route{"GET", regexp.MustCompile(projectsAPIPattern + `repository/archive.tar.bz2\z`), git.GetArchive(api)},
-
 		// CI Artifacts
 		route{"GET", regexp.MustCompile(projectPattern + `builds/[0-9]+/artifacts/file/`), contentEncodingHandler(artifacts.DownloadArtifact(api))},
 		route{"POST", regexp.MustCompile(ciAPIPattern + `v1/builds/[0-9]+/artifacts\z`), contentEncodingHandler(artifacts.UploadArtifacts(api, proxy))},

main_test.go

@@ -115,7 +115,7 @@ func TestAllowedDownloadZip(t *testing.T) {
 
 	// Prepare test server and backend
 	archiveName := "foobar.zip"
-	ts := testAuthServer(nil, 200, archiveOkBody(t, archiveName))
+	ts := archiveOKServer(t, archiveName)
 	defer ts.Close()
 	ws := startWorkhorseServer(ts.URL)
 	defer ws.Close()
@@ -134,7 +134,7 @@ func TestAllowedDownloadTar(t *testing.T) {
 
 	// Prepare test server and backend
 	archiveName := "foobar.tar"
-	ts := testAuthServer(nil, 200, archiveOkBody(t, archiveName))
+	ts := archiveOKServer(t, archiveName)
 	defer ts.Close()
 	ws := startWorkhorseServer(ts.URL)
 	defer ws.Close()
@@ -153,7 +153,7 @@ func TestAllowedDownloadTarGz(t *testing.T) {
 
 	// Prepare test server and backend
 	archiveName := "foobar.tar.gz"
-	ts := testAuthServer(nil, 200, archiveOkBody(t, archiveName))
+	ts := archiveOKServer(t, archiveName)
 	defer ts.Close()
 	ws := startWorkhorseServer(ts.URL)
 	defer ws.Close()
@@ -172,7 +172,7 @@ func TestAllowedDownloadTarBz2(t *testing.T) {
 
 	// Prepare test server and backend
 	archiveName := "foobar.tar.bz2"
-	ts := testAuthServer(nil, 200, archiveOkBody(t, archiveName))
+	ts := archiveOKServer(t, archiveName)
 	defer ts.Close()
 	ws := startWorkhorseServer(ts.URL)
 	defer ws.Close()
@@ -191,7 +191,7 @@ func TestAllowedApiDownloadZip(t *testing.T) {
 
 	// Prepare test server and backend
 	archiveName := "foobar.zip"
-	ts := testAuthServer(nil, 200, archiveOkBody(t, archiveName))
+	ts := archiveOKServer(t, archiveName)
 	defer ts.Close()
 	ws := startWorkhorseServer(ts.URL)
 	defer ws.Close()
@@ -210,7 +210,7 @@ func TestAllowedApiDownloadZipWithSlash(t *testing.T) {
 
 	// Prepare test server and backend
 	archiveName := "foobar.zip"
-	ts := testAuthServer(nil, 200, archiveOkBody(t, archiveName))
+	ts := archiveOKServer(t, archiveName)
 	defer ts.Close()
 	ws := startWorkhorseServer(ts.URL)
 	defer ws.Close()
@@ -233,7 +233,7 @@ func TestDownloadCacheHit(t *testing.T) {
 
 	// Prepare test server and backend
 	archiveName := "foobar.zip"
-	ts := testAuthServer(nil, 200, archiveOkBody(t, archiveName))
+	ts := archiveOKServer(t, archiveName)
 	defer ts.Close()
 	ws := startWorkhorseServer(ts.URL)
 	defer ws.Close()
@@ -264,7 +264,7 @@ func TestDownloadCacheCreate(t *testing.T) {
 
 	// Prepare test server and backend
 	archiveName := "foobar.zip"
-	ts := testAuthServer(nil, 200, archiveOkBody(t, archiveName))
+	ts := archiveOKServer(t, archiveName)
 	defer ts.Close()
 	ws := startWorkhorseServer(ts.URL)
 	defer ws.Close()
@@ -659,6 +659,31 @@ func testAuthServer(url *regexp.Regexp, code int, body interface{}) *httptest.Se
 	})
 }
 
+func archiveOKServer(t *testing.T, archiveName string) *httptest.Server {
+	return testhelper.TestServerWithHandler(regexp.MustCompile("."), func(w http.ResponseWriter, r *http.Request) {
+		cwd, err := os.Getwd()
+		if err != nil {
+			t.Fatal(err)
+		}
+		archivePath := path.Join(cwd, cacheDir, archiveName)
+
+		params := struct{ RepoPath, ArchivePath, CommitId, ArchivePrefix string }{
+			repoPath(t),
+			archivePath,
+			"c7fbe50c7c7419d9701eebe64b1fdacc3df5b9dd",
+			"foobar123",
+		}
+		jsonData, err := json.Marshal(params)
+		if err != nil {
+			t.Fatal(err)
+		}
+		encodedJSON := base64.StdEncoding.EncodeToString(jsonData)
+		w.Header().Set("Gitlab-Workhorse-Send-Data", "git-archive:"+encodedJSON)
+		// Prevent the Go HTTP server from setting the Content-Length to 0.
+		w.Header().Set("Transfer-Encoding", "chunked")
+	})
+}
+
 func startWorkhorseServer(authBackend string) *httptest.Server {
 	u := upstream.NewUpstream(
 		helper.URLMustParse(authBackend),
@@ -686,21 +711,6 @@ func gitOkBody(t *testing.T) interface{} {
 	}
 }
 
-func archiveOkBody(t *testing.T, archiveName string) interface{} {
-	cwd, err := os.Getwd()
-	if err != nil {
-		t.Fatal(err)
-	}
-	archivePath := path.Join(cwd, cacheDir, archiveName)
-
-	return &api.Response{
-		RepoPath:      repoPath(t),
-		ArchivePath:   archivePath,
-		CommitId:      "c7fbe50c7c7419d9701eebe64b1fdacc3df5b9dd",
-		ArchivePrefix: "foobar123",
-	}
-}
-
 func repoPath(t *testing.T) string {
 	cwd, err := os.Getwd()
 	if err != nil {