Merge branch 'master' of https://gitlab.com/gitlab-org/gitlab-workhorse into refactor-upstream

.gitignore

 gitlab-workhorse
 testdata/data
 testdata/scratch
+testdata/public

.gitlab-ci.yml

 before_script:
-  - rm -rf /usr/local/go
-  - apt-get update -qq
-  - apt-get install -y curl unzip bzip2
+  - test -f /.dockerinit && apt-get update -qq && apt-get install -y curl unzip bzip2
   - curl -O https://storage.googleapis.com/golang/go1.5.2.linux-amd64.tar.gz
   - echo 'cae87ed095e8d94a81871281d35da7829bd1234e  go1.5.2.linux-amd64.tar.gz' | shasum -c -
-  - tar -C /usr/local -xzf go1.5.2.linux-amd64.tar.gz
+  - test -f /.dockerinit && rm -rf /usr/local/go && tar -C /usr/local -xzf go1.5.2.linux-amd64.tar.gz
   - export PATH=/usr/local/go/bin:$PATH
 
 test:

CHANGELOG

@@ -2,6 +2,17 @@
 
 Formerly known as 'gitlab-git-http-server'.
 
+0.5.3
+
+Fixes merge error in 0.5.2.
+
+0.5.2 (broken!)
+
+- Always check with upstream if files in /uploads/ may be served
+- Fix project%2Fnamespace API project ID's
+- Prevent archive zombies when using gzip or bzip2
+- Don't show pretty error pages in development mode
+
 0.5.1
 
 Deprecate -relativeURLRoot option, use -authBackend instead.

VERSION

-0.5.1
+0.5.3

internal/staticpages/servefile.go

@@ -18,6 +18,9 @@ const (
 	CacheExpireMax
 )
 
+// BUG/QUIRK: If a client requests 'foo%2Fbar' and 'foo/bar' exists,
+// handleServeFile will serve foo/bar instead of passing the request
+// upstream.
 func (s *Static) ServeExisting(prefix urlprefix.Prefix, cache CacheMode, notFoundHandler http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		file := filepath.Join(s.DocumentRoot, prefix.Strip(r.URL.Path))

internal/upstream/routes.go

@@ -21,8 +21,9 @@ const projectPattern = `^/[^/]+/[^/]+/`
 const gitProjectPattern = `^/[^/]+/[^/]+\.git/`
 
 const apiPattern = `^/api/`
-const projectsAPIPattern = `^/api/v3/projects/[^/]+/`
 
+// A project ID in an API request is either a number or two strings 'namespace/project'
+const projectsAPIPattern = `^/api/v3/projects/(\d+)|([^/]+/[^/]+)/`
 const ciAPIPattern = `^/ci/api/`
 
 // Routing table
@@ -84,6 +85,12 @@ func (u *Upstream) configureRoutes() {
 			),
 		},
 
+		// For legacy reasons, user uploads are stored under the document root.
+		// To prevent anybody who knows/guesses the URL of a user-uploaded file
+		// from downloading it we make sure requests to /uploads/ do _not_ pass
+		// through static.ServeExisting.
+		route{"", regexp.MustCompile(`^/uploads/`), static.ErrorPages(u.DevelopmentMode, proxy)},
+
 		// Serve static files or forward the requests
 		route{"", nil,
 			static.ServeExisting(u.URLPrefix(), staticpages.CacheDisabled,

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.EscapedPath())
+	URIPath := urlprefix.CleanURIPath(r.URL.Path)
 	prefix := u.URLPrefix()
 	if !prefix.Match(URIPath) {
 		httpError(&w, r, fmt.Sprintf("Not found %q", URIPath), http.StatusNotFound)

main_test.go

@@ -7,6 +7,7 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"log"
 	"net/http"
@@ -22,6 +23,7 @@ import (
 
 const scratchDir = "testdata/scratch"
 const testRepoRoot = "testdata/data"
+const testDocumentRoot = "testdata/public"
 const testRepo = "group/test.git"
 const testProject = "group/test"
 
@@ -288,6 +290,143 @@ func TestDeniedXSendfileDownload(t *testing.T) {
 	deniedXSendfileDownload(t, contentFilename, "foo/uploads/bar")
 }
 
+func TestAllowedStaticFile(t *testing.T) {
+	content := "PUBLIC"
+	if err := setupStaticFile("static file.txt", content); err != nil {
+		t.Fatalf("create public/static file.txt: %v", err)
+	}
+
+	proxied := false
+	ts := helper.TestServerWithHandler(regexp.MustCompile(`.`), func(w http.ResponseWriter, r *http.Request) {
+		proxied = true
+		w.WriteHeader(404)
+	})
+	defer ts.Close()
+	ws := startWorkhorseServer(ts.URL)
+	defer ws.Close()
+
+	for _, resource := range []string{
+		"/static%20file.txt",
+		"/static file.txt",
+	} {
+		resp, err := http.Get(ws.URL + resource)
+		if err != nil {
+			t.Fatal(err)
+		}
+		defer resp.Body.Close()
+		buf := &bytes.Buffer{}
+		if _, err := io.Copy(buf, resp.Body); err != nil {
+			t.Fatal(err)
+		}
+		if buf.String() != content {
+			t.Fatalf("GET %q: Expected %q, got %q", resource, content, buf.String())
+		}
+		if resp.StatusCode != 200 {
+			t.Fatalf("GET %q: expected 200, got %d", resource, resp.StatusCode)
+		}
+		if proxied {
+			t.Fatalf("GET %q: should not have made it to backend", resource)
+		}
+	}
+}
+
+func TestAllowedPublicUploadsFile(t *testing.T) {
+	content := "PRIVATE but allowed"
+	if err := setupStaticFile("uploads/static file.txt", content); err != nil {
+		t.Fatalf("create public/uploads/static file.txt: %v", err)
+	}
+
+	proxied := false
+	ts := helper.TestServerWithHandler(regexp.MustCompile(`.`), func(w http.ResponseWriter, r *http.Request) {
+		proxied = true
+		w.Header().Add("X-Sendfile", *documentRoot+r.URL.Path)
+		w.WriteHeader(200)
+	})
+	defer ts.Close()
+	ws := startWorkhorseServer(ts.URL)
+	defer ws.Close()
+
+	for _, resource := range []string{
+		"/uploads/static%20file.txt",
+		"/uploads/static file.txt",
+	} {
+		resp, err := http.Get(ws.URL + resource)
+		if err != nil {
+			t.Fatal(err)
+		}
+		defer resp.Body.Close()
+		buf := &bytes.Buffer{}
+		if _, err := io.Copy(buf, resp.Body); err != nil {
+			t.Fatal(err)
+		}
+		if buf.String() != content {
+			t.Fatalf("GET %q: Expected %q, got %q", resource, content, buf.String())
+		}
+		if resp.StatusCode != 200 {
+			t.Fatalf("GET %q: expected 200, got %d", resource, resp.StatusCode)
+		}
+		if !proxied {
+			t.Fatalf("GET %q: never made it to backend", resource)
+		}
+	}
+}
+
+func TestDeniedPublicUploadsFile(t *testing.T) {
+	content := "PRIVATE"
+	if err := setupStaticFile("uploads/static.txt", content); err != nil {
+		t.Fatalf("create public/uploads/static.txt: %v", err)
+	}
+
+	proxied := false
+	ts := helper.TestServerWithHandler(regexp.MustCompile(`.`), func(w http.ResponseWriter, _ *http.Request) {
+		proxied = true
+		w.WriteHeader(404)
+	})
+	defer ts.Close()
+	ws := startWorkhorseServer(ts.URL)
+	defer ws.Close()
+
+	for _, resource := range []string{
+		"/uploads/static.txt",
+		"/uploads%2Fstatic.txt",
+	} {
+		resp, err := http.Get(ws.URL + resource)
+		if err != nil {
+			t.Fatal(err)
+		}
+		defer resp.Body.Close()
+		buf := &bytes.Buffer{}
+		if _, err := io.Copy(buf, resp.Body); err != nil {
+			t.Fatal(err)
+		}
+		if buf.String() == content {
+			t.Fatalf("GET %q: Got private file contents which should have been blocked by upstream", resource)
+		}
+		if resp.StatusCode != 404 {
+			t.Fatalf("GET %q: expected 404, got %d", resource, resp.StatusCode)
+		}
+		if !proxied {
+			t.Fatalf("GET %q: never made it to backend", resource)
+		}
+	}
+}
+
+func setupStaticFile(fpath, content string) error {
+	cwd, err := os.Getwd()
+	if err != nil {
+		return err
+	}
+	*documentRoot = path.Join(cwd, testDocumentRoot)
+	if err := os.MkdirAll(path.Join(*documentRoot, path.Dir(fpath)), 0755); err != nil {
+		return err
+	}
+	static_file := path.Join(*documentRoot, fpath)
+	if err := ioutil.WriteFile(static_file, []byte(content), 0666); err != nil {
+		return err
+	}
+	return nil
+}
+
 func prepareDownloadDir(t *testing.T) {
 	if err := os.RemoveAll(scratchDir); err != nil {
 		t.Fatal(err)
@@ -335,7 +474,11 @@ func testAuthServer(url *regexp.Regexp, code int, body interface{}) *httptest.Se
 }
 
 func startWorkhorseServer(authBackend string) *httptest.Server {
-	u := &upstream.Upstream{Backend: helper.URLMustParse(authBackend), Version: "123"}
+	u := &upstream.Upstream{
+		Backend:      helper.URLMustParse(authBackend),
+		Version:      "123",
+		DocumentRoot: testDocumentRoot,
+	}
 	return httptest.NewServer(u)
 }