Merge branch 'archive-header' into 'master'

Handle "git archive" via headers

Companion to https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/2675

See merge request !36

Makefile

@@ -28,6 +28,9 @@ coverage: testdata/data/group/test.git
 	go tool cover -html=test.coverage -o coverage.html
 	rm -f test.coverage
 
+fmt:
+	go fmt ./...
+
 testdata/data/group/test.git: testdata/data
 	git clone --bare https://gitlab.com/gitlab-org/gitlab-test.git $@
 

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,8 +5,8 @@ In this file we handle 'git archive' downloads
 package git
 
 import (
-	"../api"
 	"../helper"
+	"../senddata"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -20,11 +20,23 @@ import (
 	"time"
 )
 
-func GetArchive(a *api.API) http.Handler {
-	return repoPreAuthorizeHandler(a, handleGetArchive)
+type archive struct{ senddata.Prefix }
+type archiveParams struct {
+	RepoPath      string
+	ArchivePath   string
+	ArchivePrefix string
+	CommitId      string
 }
 
-func handleGetArchive(w http.ResponseWriter, r *http.Request, a *api.Response) {
+var SendArchive = &archive{"git-archive:"}
+
+func (a *archive) Inject(w http.ResponseWriter, r *http.Request, sendData string) {
+	var params archiveParams
+	if err := a.Unpack(&params, sendData); err != nil {
+		helper.Fail500(w, fmt.Errorf("SendArchive: unpack sendData: %v", err))
+		return
+	}
+
 	var format string
 	urlPath := r.URL.Path
 	switch filepath.Base(urlPath) {
@@ -41,11 +53,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 +70,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 +80,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 +137,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

@@ -2,28 +2,25 @@ package git
 
 import (
 	"../helper"
-	"encoding/base64"
-	"encoding/json"
+	"../senddata"
 	"fmt"
 	"io"
 	"log"
 	"net/http"
-	"strings"
 )
 
-type blobParams struct {
-	RepoPath string
-	BlobId   string
-}
+type blob struct{ senddata.Prefix }
+type blobParams struct{ RepoPath, BlobId string }
 
-const SendBlobPrefix = "git-blob:"
+var SendBlob = &blob{"git-blob:"}
 
-func SendBlob(w http.ResponseWriter, r *http.Request, sendData string) {
-	params, err := unpackSendData(sendData)
-	if err != nil {
+func (b *blob) Inject(w http.ResponseWriter, r *http.Request, sendData string) {
+	var params blobParams
+	if err := b.Unpack(&params, sendData); 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)
@@ -49,15 +46,3 @@ func SendBlob(w http.ResponseWriter, r *http.Request, sendData string) {
 		return
 	}
 }
-
-func unpackSendData(sendData string) (*blobParams, error) {
-	jsonBytes, err := base64.URLEncoding.DecodeString(strings.TrimPrefix(sendData, SendBlobPrefix))
-	if err != nil {
-		return nil, err
-	}
-	result := &blobParams{}
-	if err := json.Unmarshal([]byte(jsonBytes), result); err != nil {
-		return nil, err
-	}
-	return result, nil
-}

internal/lfs/lfs.go

@@ -7,7 +7,6 @@ package lfs
 import (
 	"../api"
 	"../helper"
-	"../proxy"
 	"bytes"
 	"crypto/sha256"
 	"encoding/hex"
@@ -20,8 +19,8 @@ import (
 	"path/filepath"
 )
 
-func PutStore(a *api.API, p *proxy.Proxy) http.Handler {
-	return lfsAuthorizeHandler(a, handleStoreLfsObject(p))
+func PutStore(a *api.API, h http.Handler) http.Handler {
+	return lfsAuthorizeHandler(a, handleStoreLfsObject(h))
 }
 
 func lfsAuthorizeHandler(myAPI *api.API, handleFunc api.HandleFunc) http.Handler {

internal/proxy/proxy.go

@@ -3,7 +3,6 @@ package proxy
 import (
 	"../badgateway"
 	"../helper"
-	"../senddata"
 	"net/http"
 	"net/http/httputil"
 	"net/url"
@@ -34,8 +33,6 @@ func (p *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 
 	// Set Workhorse version
 	req.Header.Set("Gitlab-Workhorse", p.Version)
-	rw := senddata.NewSendFileResponseWriter(w, &req)
-	defer rw.Flush()
 
-	p.reverseProxy.ServeHTTP(&rw, &req)
+	p.reverseProxy.ServeHTTP(w, &req)
 }

internal/senddata/injecter.go

+package senddata
+
+import (
+	"encoding/base64"
+	"encoding/json"
+	"net/http"
+	"strings"
+)
+
+type Injecter interface {
+	Match(string) bool
+	Inject(http.ResponseWriter, *http.Request, string)
+}
+
+type Prefix string
+
+const HeaderKey = "Gitlab-Workhorse-Send-Data"
+
+func (p Prefix) Match(s string) bool {
+	return strings.HasPrefix(s, string(p))
+}
+
+func (p Prefix) Unpack(result interface{}, sendData string) error {
+	jsonBytes, err := base64.URLEncoding.DecodeString(strings.TrimPrefix(sendData, string(p)))
+	if err != nil {
+		return err
+	}
+	if err := json.Unmarshal([]byte(jsonBytes), result); err != nil {
+		return err
+	}
+	return nil
+}

internal/senddata/senddata.go

+package senddata
+
+import (
+	"net/http"
+)
+
+type sendDataResponseWriter struct {
+	rw        http.ResponseWriter
+	status    int
+	hijacked  bool
+	req       *http.Request
+	injecters []Injecter
+}
+
+func SendData(h http.Handler, injecters ...Injecter) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		s := sendDataResponseWriter{
+			rw:        w,
+			req:       r,
+			injecters: injecters,
+		}
+		defer s.Flush()
+		h.ServeHTTP(&s, r)
+	})
+}
+
+func (s *sendDataResponseWriter) Header() http.Header {
+	return s.rw.Header()
+}
+
+func (s *sendDataResponseWriter) Write(data []byte) (n int, err error) {
+	if s.status == 0 {
+		s.WriteHeader(http.StatusOK)
+	}
+	if s.hijacked {
+		return
+	}
+	return s.rw.Write(data)
+}
+
+func (s *sendDataResponseWriter) WriteHeader(status int) {
+	if s.status != 0 {
+		return
+	}
+	s.status = status
+
+	if s.status == http.StatusOK && s.tryInject() {
+		return
+	}
+
+	s.rw.WriteHeader(s.status)
+}
+
+func (s *sendDataResponseWriter) tryInject() bool {
+	header := s.Header().Get(HeaderKey)
+	s.Header().Del(HeaderKey)
+	if header == "" {
+		return false
+	}
+
+	for _, injecter := range s.injecters {
+		if injecter.Match(header) {
+			s.hijacked = true
+			injecter.Inject(s.rw, s.req, header)
+			return true
+		}
+	}
+
+	return false
+}
+
+func (s *sendDataResponseWriter) Flush() {
+	s.WriteHeader(http.StatusOK)
+}

internal/senddata/sendfile.go → internal/sendfile/sendfile.go

@@ -4,20 +4,15 @@ via the X-Sendfile mechanism. All that is needed in the Rails code is the
 'send_file' method.
 */
 
-package senddata
+package sendfile
 
 import (
-	"../git"
 	"../helper"
 	"log"
 	"net/http"
-	"strings"
 )
 
-const (
-	sendDataResponseHeader = "Gitlab-Workhorse-Send-Data"
-	sendFileResponseHeader = "X-Sendfile"
-)
+const sendFileResponseHeader = "X-Sendfile"
 
 type sendFileResponseWriter struct {
 	rw       http.ResponseWriter
@@ -26,14 +21,17 @@ type sendFileResponseWriter struct {
 	req      *http.Request
 }
 
-func NewSendFileResponseWriter(rw http.ResponseWriter, req *http.Request) sendFileResponseWriter {
-	s := sendFileResponseWriter{
+func SendFile(h http.Handler) http.Handler {
+	return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
+		s := &sendFileResponseWriter{
 			rw:  rw,
 			req: req,
 		}
 		// Advertise to upstream (Rails) that we support X-Sendfile
 		req.Header.Set("X-Sendfile-Type", "X-Sendfile")
-	return s
+		defer s.Flush()
+		h.ServeHTTP(s, req)
+	})
 }
 
 func (s *sendFileResponseWriter) Header() http.Header {
@@ -70,12 +68,6 @@ 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
-	}
 
 	s.rw.WriteHeader(s.status)
 	return

internal/upstream/routes.go

@@ -6,6 +6,8 @@ import (
 	"../git"
 	"../lfs"
 	proxypkg "../proxy"
+	"../senddata"
+	"../sendfile"
 	"../staticpages"
 	"net/http"
 	"regexp"
@@ -37,10 +39,14 @@ func (u *Upstream) configureRoutes() {
 		u.RoundTripper,
 	)
 	static := &staticpages.Static{u.DocumentRoot}
-	proxy := proxypkg.NewProxy(
+	proxy := senddata.SendData(
+		sendfile.SendFile(proxypkg.NewProxy(
 			u.Backend,
 			u.Version,
 			u.RoundTripper,
+		)),
+		git.SendArchive,
+		git.SendBlob,
 	)
 
 	u.Routes = []route{
@@ -50,20 +56,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()
@@ -657,6 +657,29 @@ 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)
+	})
+}
+
 func startWorkhorseServer(authBackend string) *httptest.Server {
 	u := upstream.NewUpstream(
 		helper.URLMustParse(authBackend),
@@ -684,21 +707,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 {