Merge remote-tracking branch 'origin/master' into refactor-upstream

README.md

 # gitlab-workhorse
 
-gitlab-workhorse was designed to unload Git HTTP traffic from
-the GitLab Rails app (Unicorn) to a separate daemon.  It also serves
-'git archive' downloads for GitLab.  All authentication and
-authorization logic is still handled by the GitLab Rails app.
+Gitlab-workhorse is a smart reverse proxy for GitLab. It handles
+"large" HTTP requests such as file downloads, file uploads, Git
+push/pull and Git archive downloads.
 
-Architecture: Git client -> NGINX -> gitlab-workhorse (makes
-auth request to GitLab Rails app) -> git-upload-pack
 
 ## Usage
 
@@ -18,6 +15,10 @@ Options:
     	Authentication/authorization backend (default "http://localhost:8080")
   -authSocket string
     	Optional: Unix domain socket to dial authBackend at
+  -developmentMode
+    	Allow to serve assets from Rails app
+  -documentRoot string
+    	Path to static files content (default "public")
   -listenAddr string
     	Listen address for HTTP server (default "localhost:8181")
   -listenNetwork string
@@ -26,19 +27,17 @@ Options:
     	Umask for Unix socket, default: 022 (default 18)
   -pprofListenAddr string
     	pprof listening address, e.g. 'localhost:6060'
+  -proxyHeadersTimeout duration
+    	How long to wait for response headers when proxying the request (default 1m0s)
   -version
     	Print version and exit
 ```
 
-gitlab-workhorse allows Git HTTP clients to push and pull to
-and from Git repositories. Each incoming request is first replayed
-(with an empty request body) to an external authentication/authorization
-HTTP server: the 'auth backend'. The auth backend is expected to
-be a GitLab Unicorn process.  The 'auth response' is a JSON message
-which tells gitlab-workhorse the path of the Git repository
-to read from/write to.
+The 'auth backend' refers to the GitLab Rails applicatoin. The name is
+a holdover from when gitlab-workhorse only handled Git push/pull over
+HTTP.
 
-gitlab-workhorse can listen on either a TCP or a Unix domain socket. It
+Gitlab-workhorse can listen on either a TCP or a Unix domain socket. It
 can also open a second listening TCP listening socket with the Go
 [net/http/pprof profiler server](http://golang.org/pkg/net/http/pprof/).
 

internal/git/archive.go

@@ -16,6 +16,7 @@ import (
 	"os/exec"
 	"path"
 	"path/filepath"
+	"syscall"
 	"time"
 )
 
@@ -84,6 +85,7 @@ func handleGetArchive(w http.ResponseWriter, r *http.Request, a *api.Response) {
 		stdout = archiveStdout
 	} else {
 		compressCmd.Stdin = archiveStdout
+		compressCmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
 
 		stdout, err = compressCmd.StdoutPipe()
 		if err != nil {
@@ -96,7 +98,7 @@ func handleGetArchive(w http.ResponseWriter, r *http.Request, a *api.Response) {
 			helper.Fail500(w, fmt.Errorf("handleGetArchive: start %v: %v", compressCmd.Args, err))
 			return
 		}
-		defer compressCmd.Wait()
+		defer cleanUpProcessGroup(compressCmd)
 
 		archiveStdout.Close()
 	}

internal/staticpages/error_pages.go

@@ -60,7 +60,10 @@ func (s *errorPageResponseWriter) Flush() {
 	s.WriteHeader(http.StatusOK)
 }
 
-func (st *Static) ErrorPages(handler http.Handler) http.Handler {
+func (st *Static) ErrorPages(enabled bool, handler http.Handler) http.Handler {
+	if !enabled {
+		return handler
+	}
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		rw := errorPageResponseWriter{
 			rw:   w,

internal/staticpages/error_pages_test.go

@@ -27,7 +27,7 @@ func TestIfErrorPageIsPresented(t *testing.T) {
 		fmt.Fprint(w, "Not Found")
 	})
 	st := &Static{dir}
-	st.ErrorPages(h).ServeHTTP(w, nil)
+	st.ErrorPages(true, h).ServeHTTP(w, nil)
 	w.Flush()
 
 	helper.AssertResponseCode(t, w, 404)
@@ -48,9 +48,32 @@ func TestIfErrorPassedIfNoErrorPageIsFound(t *testing.T) {
 		fmt.Fprint(w, errorResponse)
 	})
 	st := &Static{dir}
-	st.ErrorPages(h).ServeHTTP(w, nil)
+	st.ErrorPages(true, h).ServeHTTP(w, nil)
 	w.Flush()
 
 	helper.AssertResponseCode(t, w, 404)
 	helper.AssertResponseBody(t, w, errorResponse)
 }
+
+func TestIfErrorPageIsIgnoredInDevelopment(t *testing.T) {
+	dir, err := ioutil.TempDir("", "error_page")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(dir)
+
+	errorPage := "ERROR"
+	ioutil.WriteFile(filepath.Join(dir, "500.html"), []byte(errorPage), 0600)
+
+	w := httptest.NewRecorder()
+	serverError := "Interesting Server Error"
+	h := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(500)
+		fmt.Fprint(w, serverError)
+	})
+	st := &Static{dir}
+	st.ErrorPages(false, h).ServeHTTP(w, nil)
+	w.Flush()
+	helper.AssertResponseCode(t, w, 500)
+	helper.AssertResponseBody(t, w, serverError)
+}

internal/upstream/routes.go

@@ -88,7 +88,7 @@ func (u *Upstream) configureRoutes() {
 		route{"", nil,
 			static.ServeExisting(u.URLPrefix(), staticpages.CacheDisabled,
 				static.DeployPage(
-					static.ErrorPages(
+					static.ErrorPages(u.DevelopmentMode,
 						proxy,
 					),
 				),

internal/upstream/upstream.go

@@ -81,7 +81,7 @@ func (u *Upstream) ServeHTTP(ow http.ResponseWriter, r *http.Request) {
 	}
 
 	// Check URL Root
-	URIPath := urlprefix.CleanURIPath(r.URL.Path)
+	URIPath := urlprefix.CleanURIPath(r.URL.EscapedPath())
 	prefix := u.URLPrefix()
 	if !prefix.Match(URIPath) {
 		httpError(&w, r, fmt.Sprintf("Not found %q", URIPath), http.StatusNotFound)

main_test.go

@@ -15,6 +15,7 @@ import (
 	"os/exec"
 	"path"
 	"regexp"
+	"strings"
 	"testing"
 	"time"
 )
@@ -199,6 +200,29 @@ func TestAllowedApiDownloadZip(t *testing.T) {
 	runOrFail(t, extractCmd)
 }
 
+func TestAllowedApiDownloadZipWithSlash(t *testing.T) {
+	prepareDownloadDir(t)
+
+	// Prepare test server and backend
+	archiveName := "foobar.zip"
+	ts := testAuthServer(nil, 200, archiveOkBody(t, archiveName))
+	defer ts.Close()
+	ws := startWorkhorseServer(ts.URL)
+	defer ws.Close()
+
+	// Use foo%2Fbar instead of a numeric ID
+	downloadCmd := exec.Command("curl", "-J", "-O", fmt.Sprintf("%s/api/v3/projects/foo%%2Fbar/repository/archive.zip", ws.URL))
+	if !strings.Contains(downloadCmd.Args[3], `projects/foo%2Fbar/repository`) {
+		t.Fatalf("Cannot find percent-2F: %v", downloadCmd.Args)
+	}
+	downloadCmd.Dir = scratchDir
+	runOrFail(t, downloadCmd)
+
+	extractCmd := exec.Command("unzip", archiveName)
+	extractCmd.Dir = scratchDir
+	runOrFail(t, extractCmd)
+}
+
 func TestDownloadCacheHit(t *testing.T) {
 	prepareDownloadDir(t)