From b314aa644bd76934ab98b3c81c93fa023d8f27a4 Mon Sep 17 00:00:00 2001
From: Jaime Martinez <jmartinez@gitlab.com>
Date: Thu, 8 Oct 2020 16:38:13 +1100
Subject: [PATCH 1/7] Do not try to resolve pages-domain

---
 app.go | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/app.go b/app.go
index 5a1953962..8473e068f 100644
--- a/app.go
+++ b/app.go
@@ -12,10 +12,11 @@ import (
 	ghandlers "github.com/gorilla/handlers"
 	"github.com/rs/cors"
 	log "github.com/sirupsen/logrus"
+	"gitlab.com/lupine/go-mimedb"
+
 	"gitlab.com/gitlab-org/labkit/errortracking"
 	labmetrics "gitlab.com/gitlab-org/labkit/metrics"
 	"gitlab.com/gitlab-org/labkit/monitoring"
-	"gitlab.com/lupine/go-mimedb"
 
 	"gitlab.com/gitlab-org/gitlab-pages/internal/acme"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/artifact"
@@ -85,6 +86,12 @@ func (a *theApp) redirectToHTTPS(w http.ResponseWriter, r *http.Request, statusC
 
 func (a *theApp) getHostAndDomain(r *http.Request) (string, *domain.Domain, error) {
 	host := request.GetHostWithoutPort(r)
+
+	// do not try to serve files under -pages-domain
+	if a.Domain == host {
+		return host, nil, nil
+	}
+
 	domain, err := a.domain(host)
 
 	return host, domain, err
-- 
GitLab


From 3776d21a54c61ffa1abb43791ea2b5221444d4a9 Mon Sep 17 00:00:00 2001
From: Jaime Martinez <jmartinez@gitlab.com>
Date: Thu, 8 Oct 2020 17:55:28 +1100
Subject: [PATCH 2/7] WIP: cache the serving request

---
 internal/auth/auth.go               |  1 +
 internal/domain/domain.go           | 10 +++++++++-
 internal/source/disk/domain_test.go |  2 ++
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/internal/auth/auth.go b/internal/auth/auth.go
index eaf3c25dd..04adbbb5d 100644
--- a/internal/auth/auth.go
+++ b/internal/auth/auth.go
@@ -16,6 +16,7 @@ import (
 	"github.com/gorilla/securecookie"
 	"github.com/gorilla/sessions"
 	log "github.com/sirupsen/logrus"
+
 	"gitlab.com/gitlab-org/labkit/errortracking"
 
 	"gitlab.com/gitlab-org/gitlab-pages/internal/httperrors"
diff --git a/internal/domain/domain.go b/internal/domain/domain.go
index deff2cc5c..1dac872b5 100644
--- a/internal/domain/domain.go
+++ b/internal/domain/domain.go
@@ -14,6 +14,8 @@ import (
 
 // Domain is a domain that gitlab-pages can serve.
 type Domain struct {
+	servingRequest *serving.Request
+
 	Name            string
 	CertificateCert string
 	CertificateKey  string
@@ -39,14 +41,20 @@ func (d *Domain) isUnconfigured() bool {
 }
 
 func (d *Domain) resolve(r *http.Request) *serving.Request {
+	if d.servingRequest != nil {
+		return d.servingRequest
+	}
+
 	request, _ := d.Resolver.Resolve(r)
 
 	// TODO improve code around default serving, when `disk` serving gets removed
 	// https://gitlab.com/gitlab-org/gitlab-pages/issues/353
 	if request == nil {
-		return &serving.Request{Serving: local.Instance()}
+		request = &serving.Request{Serving: local.Instance()}
 	}
 
+	d.servingRequest = request
+
 	return request
 }
 
diff --git a/internal/source/disk/domain_test.go b/internal/source/disk/domain_test.go
index 1c81ba228..65cc98c07 100644
--- a/internal/source/disk/domain_test.go
+++ b/internal/source/disk/domain_test.go
@@ -27,6 +27,8 @@ func serveFileOrNotFound(domain *domain.Domain) http.HandlerFunc {
 }
 
 func testGroupServeHTTPHost(t *testing.T, host string) {
+	t.Helper()
+
 	testGroup := &domain.Domain{
 		Resolver: &Group{
 			name: "group",
-- 
GitLab


From 6b1237b0fb3d64efb31cb754931ccd0d8302c4d2 Mon Sep 17 00:00:00 2001
From: Jaime Martinez <jmartinez@gitlab.com>
Date: Mon, 12 Oct 2020 17:47:44 +1100
Subject: [PATCH 3/7] Fix unit tests

---
 internal/domain/domain.go           |  17 +--
 internal/source/disk/domain_test.go | 157 +++++++++++++++-------------
 2 files changed, 89 insertions(+), 85 deletions(-)

diff --git a/internal/domain/domain.go b/internal/domain/domain.go
index 1dac872b5..56e5f4851 100644
--- a/internal/domain/domain.go
+++ b/internal/domain/domain.go
@@ -53,6 +53,7 @@ func (d *Domain) resolve(r *http.Request) *serving.Request {
 		request = &serving.Request{Serving: local.Instance()}
 	}
 
+	// store serving.Request to avoid calling d.Resolver.Resolve multiple times
 	d.servingRequest = request
 
 	return request
@@ -65,13 +66,7 @@ func (d *Domain) GetLookupPath(r *http.Request) *serving.LookupPath {
 		return nil
 	}
 
-	request := d.resolve(r)
-
-	if request == nil {
-		return nil
-	}
-
-	return request.LookupPath
+	return d.resolve(r).LookupPath
 }
 
 // IsHTTPSOnly figures out if the request should be handled with HTTPS
@@ -145,9 +140,7 @@ func (d *Domain) ServeFileHTTP(w http.ResponseWriter, r *http.Request) bool {
 		return true
 	}
 
-	request := d.resolve(r)
-
-	return request.ServeFileHTTP(w, r)
+	return d.resolve(r).ServeFileHTTP(w, r)
 }
 
 // ServeNotFoundHTTP serves the not found pages from the projects.
@@ -157,9 +150,7 @@ func (d *Domain) ServeNotFoundHTTP(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	request := d.resolve(r)
-
-	request.ServeNotFoundHTTP(w, r)
+	d.resolve(r).ServeNotFoundHTTP(w, r)
 }
 
 // serveNamespaceNotFound will try to find a parent namespace domain for a request
diff --git a/internal/source/disk/domain_test.go b/internal/source/disk/domain_test.go
index 65cc98c07..87827fb8a 100644
--- a/internal/source/disk/domain_test.go
+++ b/internal/source/disk/domain_test.go
@@ -18,10 +18,12 @@ import (
 	"gitlab.com/gitlab-org/gitlab-pages/internal/testhelpers"
 )
 
-func serveFileOrNotFound(domain *domain.Domain) http.HandlerFunc {
+func serveFileOrNotFound(getDomain func() *domain.Domain) http.HandlerFunc {
+	d := getDomain()
 	return func(w http.ResponseWriter, r *http.Request) {
-		if !domain.ServeFileHTTP(w, r) {
-			domain.ServeNotFoundHTTP(w, r)
+
+		if !d.ServeFileHTTP(w, r) {
+			d.ServeNotFoundHTTP(w, r)
 		}
 	}
 }
@@ -29,45 +31,45 @@ func serveFileOrNotFound(domain *domain.Domain) http.HandlerFunc {
 func testGroupServeHTTPHost(t *testing.T, host string) {
 	t.Helper()
 
-	testGroup := &domain.Domain{
-		Resolver: &Group{
-			name: "group",
-			projects: map[string]*projectConfig{
-				"group.test.io":            &projectConfig{},
-				"group.gitlab-example.com": &projectConfig{},
-				"project":                  &projectConfig{},
-				"project2":                 &projectConfig{},
+	testGroup := func() *domain.Domain {
+		return &domain.Domain{
+			Resolver: &Group{
+				name: "group",
+				projects: map[string]*projectConfig{
+					"group.test.io":            &projectConfig{},
+					"group.gitlab-example.com": &projectConfig{},
+					"project":                  &projectConfig{},
+					"project2":                 &projectConfig{},
+				},
 			},
-		},
+		}
 	}
 
 	makeURL := func(path string) string {
 		return "http://" + host + path
 	}
 
-	serve := serveFileOrNotFound(testGroup)
-
-	require.HTTPBodyContains(t, serve, "GET", makeURL("/"), nil, "main-dir")
-	require.HTTPBodyContains(t, serve, "GET", makeURL("/index"), nil, "main-dir")
-	require.HTTPBodyContains(t, serve, "GET", makeURL("/index.html"), nil, "main-dir")
-	testhelpers.AssertRedirectTo(t, serve, "GET", makeURL("/project"), nil, "//"+host+"/project/")
-	require.HTTPBodyContains(t, serve, "GET", makeURL("/project/"), nil, "project-subdir")
-	require.HTTPBodyContains(t, serve, "GET", makeURL("/project/index"), nil, "project-subdir")
-	require.HTTPBodyContains(t, serve, "GET", makeURL("/project/index/"), nil, "project-subdir")
-	require.HTTPBodyContains(t, serve, "GET", makeURL("/project/index.html"), nil, "project-subdir")
-	testhelpers.AssertRedirectTo(t, serve, "GET", makeURL("/project/subdir"), nil, "//"+host+"/project/subdir/")
-	require.HTTPBodyContains(t, serve, "GET", makeURL("/project/subdir/"), nil, "project-subsubdir")
-	require.HTTPBodyContains(t, serve, "GET", makeURL("/project2/"), nil, "project2-main")
-	require.HTTPBodyContains(t, serve, "GET", makeURL("/project2/index"), nil, "project2-main")
-	require.HTTPBodyContains(t, serve, "GET", makeURL("/project2/index.html"), nil, "project2-main")
-	require.HTTPError(t, serve, "GET", makeURL("/private.project/"), nil)
-	require.HTTPError(t, serve, "GET", makeURL("//about.gitlab.com/%2e%2e"), nil)
-	require.HTTPError(t, serve, "GET", makeURL("/symlink"), nil)
-	require.HTTPError(t, serve, "GET", makeURL("/symlink/index.html"), nil)
-	require.HTTPError(t, serve, "GET", makeURL("/symlink/subdir/"), nil)
-	require.HTTPError(t, serve, "GET", makeURL("/project/fifo"), nil)
-	require.HTTPError(t, serve, "GET", makeURL("/not-existing-file"), nil)
-	require.HTTPRedirect(t, serve, "GET", makeURL("/project//about.gitlab.com/%2e%2e"), nil)
+	require.HTTPBodyContains(t, serveFileOrNotFound(testGroup), "GET", makeURL("/"), nil, "main-dir")
+	require.HTTPBodyContains(t, serveFileOrNotFound(testGroup), "GET", makeURL("/index"), nil, "main-dir")
+	require.HTTPBodyContains(t, serveFileOrNotFound(testGroup), "GET", makeURL("/index.html"), nil, "main-dir")
+	testhelpers.AssertRedirectTo(t, serveFileOrNotFound(testGroup), "GET", makeURL("/project"), nil, "//"+host+"/project/")
+	require.HTTPBodyContains(t, serveFileOrNotFound(testGroup), "GET", makeURL("/project/"), nil, "project-subdir")
+	require.HTTPBodyContains(t, serveFileOrNotFound(testGroup), "GET", makeURL("/project/index"), nil, "project-subdir")
+	require.HTTPBodyContains(t, serveFileOrNotFound(testGroup), "GET", makeURL("/project/index/"), nil, "project-subdir")
+	require.HTTPBodyContains(t, serveFileOrNotFound(testGroup), "GET", makeURL("/project/index.html"), nil, "project-subdir")
+	testhelpers.AssertRedirectTo(t, serveFileOrNotFound(testGroup), "GET", makeURL("/project/subdir"), nil, "//"+host+"/project/subdir/")
+	require.HTTPBodyContains(t, serveFileOrNotFound(testGroup), "GET", makeURL("/project/subdir/"), nil, "project-subsubdir")
+	require.HTTPBodyContains(t, serveFileOrNotFound(testGroup), "GET", makeURL("/project2/"), nil, "project2-main")
+	require.HTTPBodyContains(t, serveFileOrNotFound(testGroup), "GET", makeURL("/project2/index"), nil, "project2-main")
+	require.HTTPBodyContains(t, serveFileOrNotFound(testGroup), "GET", makeURL("/project2/index.html"), nil, "project2-main")
+	require.HTTPError(t, serveFileOrNotFound(testGroup), "GET", makeURL("/private.project/"), nil)
+	require.HTTPError(t, serveFileOrNotFound(testGroup), "GET", makeURL("//about.gitlab.com/%2e%2e"), nil)
+	require.HTTPError(t, serveFileOrNotFound(testGroup), "GET", makeURL("/symlink"), nil)
+	require.HTTPError(t, serveFileOrNotFound(testGroup), "GET", makeURL("/symlink/index.html"), nil)
+	require.HTTPError(t, serveFileOrNotFound(testGroup), "GET", makeURL("/symlink/subdir/"), nil)
+	require.HTTPError(t, serveFileOrNotFound(testGroup), "GET", makeURL("/project/fifo"), nil)
+	require.HTTPError(t, serveFileOrNotFound(testGroup), "GET", makeURL("/not-existing-file"), nil)
+	require.HTTPRedirect(t, serveFileOrNotFound(testGroup), "GET", makeURL("/project//about.gitlab.com/%2e%2e"), nil)
 }
 
 func TestGroupServeHTTP(t *testing.T) {
@@ -82,12 +84,14 @@ func TestDomainServeHTTP(t *testing.T) {
 	cleanup := setUpTests(t)
 	defer cleanup()
 
-	testDomain := &domain.Domain{
-		Name: "test.domain.com",
-		Resolver: &customProjectResolver{
-			path:   "group/project2/public",
-			config: &domainConfig{},
-		},
+	testDomain := func() *domain.Domain {
+		return &domain.Domain{
+			Name: "test.domain.com",
+			Resolver: &customProjectResolver{
+				path:   "group/project2/public",
+				config: &domainConfig{},
+			},
+		}
 	}
 
 	require.HTTPBodyContains(t, serveFileOrNotFound(testDomain), "GET", "/", nil, "project2-main")
@@ -213,16 +217,18 @@ func TestGroupServeHTTPGzip(t *testing.T) {
 	cleanup := setUpTests(t)
 	defer cleanup()
 
-	testGroup := &domain.Domain{
-		Resolver: &Group{
-			name: "group",
-			projects: map[string]*projectConfig{
-				"group.test.io":            &projectConfig{},
-				"group.gitlab-example.com": &projectConfig{},
-				"project":                  &projectConfig{},
-				"project2":                 &projectConfig{},
+	testGroup := func() *domain.Domain {
+		return &domain.Domain{
+			Resolver: &Group{
+				name: "group",
+				projects: map[string]*projectConfig{
+					"group.test.io":            &projectConfig{},
+					"group.gitlab-example.com": &projectConfig{},
+					"project":                  &projectConfig{},
+					"project2":                 &projectConfig{},
+				},
 			},
-		},
+		}
 	}
 
 	testSet := []struct {
@@ -380,17 +386,19 @@ func TestGroup404ServeHTTP(t *testing.T) {
 	cleanup := setUpTests(t)
 	defer cleanup()
 
-	testGroup := &domain.Domain{
-		Resolver: &Group{
-			name: "group.404",
-			projects: map[string]*projectConfig{
-				"domain.404":          &projectConfig{},
-				"group.404.test.io":   &projectConfig{},
-				"project.404":         &projectConfig{},
-				"project.404.symlink": &projectConfig{},
-				"project.no.404":      &projectConfig{},
+	testGroup := func() *domain.Domain {
+		return &domain.Domain{
+			Resolver: &Group{
+				name: "group.404",
+				projects: map[string]*projectConfig{
+					"domain.404":          &projectConfig{},
+					"group.404.test.io":   &projectConfig{},
+					"project.404":         &projectConfig{},
+					"project.404.symlink": &projectConfig{},
+					"project.no.404":      &projectConfig{},
+				},
 			},
-		},
+		}
 	}
 
 	testhelpers.AssertHTTP404(t, serveFileOrNotFound(testGroup), "GET", "http://group.404.test.io/project.404/not/existing-file", nil, "Custom 404 project page")
@@ -408,11 +416,13 @@ func TestDomain404ServeHTTP(t *testing.T) {
 	cleanup := setUpTests(t)
 	defer cleanup()
 
-	testDomain := &domain.Domain{
-		Resolver: &customProjectResolver{
-			path:   "group.404/domain.404/public/",
-			config: &domainConfig{Domain: "domain.404.com"},
-		},
+	testDomain := func() *domain.Domain {
+		return &domain.Domain{
+			Resolver: &customProjectResolver{
+				path:   "group.404/domain.404/public/",
+				config: &domainConfig{Domain: "domain.404.com"},
+			},
+		}
 	}
 
 	testhelpers.AssertHTTP404(t, serveFileOrNotFound(testDomain), "GET", "http://group.404.test.io/not-existing-file", nil, "Custom domain.404 page")
@@ -423,7 +433,7 @@ func TestPredefined404ServeHTTP(t *testing.T) {
 	cleanup := setUpTests(t)
 	defer cleanup()
 
-	testDomain := &domain.Domain{}
+	testDomain := func() *domain.Domain { return &domain.Domain{} }
 
 	testhelpers.AssertHTTP404(t, serveFileOrNotFound(testDomain), "GET", "http://group.test.io/not-existing-file", nil, "The page you're looking for could not be found")
 }
@@ -472,14 +482,17 @@ func TestCacheControlHeaders(t *testing.T) {
 	cleanup := setUpTests(t)
 	defer cleanup()
 
-	testGroup := &domain.Domain{
-		Resolver: &Group{
-			name: "group",
-			projects: map[string]*projectConfig{
-				"group.test.io": &projectConfig{},
+	testGroup := func() *domain.Domain {
+		return &domain.Domain{
+			Resolver: &Group{
+				name: "group",
+				projects: map[string]*projectConfig{
+					"group.test.io": &projectConfig{},
+				},
 			},
-		},
+		}
 	}
+
 	w := httptest.NewRecorder()
 	req, err := http.NewRequest("GET", "http://group.test.io/", nil)
 	require.NoError(t, err)
-- 
GitLab


From 0fcc0667e6f2e78ca85abdb5c9f631a964dd77c2 Mon Sep 17 00:00:00 2001
From: Jaime Martinez <jmartinez@gitlab.com>
Date: Tue, 13 Oct 2020 14:45:26 +1100
Subject: [PATCH 4/7] Remove serving.Handler

---
 internal/domain/domain_test.go                |  8 +--
 internal/serving/disk/local/serving_test.go   | 13 ++--
 internal/serving/disk/reader.go               | 60 ++++++++++---------
 internal/serving/disk/serving.go              | 13 ++--
 internal/serving/disk/zip/serving_test.go     | 13 ++--
 internal/serving/handler.go                   | 12 ----
 internal/serving/lookup_path.go               |  1 +
 internal/serving/request.go                   | 19 +-----
 internal/serving/serverless/serverless.go     | 11 ++--
 .../serving/serverless/serverless_test.go     | 13 ++--
 internal/serving/serving.go                   |  8 ++-
 internal/source/disk/custom.go                |  2 +-
 internal/source/disk/group.go                 |  2 +-
 internal/source/gitlab/factory.go             |  5 +-
 internal/source/gitlab/factory_test.go        |  5 +-
 internal/source/gitlab/gitlab.go              |  4 +-
 internal/source/gitlab/gitlab_test.go         | 10 ++--
 17 files changed, 86 insertions(+), 113 deletions(-)
 delete mode 100644 internal/serving/handler.go

diff --git a/internal/domain/domain_test.go b/internal/domain/domain_test.go
index 2df1d9929..1020d70a6 100644
--- a/internal/domain/domain_test.go
+++ b/internal/domain/domain_test.go
@@ -17,7 +17,6 @@ import (
 
 type stubbedResolver struct {
 	project *serving.LookupPath
-	subpath string
 	err     error
 }
 
@@ -25,7 +24,6 @@ func (resolver *stubbedResolver) Resolve(*http.Request) (*serving.Request, error
 	return &serving.Request{
 		Serving:    local.Instance(),
 		LookupPath: resolver.project,
-		SubPath:    resolver.subpath,
 	}, resolver.err
 }
 
@@ -154,8 +152,8 @@ func TestServeNamespaceNotFound(t *testing.T) {
 				project: &serving.LookupPath{
 					Path:               "group.404/group.404.gitlab-example.com/public",
 					IsNamespaceProject: true,
+					SubPath:            "/unknown",
 				},
-				subpath: "/unknown",
 			},
 			expectedResponse: "Custom 404 group page",
 		},
@@ -169,7 +167,6 @@ func TestServeNamespaceNotFound(t *testing.T) {
 					IsNamespaceProject: true,
 					HasAccessControl:   false,
 				},
-				subpath: "/",
 			},
 			expectedResponse: "Custom 404 group page",
 		},
@@ -182,8 +179,8 @@ func TestServeNamespaceNotFound(t *testing.T) {
 					Path:               "group.404/group.404.gitlab-example.com/public",
 					IsNamespaceProject: true,
 					HasAccessControl:   true,
+					SubPath:            "/",
 				},
-				subpath: "/",
 			},
 			expectedResponse: "The page you're looking for could not be found.",
 		},
@@ -193,7 +190,6 @@ func TestServeNamespaceNotFound(t *testing.T) {
 			path:   "/unknown",
 			resolver: &stubbedResolver{
 				project: nil,
-				subpath: "/",
 			},
 			expectedResponse: "The page you're looking for could not be found.",
 		},
diff --git a/internal/serving/disk/local/serving_test.go b/internal/serving/disk/local/serving_test.go
index 60f01acd6..1f6929077 100644
--- a/internal/serving/disk/local/serving_test.go
+++ b/internal/serving/disk/local/serving_test.go
@@ -18,17 +18,14 @@ func TestDisk_ServeFileHTTP(t *testing.T) {
 	s := Instance()
 	w := httptest.NewRecorder()
 	r := httptest.NewRequest("GET", "http://group.gitlab-example.com/serving/index.html", nil)
-	handler := serving.Handler{
-		Writer:  w,
-		Request: r,
-		LookupPath: &serving.LookupPath{
-			Prefix: "/serving",
-			Path:   "group/serving/public",
-		},
+
+	lookupPath := &serving.LookupPath{
+		Prefix:  "/serving",
+		Path:    "group/serving/public",
 		SubPath: "/index.html",
 	}
 
-	require.True(t, s.ServeFileHTTP(handler))
+	require.True(t, s.ServeFileHTTP(w, r, lookupPath))
 
 	resp := w.Result()
 	defer resp.Body.Close()
diff --git a/internal/serving/disk/reader.go b/internal/serving/disk/reader.go
index e7f15a1e3..28ca11143 100644
--- a/internal/serving/disk/reader.go
+++ b/internal/serving/disk/reader.go
@@ -25,50 +25,52 @@ type Reader struct {
 }
 
 // Show the user some validation messages for their _redirects file
-func (reader *Reader) serveRedirectsStatus(h serving.Handler, redirects *redirects.Redirects) error {
-	h.Writer.Header().Set("Content-Type", "text/plain; charset=utf-8")
-	h.Writer.Header().Set("X-Content-Type-Options", "nosniff")
-	h.Writer.WriteHeader(http.StatusOK)
-	_, err := fmt.Fprintln(h.Writer, redirects.Status())
+func (reader *Reader) serveRedirectsStatus(w http.ResponseWriter, redirects *redirects.Redirects) error {
+	w.Header().Set("Content-Type", "text/plain; charset=utf-8")
+	w.Header().Set("X-Content-Type-Options", "nosniff")
+	w.WriteHeader(http.StatusOK)
+
+	_, err := fmt.Fprintln(w, redirects.Status())
 	return err
 }
 
-func (reader *Reader) tryRedirects(h serving.Handler) error {
-	ctx := h.Request.Context()
-	root, err := reader.vfs.Root(ctx, h.LookupPath.Path)
+func (reader *Reader) tryRedirects(w http.ResponseWriter, r *http.Request, lookupPath *serving.LookupPath) error {
+	ctx := r.Context()
+	root, err := reader.vfs.Root(ctx, lookupPath.Path)
 	if err != nil {
 		return err
 	}
 
-	r := redirects.ParseRedirects(ctx, root)
+	redirs := redirects.ParseRedirects(ctx, root)
 
-	rewrittenURL, status, err := r.Rewrite(h.Request.URL)
+	rewrittenURL, status, err := redirs.Rewrite(r.URL)
 	if err != nil {
 		return err
 	}
 
-	http.Redirect(h.Writer, h.Request, rewrittenURL.Path, status)
+	http.Redirect(w, r, rewrittenURL.Path, status)
 
 	return nil
 }
 
-func (reader *Reader) tryFile(h serving.Handler) error {
-	ctx := h.Request.Context()
+func (reader *Reader) tryFile(w http.ResponseWriter, r *http.Request, lookupPath *serving.LookupPath) error {
+	ctx := r.Context()
 
-	root, err := reader.vfs.Root(ctx, h.LookupPath.Path)
+	root, err := reader.vfs.Root(ctx, lookupPath.Path)
 	if err != nil {
 		return err
 	}
 
-	fullPath, err := reader.resolvePath(ctx, root, h.SubPath)
+	fullPath, err := reader.resolvePath(ctx, root, lookupPath.SubPath)
 
-	request := h.Request
-	host := request.Host
-	urlPath := request.URL.Path
+	// request := h.Request
+	host := r.Host
+	urlPath := r.URL.Path
 
 	if locationError, _ := err.(*locationDirectoryError); locationError != nil {
 		if endsWithSlash(urlPath) {
-			fullPath, err = reader.resolvePath(ctx, root, h.SubPath, "index.html")
+			fullPath, err = reader.resolvePath(ctx, root, lookupPath.SubPath,
+				"index.html")
 		} else {
 			// TODO why are we doing that? In tests it redirects to HTTPS. This seems wrong,
 			// issue about this: https://gitlab.com/gitlab-org/gitlab-pages/issues/273
@@ -79,13 +81,14 @@ func (reader *Reader) tryFile(h serving.Handler) error {
 
 			// Ensure that there's always "/" at end
 			redirectPath = strings.TrimSuffix(redirectPath, "/") + "/"
-			http.Redirect(h.Writer, h.Request, redirectPath, 302)
+			http.Redirect(w, r, redirectPath, 302)
 			return nil
 		}
 	}
 
 	if locationError, _ := err.(*locationFileNoExtensionError); locationError != nil {
-		fullPath, err = reader.resolvePath(ctx, root, strings.TrimSuffix(h.SubPath, "/")+".html")
+		fullPath, err = reader.resolvePath(ctx, root,
+			strings.TrimSuffix(lookupPath.SubPath, "/")+".html")
 	}
 
 	if err != nil {
@@ -97,20 +100,20 @@ func (reader *Reader) tryFile(h serving.Handler) error {
 	if fullPath == redirects.ConfigFile {
 		if os.Getenv("FF_ENABLE_REDIRECTS") != "false" {
 			r := redirects.ParseRedirects(ctx, root)
-			return reader.serveRedirectsStatus(h, r)
+			return reader.serveRedirectsStatus(w, r)
 		}
 
-		h.Writer.WriteHeader(http.StatusForbidden)
+		w.WriteHeader(http.StatusForbidden)
 		return nil
 	}
 
-	return reader.serveFile(ctx, h.Writer, h.Request, root, fullPath, h.LookupPath.HasAccessControl)
+	return reader.serveFile(ctx, w, r, root, fullPath, lookupPath.HasAccessControl)
 }
 
-func (reader *Reader) tryNotFound(h serving.Handler) error {
-	ctx := h.Request.Context()
+func (reader *Reader) tryNotFound(w http.ResponseWriter, r *http.Request, lookupPath *serving.LookupPath) error {
+	ctx := r.Context()
 
-	root, err := reader.vfs.Root(ctx, h.LookupPath.Path)
+	root, err := reader.vfs.Root(ctx, lookupPath.Path)
 	if err != nil {
 		return err
 	}
@@ -120,7 +123,8 @@ func (reader *Reader) tryNotFound(h serving.Handler) error {
 		return err
 	}
 
-	err = reader.serveCustomFile(ctx, h.Writer, h.Request, http.StatusNotFound, root, page404)
+	err = reader.serveCustomFile(ctx, w, r, http.StatusNotFound,
+		root, page404)
 	if err != nil {
 		return err
 	}
diff --git a/internal/serving/disk/serving.go b/internal/serving/disk/serving.go
index 11b1689e3..34762491f 100644
--- a/internal/serving/disk/serving.go
+++ b/internal/serving/disk/serving.go
@@ -1,6 +1,7 @@
 package disk
 
 import (
+	"net/http"
 	"os"
 
 	"gitlab.com/gitlab-org/gitlab-pages/internal/httperrors"
@@ -16,13 +17,13 @@ type Disk struct {
 
 // ServeFileHTTP serves a file from disk and returns true. It returns false
 // when a file could not been found.
-func (s *Disk) ServeFileHTTP(h serving.Handler) bool {
-	if s.reader.tryFile(h) == nil {
+func (s *Disk) ServeFileHTTP(w http.ResponseWriter, r *http.Request, lookupPath *serving.LookupPath) bool {
+	if s.reader.tryFile(w, r, lookupPath) == nil {
 		return true
 	}
 
 	if os.Getenv("FF_ENABLE_REDIRECTS") != "false" {
-		if s.reader.tryRedirects(h) == nil {
+		if s.reader.tryRedirects(w, r, lookupPath) == nil {
 			return true
 		}
 	}
@@ -31,13 +32,13 @@ func (s *Disk) ServeFileHTTP(h serving.Handler) bool {
 }
 
 // ServeNotFoundHTTP tries to read a custom 404 page
-func (s *Disk) ServeNotFoundHTTP(h serving.Handler) {
-	if s.reader.tryNotFound(h) == nil {
+func (s *Disk) ServeNotFoundHTTP(w http.ResponseWriter, r *http.Request, lookupPath *serving.LookupPath) {
+	if s.reader.tryNotFound(w, r, lookupPath) == nil {
 		return
 	}
 
 	// Generic 404
-	httperrors.Serve404(h.Writer)
+	httperrors.Serve404(w)
 }
 
 // New returns a serving instance that is capable of reading files
diff --git a/internal/serving/disk/zip/serving_test.go b/internal/serving/disk/zip/serving_test.go
index 14fdabdf6..134fe4dc3 100644
--- a/internal/serving/disk/zip/serving_test.go
+++ b/internal/serving/disk/zip/serving_test.go
@@ -19,17 +19,14 @@ func TestZip_ServeFileHTTP(t *testing.T) {
 	s := Instance()
 	w := httptest.NewRecorder()
 	r := httptest.NewRequest("GET", "http://zip.gitlab.io/zip/index.html", nil)
-	handler := serving.Handler{
-		Writer:  w,
-		Request: r,
-		LookupPath: &serving.LookupPath{
-			Prefix: "",
-			Path:   testServerURL + "/public.zip",
-		},
+
+	lookupPath := &serving.LookupPath{
+		Prefix:  "",
+		Path:    testServerURL + "/public.zip",
 		SubPath: "/index.html",
 	}
 
-	require.True(t, s.ServeFileHTTP(handler))
+	require.True(t, s.ServeFileHTTP(w, r, lookupPath))
 
 	resp := w.Result()
 	defer resp.Body.Close()
diff --git a/internal/serving/handler.go b/internal/serving/handler.go
deleted file mode 100644
index a0d66ecbf..000000000
--- a/internal/serving/handler.go
+++ /dev/null
@@ -1,12 +0,0 @@
-package serving
-
-import "net/http"
-
-// Handler aggregates response/request and lookup path + subpath needed to
-// handle a request and response.
-type Handler struct {
-	Writer     http.ResponseWriter
-	Request    *http.Request
-	LookupPath *LookupPath
-	SubPath    string
-}
diff --git a/internal/serving/lookup_path.go b/internal/serving/lookup_path.go
index 1aefe1b85..1f0ec0f3a 100644
--- a/internal/serving/lookup_path.go
+++ b/internal/serving/lookup_path.go
@@ -9,4 +9,5 @@ type LookupPath struct {
 	IsHTTPSOnly        bool
 	HasAccessControl   bool
 	ProjectID          uint64
+	SubPath            string
 }
diff --git a/internal/serving/request.go b/internal/serving/request.go
index 694d0df4f..dac03f91e 100644
--- a/internal/serving/request.go
+++ b/internal/serving/request.go
@@ -7,29 +7,14 @@ import "net/http"
 type Request struct {
 	Serving    Serving     // Serving chosen to serve this request
 	LookupPath *LookupPath // LookupPath contains pages project details
-	SubPath    string      // Subpath is a URL path subcomponent for this request
 }
 
 // ServeFileHTTP forwards serving request handler to the serving itself
 func (s *Request) ServeFileHTTP(w http.ResponseWriter, r *http.Request) bool {
-	handler := Handler{
-		Writer:     w,
-		Request:    r,
-		LookupPath: s.LookupPath,
-		SubPath:    s.SubPath,
-	}
-
-	return s.Serving.ServeFileHTTP(handler)
+	return s.Serving.ServeFileHTTP(w, r, s.LookupPath)
 }
 
 // ServeNotFoundHTTP forwards serving request handler to the serving itself
 func (s *Request) ServeNotFoundHTTP(w http.ResponseWriter, r *http.Request) {
-	handler := Handler{
-		Writer:     w,
-		Request:    r,
-		LookupPath: s.LookupPath,
-		SubPath:    s.SubPath,
-	}
-
-	s.Serving.ServeNotFoundHTTP(handler)
+	s.Serving.ServeNotFoundHTTP(w, r, s.LookupPath)
 }
diff --git a/internal/serving/serverless/serverless.go b/internal/serving/serverless/serverless.go
index e1881362a..c2f592de1 100644
--- a/internal/serving/serverless/serverless.go
+++ b/internal/serving/serverless/serverless.go
@@ -2,6 +2,7 @@ package serverless
 
 import (
 	"errors"
+	"net/http"
 	"net/http/httputil"
 
 	"gitlab.com/gitlab-org/gitlab-pages/internal/httperrors"
@@ -53,15 +54,17 @@ func New(service string, cluster Cluster) serving.Serving {
 }
 
 // ServeFileHTTP handle an incoming request and proxies it to Knative cluster
-func (s *Serverless) ServeFileHTTP(h serving.Handler) bool {
+func (s *Serverless) ServeFileHTTP(w http.ResponseWriter, r *http.Request,
+	_ *serving.LookupPath) bool {
 	metrics.ServerlessRequests.Inc()
 
-	s.proxy.ServeHTTP(h.Writer, h.Request)
+	s.proxy.ServeHTTP(w, r)
 
 	return true
 }
 
 // ServeNotFoundHTTP responds with 404
-func (s *Serverless) ServeNotFoundHTTP(h serving.Handler) {
-	httperrors.Serve404(h.Writer)
+func (s *Serverless) ServeNotFoundHTTP(w http.ResponseWriter,
+	_ *http.Request, _ *serving.LookupPath) {
+	httperrors.Serve404(w)
 }
diff --git a/internal/serving/serverless/serverless_test.go b/internal/serving/serverless/serverless_test.go
index ebd143432..54cf23198 100644
--- a/internal/serving/serverless/serverless_test.go
+++ b/internal/serving/serverless/serverless_test.go
@@ -11,7 +11,6 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"gitlab.com/gitlab-org/gitlab-pages/internal/fixture"
-	"gitlab.com/gitlab-org/gitlab-pages/internal/serving"
 )
 
 func withTestCluster(t *testing.T, cert, key string, block func(*http.ServeMux, *url.URL, *Certs)) {
@@ -50,7 +49,6 @@ func TestServeFileHTTP(t *testing.T) {
 
 			writer := httptest.NewRecorder()
 			request := httptest.NewRequest("GET", "http://example.gitlab.com/", nil)
-			handler := serving.Handler{Writer: writer, Request: request}
 			request.Header.Set("X-Real-IP", "127.0.0.105")
 
 			mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
@@ -60,7 +58,7 @@ func TestServeFileHTTP(t *testing.T) {
 				require.Contains(t, r.Header.Get("X-Forwarded-For"), "127.0.0.105")
 			})
 
-			served := serverless.ServeFileHTTP(handler)
+			served := serverless.ServeFileHTTP(writer, request, nil)
 			result := writer.Result()
 
 			require.True(t, served)
@@ -82,13 +80,12 @@ func TestServeFileHTTP(t *testing.T) {
 
 			writer := httptest.NewRecorder()
 			request := httptest.NewRequest("GET", "http://example.gitlab.com/", nil)
-			handler := serving.Handler{Writer: writer, Request: request}
 
 			mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
 				w.WriteHeader(http.StatusBadRequest)
 			})
 
-			served := serverless.ServeFileHTTP(handler)
+			served := serverless.ServeFileHTTP(writer, request, nil)
 			result := writer.Result()
 			body, err := ioutil.ReadAll(writer.Body)
 			require.NoError(t, err)
@@ -113,14 +110,13 @@ func TestServeFileHTTP(t *testing.T) {
 
 			writer := httptest.NewRecorder()
 			request := httptest.NewRequest("GET", "http://example.gitlab.com/", nil)
-			handler := serving.Handler{Writer: writer, Request: request}
 
 			mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
 				w.WriteHeader(http.StatusServiceUnavailable)
 				w.Write([]byte("sorry, service unavailable"))
 			})
 
-			served := serverless.ServeFileHTTP(handler)
+			served := serverless.ServeFileHTTP(writer, request, nil)
 			result := writer.Result()
 			body, err := ioutil.ReadAll(writer.Body)
 			require.NoError(t, err)
@@ -145,14 +141,13 @@ func TestServeFileHTTP(t *testing.T) {
 
 			writer := httptest.NewRecorder()
 			request := httptest.NewRequest("GET", "http://example.gitlab.com/", nil)
-			handler := serving.Handler{Writer: writer, Request: request}
 
 			mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
 				w.WriteHeader(http.StatusOK)
 				w.Write([]byte("OK"))
 			})
 
-			served := serverless.ServeFileHTTP(handler)
+			served := serverless.ServeFileHTTP(writer, request, nil)
 			result := writer.Result()
 			body, err := ioutil.ReadAll(writer.Body)
 			require.NoError(t, err)
diff --git a/internal/serving/serving.go b/internal/serving/serving.go
index 6fde82165..1c89e8f4c 100644
--- a/internal/serving/serving.go
+++ b/internal/serving/serving.go
@@ -1,7 +1,11 @@
 package serving
 
+import "net/http"
+
 // Serving is an interface used to define a serving driver
 type Serving interface {
-	ServeFileHTTP(Handler) bool
-	ServeNotFoundHTTP(Handler)
+	ServeFileHTTP(w http.ResponseWriter, r *http.Request,
+		lookupPath *LookupPath) bool
+	ServeNotFoundHTTP(w http.ResponseWriter, r *http.Request,
+		lookupPath *LookupPath)
 }
diff --git a/internal/source/disk/custom.go b/internal/source/disk/custom.go
index fb2aafcdc..9e8bb4572 100644
--- a/internal/source/disk/custom.go
+++ b/internal/source/disk/custom.go
@@ -22,11 +22,11 @@ func (p *customProjectResolver) Resolve(r *http.Request) (*serving.Request, erro
 		IsHTTPSOnly:        p.config.HTTPSOnly,
 		HasAccessControl:   p.config.AccessControl,
 		ProjectID:          p.config.ID,
+		SubPath:            r.URL.Path,
 	}
 
 	return &serving.Request{
 		Serving:    local.Instance(),
 		LookupPath: lookupPath,
-		SubPath:    r.URL.Path,
 	}, nil
 }
diff --git a/internal/source/disk/group.go b/internal/source/disk/group.go
index b12727aa6..8c54cdd4d 100644
--- a/internal/source/disk/group.go
+++ b/internal/source/disk/group.go
@@ -95,11 +95,11 @@ func (g *Group) Resolve(r *http.Request) (*serving.Request, error) {
 		IsHTTPSOnly:        projectConfig.HTTPSOnly,
 		HasAccessControl:   projectConfig.AccessControl,
 		ProjectID:          projectConfig.ID,
+		SubPath:            subPath,
 	}
 
 	return &serving.Request{
 		Serving:    local.Instance(),
 		LookupPath: lookupPath,
-		SubPath:    subPath,
 	}, nil
 }
diff --git a/internal/source/gitlab/factory.go b/internal/source/gitlab/factory.go
index 41f7ea56b..0df2e8c60 100644
--- a/internal/source/gitlab/factory.go
+++ b/internal/source/gitlab/factory.go
@@ -13,15 +13,16 @@ import (
 // fabricateLookupPath fabricates a serving LookupPath based on the API LookupPath
 // `size` argument is DEPRECATED, see
 // https://gitlab.com/gitlab-org/gitlab-pages/issues/272
-func fabricateLookupPath(size int, lookup api.LookupPath) *serving.LookupPath {
+func fabricateLookupPath(size int, lookup api.LookupPath, subPath string) *serving.LookupPath {
 	return &serving.LookupPath{
 		ServingType:        lookup.Source.Type,
 		Path:               lookup.Source.Path,
 		Prefix:             lookup.Prefix,
-		IsNamespaceProject: (lookup.Prefix == "/" && size > 1),
+		IsNamespaceProject: lookup.Prefix == "/" && size > 1,
 		IsHTTPSOnly:        lookup.HTTPSOnly,
 		HasAccessControl:   lookup.AccessControl,
 		ProjectID:          uint64(lookup.ProjectID),
+		SubPath:            subPath,
 	}
 }
 
diff --git a/internal/source/gitlab/factory_test.go b/internal/source/gitlab/factory_test.go
index 2f3e19940..804f5d020 100644
--- a/internal/source/gitlab/factory_test.go
+++ b/internal/source/gitlab/factory_test.go
@@ -15,16 +15,17 @@ func TestFabricateLookupPath(t *testing.T) {
 	t.Run("when lookup path is not a namespace project", func(t *testing.T) {
 		lookup := api.LookupPath{Prefix: "/something"}
 
-		path := fabricateLookupPath(1, lookup)
+		path := fabricateLookupPath(1, lookup, "/subpath")
 
 		require.Equal(t, path.Prefix, "/something")
+		require.Equal(t, path.SubPath, "/subpath")
 		require.False(t, path.IsNamespaceProject)
 	})
 
 	t.Run("when lookup path is a namespace project", func(t *testing.T) {
 		lookup := api.LookupPath{Prefix: "/"}
 
-		path := fabricateLookupPath(2, lookup)
+		path := fabricateLookupPath(2, lookup, "")
 
 		require.Equal(t, path.Prefix, "/")
 		require.True(t, path.IsNamespaceProject)
diff --git a/internal/source/gitlab/gitlab.go b/internal/source/gitlab/gitlab.go
index 67c4eb6ba..eecc2f2f8 100644
--- a/internal/source/gitlab/gitlab.go
+++ b/internal/source/gitlab/gitlab.go
@@ -96,8 +96,8 @@ func (g *Gitlab) Resolve(r *http.Request) (*serving.Request, error) {
 
 			return &serving.Request{
 				Serving:    fabricateServing(lookup),
-				LookupPath: fabricateLookupPath(size, lookup),
-				SubPath:    subPath}, nil
+				LookupPath: fabricateLookupPath(size, lookup, subPath),
+			}, nil
 		}
 	}
 
diff --git a/internal/source/gitlab/gitlab_test.go b/internal/source/gitlab/gitlab_test.go
index e6f194eee..5537a0339 100644
--- a/internal/source/gitlab/gitlab_test.go
+++ b/internal/source/gitlab/gitlab_test.go
@@ -44,7 +44,7 @@ func TestResolve(t *testing.T) {
 
 		require.Equal(t, "/my/pages/project/", response.LookupPath.Prefix)
 		require.Equal(t, "some/path/to/project/", response.LookupPath.Path)
-		require.Equal(t, "", response.SubPath)
+		require.Equal(t, "", response.LookupPath.SubPath)
 		require.False(t, response.LookupPath.IsNamespaceProject)
 	})
 
@@ -57,7 +57,7 @@ func TestResolve(t *testing.T) {
 
 		require.Equal(t, "/my/pages/project/", response.LookupPath.Prefix)
 		require.Equal(t, "some/path/to/project/", response.LookupPath.Path)
-		require.Equal(t, "path/index.html", response.SubPath)
+		require.Equal(t, "path/index.html", response.LookupPath.SubPath)
 		require.False(t, response.LookupPath.IsNamespaceProject)
 	})
 
@@ -70,7 +70,7 @@ func TestResolve(t *testing.T) {
 
 		require.Equal(t, "/", response.LookupPath.Prefix)
 		require.Equal(t, "some/path/to/project-3/", response.LookupPath.Path)
-		require.Equal(t, "", response.SubPath)
+		require.Equal(t, "", response.LookupPath.SubPath)
 		require.True(t, response.LookupPath.IsNamespaceProject)
 	})
 
@@ -82,7 +82,7 @@ func TestResolve(t *testing.T) {
 		require.NoError(t, err)
 
 		require.Equal(t, "/", response.LookupPath.Prefix)
-		require.Equal(t, "path/to/index.html", response.SubPath)
+		require.Equal(t, "path/to/index.html", response.LookupPath.SubPath)
 		require.Equal(t, "some/path/to/project-3/", response.LookupPath.Path)
 		require.True(t, response.LookupPath.IsNamespaceProject)
 	})
@@ -95,6 +95,6 @@ func TestResolve(t *testing.T) {
 		require.NoError(t, err)
 
 		require.Equal(t, "/my/pages/project/", response.LookupPath.Prefix)
-		require.Equal(t, "index.html", response.SubPath)
+		require.Equal(t, "index.html", response.LookupPath.SubPath)
 	})
 }
-- 
GitLab


From 72f6878d0de009f57720726a741af2610bc61d6e Mon Sep 17 00:00:00 2001
From: Jaime Martinez <jmartinez@gitlab.com>
Date: Tue, 13 Oct 2020 15:36:29 +1100
Subject: [PATCH 5/7] Check if same project

---
 internal/domain/domain.go           |  22 +++-
 internal/source/disk/domain_test.go | 159 +++++++++++++---------------
 2 files changed, 93 insertions(+), 88 deletions(-)

diff --git a/internal/domain/domain.go b/internal/domain/domain.go
index 56e5f4851..fe34d4d2b 100644
--- a/internal/domain/domain.go
+++ b/internal/domain/domain.go
@@ -5,6 +5,7 @@ import (
 	"crypto/tls"
 	"errors"
 	"net/http"
+	"strings"
 	"sync"
 
 	"gitlab.com/gitlab-org/gitlab-pages/internal/httperrors"
@@ -40,8 +41,27 @@ func (d *Domain) isUnconfigured() bool {
 	return d.Resolver == nil
 }
 
+func (d *Domain) isSameProject(reqPath string) bool {
+	return d.servingRequest != nil &&
+		d.servingRequest.LookupPath != nil &&
+		strings.Contains(reqPath, d.servingRequest.LookupPath.Path)
+}
+
 func (d *Domain) resolve(r *http.Request) *serving.Request {
-	if d.servingRequest != nil {
+	if d.isSameProject(r.URL.Path) {
+		// // && strings.
+		// // 	Contains(r.URL.
+		// // 		Path,
+		// // 		d.servingRequest.LookupPath.Path) {
+		// fmt.Printf("d.resolve d: %q - count: %d\n - r.URL."+
+		// 	"Path: %q - servingReq.Path: %q - servingReq."+
+		// 	"SubPath: %q\n",
+		// 	d.Name,
+		// 	count[d.Name],
+		// 	r.URL.Path,
+		// 	d.servingRequest.LookupPath.Path,
+		// 	d.servingRequest.LookupPath.SubPath,
+		// )
 		return d.servingRequest
 	}
 
diff --git a/internal/source/disk/domain_test.go b/internal/source/disk/domain_test.go
index 87827fb8a..1c81ba228 100644
--- a/internal/source/disk/domain_test.go
+++ b/internal/source/disk/domain_test.go
@@ -18,58 +18,54 @@ import (
 	"gitlab.com/gitlab-org/gitlab-pages/internal/testhelpers"
 )
 
-func serveFileOrNotFound(getDomain func() *domain.Domain) http.HandlerFunc {
-	d := getDomain()
+func serveFileOrNotFound(domain *domain.Domain) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
-
-		if !d.ServeFileHTTP(w, r) {
-			d.ServeNotFoundHTTP(w, r)
+		if !domain.ServeFileHTTP(w, r) {
+			domain.ServeNotFoundHTTP(w, r)
 		}
 	}
 }
 
 func testGroupServeHTTPHost(t *testing.T, host string) {
-	t.Helper()
-
-	testGroup := func() *domain.Domain {
-		return &domain.Domain{
-			Resolver: &Group{
-				name: "group",
-				projects: map[string]*projectConfig{
-					"group.test.io":            &projectConfig{},
-					"group.gitlab-example.com": &projectConfig{},
-					"project":                  &projectConfig{},
-					"project2":                 &projectConfig{},
-				},
+	testGroup := &domain.Domain{
+		Resolver: &Group{
+			name: "group",
+			projects: map[string]*projectConfig{
+				"group.test.io":            &projectConfig{},
+				"group.gitlab-example.com": &projectConfig{},
+				"project":                  &projectConfig{},
+				"project2":                 &projectConfig{},
 			},
-		}
+		},
 	}
 
 	makeURL := func(path string) string {
 		return "http://" + host + path
 	}
 
-	require.HTTPBodyContains(t, serveFileOrNotFound(testGroup), "GET", makeURL("/"), nil, "main-dir")
-	require.HTTPBodyContains(t, serveFileOrNotFound(testGroup), "GET", makeURL("/index"), nil, "main-dir")
-	require.HTTPBodyContains(t, serveFileOrNotFound(testGroup), "GET", makeURL("/index.html"), nil, "main-dir")
-	testhelpers.AssertRedirectTo(t, serveFileOrNotFound(testGroup), "GET", makeURL("/project"), nil, "//"+host+"/project/")
-	require.HTTPBodyContains(t, serveFileOrNotFound(testGroup), "GET", makeURL("/project/"), nil, "project-subdir")
-	require.HTTPBodyContains(t, serveFileOrNotFound(testGroup), "GET", makeURL("/project/index"), nil, "project-subdir")
-	require.HTTPBodyContains(t, serveFileOrNotFound(testGroup), "GET", makeURL("/project/index/"), nil, "project-subdir")
-	require.HTTPBodyContains(t, serveFileOrNotFound(testGroup), "GET", makeURL("/project/index.html"), nil, "project-subdir")
-	testhelpers.AssertRedirectTo(t, serveFileOrNotFound(testGroup), "GET", makeURL("/project/subdir"), nil, "//"+host+"/project/subdir/")
-	require.HTTPBodyContains(t, serveFileOrNotFound(testGroup), "GET", makeURL("/project/subdir/"), nil, "project-subsubdir")
-	require.HTTPBodyContains(t, serveFileOrNotFound(testGroup), "GET", makeURL("/project2/"), nil, "project2-main")
-	require.HTTPBodyContains(t, serveFileOrNotFound(testGroup), "GET", makeURL("/project2/index"), nil, "project2-main")
-	require.HTTPBodyContains(t, serveFileOrNotFound(testGroup), "GET", makeURL("/project2/index.html"), nil, "project2-main")
-	require.HTTPError(t, serveFileOrNotFound(testGroup), "GET", makeURL("/private.project/"), nil)
-	require.HTTPError(t, serveFileOrNotFound(testGroup), "GET", makeURL("//about.gitlab.com/%2e%2e"), nil)
-	require.HTTPError(t, serveFileOrNotFound(testGroup), "GET", makeURL("/symlink"), nil)
-	require.HTTPError(t, serveFileOrNotFound(testGroup), "GET", makeURL("/symlink/index.html"), nil)
-	require.HTTPError(t, serveFileOrNotFound(testGroup), "GET", makeURL("/symlink/subdir/"), nil)
-	require.HTTPError(t, serveFileOrNotFound(testGroup), "GET", makeURL("/project/fifo"), nil)
-	require.HTTPError(t, serveFileOrNotFound(testGroup), "GET", makeURL("/not-existing-file"), nil)
-	require.HTTPRedirect(t, serveFileOrNotFound(testGroup), "GET", makeURL("/project//about.gitlab.com/%2e%2e"), nil)
+	serve := serveFileOrNotFound(testGroup)
+
+	require.HTTPBodyContains(t, serve, "GET", makeURL("/"), nil, "main-dir")
+	require.HTTPBodyContains(t, serve, "GET", makeURL("/index"), nil, "main-dir")
+	require.HTTPBodyContains(t, serve, "GET", makeURL("/index.html"), nil, "main-dir")
+	testhelpers.AssertRedirectTo(t, serve, "GET", makeURL("/project"), nil, "//"+host+"/project/")
+	require.HTTPBodyContains(t, serve, "GET", makeURL("/project/"), nil, "project-subdir")
+	require.HTTPBodyContains(t, serve, "GET", makeURL("/project/index"), nil, "project-subdir")
+	require.HTTPBodyContains(t, serve, "GET", makeURL("/project/index/"), nil, "project-subdir")
+	require.HTTPBodyContains(t, serve, "GET", makeURL("/project/index.html"), nil, "project-subdir")
+	testhelpers.AssertRedirectTo(t, serve, "GET", makeURL("/project/subdir"), nil, "//"+host+"/project/subdir/")
+	require.HTTPBodyContains(t, serve, "GET", makeURL("/project/subdir/"), nil, "project-subsubdir")
+	require.HTTPBodyContains(t, serve, "GET", makeURL("/project2/"), nil, "project2-main")
+	require.HTTPBodyContains(t, serve, "GET", makeURL("/project2/index"), nil, "project2-main")
+	require.HTTPBodyContains(t, serve, "GET", makeURL("/project2/index.html"), nil, "project2-main")
+	require.HTTPError(t, serve, "GET", makeURL("/private.project/"), nil)
+	require.HTTPError(t, serve, "GET", makeURL("//about.gitlab.com/%2e%2e"), nil)
+	require.HTTPError(t, serve, "GET", makeURL("/symlink"), nil)
+	require.HTTPError(t, serve, "GET", makeURL("/symlink/index.html"), nil)
+	require.HTTPError(t, serve, "GET", makeURL("/symlink/subdir/"), nil)
+	require.HTTPError(t, serve, "GET", makeURL("/project/fifo"), nil)
+	require.HTTPError(t, serve, "GET", makeURL("/not-existing-file"), nil)
+	require.HTTPRedirect(t, serve, "GET", makeURL("/project//about.gitlab.com/%2e%2e"), nil)
 }
 
 func TestGroupServeHTTP(t *testing.T) {
@@ -84,14 +80,12 @@ func TestDomainServeHTTP(t *testing.T) {
 	cleanup := setUpTests(t)
 	defer cleanup()
 
-	testDomain := func() *domain.Domain {
-		return &domain.Domain{
-			Name: "test.domain.com",
-			Resolver: &customProjectResolver{
-				path:   "group/project2/public",
-				config: &domainConfig{},
-			},
-		}
+	testDomain := &domain.Domain{
+		Name: "test.domain.com",
+		Resolver: &customProjectResolver{
+			path:   "group/project2/public",
+			config: &domainConfig{},
+		},
 	}
 
 	require.HTTPBodyContains(t, serveFileOrNotFound(testDomain), "GET", "/", nil, "project2-main")
@@ -217,18 +211,16 @@ func TestGroupServeHTTPGzip(t *testing.T) {
 	cleanup := setUpTests(t)
 	defer cleanup()
 
-	testGroup := func() *domain.Domain {
-		return &domain.Domain{
-			Resolver: &Group{
-				name: "group",
-				projects: map[string]*projectConfig{
-					"group.test.io":            &projectConfig{},
-					"group.gitlab-example.com": &projectConfig{},
-					"project":                  &projectConfig{},
-					"project2":                 &projectConfig{},
-				},
+	testGroup := &domain.Domain{
+		Resolver: &Group{
+			name: "group",
+			projects: map[string]*projectConfig{
+				"group.test.io":            &projectConfig{},
+				"group.gitlab-example.com": &projectConfig{},
+				"project":                  &projectConfig{},
+				"project2":                 &projectConfig{},
 			},
-		}
+		},
 	}
 
 	testSet := []struct {
@@ -386,19 +378,17 @@ func TestGroup404ServeHTTP(t *testing.T) {
 	cleanup := setUpTests(t)
 	defer cleanup()
 
-	testGroup := func() *domain.Domain {
-		return &domain.Domain{
-			Resolver: &Group{
-				name: "group.404",
-				projects: map[string]*projectConfig{
-					"domain.404":          &projectConfig{},
-					"group.404.test.io":   &projectConfig{},
-					"project.404":         &projectConfig{},
-					"project.404.symlink": &projectConfig{},
-					"project.no.404":      &projectConfig{},
-				},
+	testGroup := &domain.Domain{
+		Resolver: &Group{
+			name: "group.404",
+			projects: map[string]*projectConfig{
+				"domain.404":          &projectConfig{},
+				"group.404.test.io":   &projectConfig{},
+				"project.404":         &projectConfig{},
+				"project.404.symlink": &projectConfig{},
+				"project.no.404":      &projectConfig{},
 			},
-		}
+		},
 	}
 
 	testhelpers.AssertHTTP404(t, serveFileOrNotFound(testGroup), "GET", "http://group.404.test.io/project.404/not/existing-file", nil, "Custom 404 project page")
@@ -416,13 +406,11 @@ func TestDomain404ServeHTTP(t *testing.T) {
 	cleanup := setUpTests(t)
 	defer cleanup()
 
-	testDomain := func() *domain.Domain {
-		return &domain.Domain{
-			Resolver: &customProjectResolver{
-				path:   "group.404/domain.404/public/",
-				config: &domainConfig{Domain: "domain.404.com"},
-			},
-		}
+	testDomain := &domain.Domain{
+		Resolver: &customProjectResolver{
+			path:   "group.404/domain.404/public/",
+			config: &domainConfig{Domain: "domain.404.com"},
+		},
 	}
 
 	testhelpers.AssertHTTP404(t, serveFileOrNotFound(testDomain), "GET", "http://group.404.test.io/not-existing-file", nil, "Custom domain.404 page")
@@ -433,7 +421,7 @@ func TestPredefined404ServeHTTP(t *testing.T) {
 	cleanup := setUpTests(t)
 	defer cleanup()
 
-	testDomain := func() *domain.Domain { return &domain.Domain{} }
+	testDomain := &domain.Domain{}
 
 	testhelpers.AssertHTTP404(t, serveFileOrNotFound(testDomain), "GET", "http://group.test.io/not-existing-file", nil, "The page you're looking for could not be found")
 }
@@ -482,17 +470,14 @@ func TestCacheControlHeaders(t *testing.T) {
 	cleanup := setUpTests(t)
 	defer cleanup()
 
-	testGroup := func() *domain.Domain {
-		return &domain.Domain{
-			Resolver: &Group{
-				name: "group",
-				projects: map[string]*projectConfig{
-					"group.test.io": &projectConfig{},
-				},
+	testGroup := &domain.Domain{
+		Resolver: &Group{
+			name: "group",
+			projects: map[string]*projectConfig{
+				"group.test.io": &projectConfig{},
 			},
-		}
+		},
 	}
-
 	w := httptest.NewRecorder()
 	req, err := http.NewRequest("GET", "http://group.test.io/", nil)
 	require.NoError(t, err)
-- 
GitLab


From 2533637ca527243c743458a573f80cc6dddd4644 Mon Sep 17 00:00:00 2001
From: Jaime Martinez <jmartinez@gitlab.com>
Date: Tue, 13 Oct 2020 15:43:23 +1100
Subject: [PATCH 6/7] Cleanup a bit

---
 internal/domain/domain.go       | 26 ++------------------------
 internal/serving/disk/reader.go |  8 ++------
 2 files changed, 4 insertions(+), 30 deletions(-)

diff --git a/internal/domain/domain.go b/internal/domain/domain.go
index fe34d4d2b..076dea0c9 100644
--- a/internal/domain/domain.go
+++ b/internal/domain/domain.go
@@ -10,7 +10,6 @@ import (
 
 	"gitlab.com/gitlab-org/gitlab-pages/internal/httperrors"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/serving"
-	"gitlab.com/gitlab-org/gitlab-pages/internal/serving/disk/local"
 )
 
 // Domain is a domain that gitlab-pages can serve.
@@ -49,34 +48,13 @@ func (d *Domain) isSameProject(reqPath string) bool {
 
 func (d *Domain) resolve(r *http.Request) *serving.Request {
 	if d.isSameProject(r.URL.Path) {
-		// // && strings.
-		// // 	Contains(r.URL.
-		// // 		Path,
-		// // 		d.servingRequest.LookupPath.Path) {
-		// fmt.Printf("d.resolve d: %q - count: %d\n - r.URL."+
-		// 	"Path: %q - servingReq.Path: %q - servingReq."+
-		// 	"SubPath: %q\n",
-		// 	d.Name,
-		// 	count[d.Name],
-		// 	r.URL.Path,
-		// 	d.servingRequest.LookupPath.Path,
-		// 	d.servingRequest.LookupPath.SubPath,
-		// )
 		return d.servingRequest
 	}
 
-	request, _ := d.Resolver.Resolve(r)
-
-	// TODO improve code around default serving, when `disk` serving gets removed
-	// https://gitlab.com/gitlab-org/gitlab-pages/issues/353
-	if request == nil {
-		request = &serving.Request{Serving: local.Instance()}
-	}
-
 	// store serving.Request to avoid calling d.Resolver.Resolve multiple times
-	d.servingRequest = request
+	d.servingRequest, _ = d.Resolver.Resolve(r)
 
-	return request
+	return d.servingRequest
 }
 
 // GetLookupPath returns a project details based on the request. It returns nil
diff --git a/internal/serving/disk/reader.go b/internal/serving/disk/reader.go
index 28ca11143..34f2c5fce 100644
--- a/internal/serving/disk/reader.go
+++ b/internal/serving/disk/reader.go
@@ -61,12 +61,8 @@ func (reader *Reader) tryFile(w http.ResponseWriter, r *http.Request, lookupPath
 		return err
 	}
 
-	fullPath, err := reader.resolvePath(ctx, root, lookupPath.SubPath)
-
-	// request := h.Request
-	host := r.Host
 	urlPath := r.URL.Path
-
+	fullPath, err := reader.resolvePath(ctx, root, lookupPath.SubPath)
 	if locationError, _ := err.(*locationDirectoryError); locationError != nil {
 		if endsWithSlash(urlPath) {
 			fullPath, err = reader.resolvePath(ctx, root, lookupPath.SubPath,
@@ -76,7 +72,7 @@ func (reader *Reader) tryFile(w http.ResponseWriter, r *http.Request, lookupPath
 			// issue about this: https://gitlab.com/gitlab-org/gitlab-pages/issues/273
 
 			// Concat Host with URL.Path
-			redirectPath := "//" + host + "/"
+			redirectPath := "//" + r.Host + "/"
 			redirectPath += strings.TrimPrefix(urlPath, "/")
 
 			// Ensure that there's always "/" at end
-- 
GitLab


From 0db3e9e21a8c6541ffeb955f66284678b901d301 Mon Sep 17 00:00:00 2001
From: Jaime Martinez <jmartinez@gitlab.com>
Date: Wed, 14 Oct 2020 15:19:11 +1100
Subject: [PATCH 7/7] Revert "Remove serving.Handler"

This reverts commit 0fcc0667e6f2e78ca85abdb5c9f631a964dd77c2.
---
 internal/domain/domain_test.go                |  8 ++-
 internal/serving/disk/local/serving_test.go   | 13 ++--
 internal/serving/disk/reader.go               | 62 +++++++++----------
 internal/serving/disk/serving.go              | 13 ++--
 internal/serving/disk/zip/serving_test.go     | 13 ++--
 internal/serving/handler.go                   | 12 ++++
 internal/serving/lookup_path.go               |  1 -
 internal/serving/request.go                   | 19 +++++-
 internal/serving/serverless/serverless.go     | 11 ++--
 .../serving/serverless/serverless_test.go     | 13 ++--
 internal/serving/serving.go                   |  8 +--
 internal/source/disk/custom.go                |  2 +-
 internal/source/disk/group.go                 |  2 +-
 internal/source/gitlab/factory.go             |  5 +-
 internal/source/gitlab/factory_test.go        |  5 +-
 internal/source/gitlab/gitlab.go              |  4 +-
 internal/source/gitlab/gitlab_test.go         | 10 +--
 17 files changed, 116 insertions(+), 85 deletions(-)
 create mode 100644 internal/serving/handler.go

diff --git a/internal/domain/domain_test.go b/internal/domain/domain_test.go
index 1020d70a6..2df1d9929 100644
--- a/internal/domain/domain_test.go
+++ b/internal/domain/domain_test.go
@@ -17,6 +17,7 @@ import (
 
 type stubbedResolver struct {
 	project *serving.LookupPath
+	subpath string
 	err     error
 }
 
@@ -24,6 +25,7 @@ func (resolver *stubbedResolver) Resolve(*http.Request) (*serving.Request, error
 	return &serving.Request{
 		Serving:    local.Instance(),
 		LookupPath: resolver.project,
+		SubPath:    resolver.subpath,
 	}, resolver.err
 }
 
@@ -152,8 +154,8 @@ func TestServeNamespaceNotFound(t *testing.T) {
 				project: &serving.LookupPath{
 					Path:               "group.404/group.404.gitlab-example.com/public",
 					IsNamespaceProject: true,
-					SubPath:            "/unknown",
 				},
+				subpath: "/unknown",
 			},
 			expectedResponse: "Custom 404 group page",
 		},
@@ -167,6 +169,7 @@ func TestServeNamespaceNotFound(t *testing.T) {
 					IsNamespaceProject: true,
 					HasAccessControl:   false,
 				},
+				subpath: "/",
 			},
 			expectedResponse: "Custom 404 group page",
 		},
@@ -179,8 +182,8 @@ func TestServeNamespaceNotFound(t *testing.T) {
 					Path:               "group.404/group.404.gitlab-example.com/public",
 					IsNamespaceProject: true,
 					HasAccessControl:   true,
-					SubPath:            "/",
 				},
+				subpath: "/",
 			},
 			expectedResponse: "The page you're looking for could not be found.",
 		},
@@ -190,6 +193,7 @@ func TestServeNamespaceNotFound(t *testing.T) {
 			path:   "/unknown",
 			resolver: &stubbedResolver{
 				project: nil,
+				subpath: "/",
 			},
 			expectedResponse: "The page you're looking for could not be found.",
 		},
diff --git a/internal/serving/disk/local/serving_test.go b/internal/serving/disk/local/serving_test.go
index 1f6929077..60f01acd6 100644
--- a/internal/serving/disk/local/serving_test.go
+++ b/internal/serving/disk/local/serving_test.go
@@ -18,14 +18,17 @@ func TestDisk_ServeFileHTTP(t *testing.T) {
 	s := Instance()
 	w := httptest.NewRecorder()
 	r := httptest.NewRequest("GET", "http://group.gitlab-example.com/serving/index.html", nil)
-
-	lookupPath := &serving.LookupPath{
-		Prefix:  "/serving",
-		Path:    "group/serving/public",
+	handler := serving.Handler{
+		Writer:  w,
+		Request: r,
+		LookupPath: &serving.LookupPath{
+			Prefix: "/serving",
+			Path:   "group/serving/public",
+		},
 		SubPath: "/index.html",
 	}
 
-	require.True(t, s.ServeFileHTTP(w, r, lookupPath))
+	require.True(t, s.ServeFileHTTP(handler))
 
 	resp := w.Result()
 	defer resp.Body.Close()
diff --git a/internal/serving/disk/reader.go b/internal/serving/disk/reader.go
index 34f2c5fce..e7f15a1e3 100644
--- a/internal/serving/disk/reader.go
+++ b/internal/serving/disk/reader.go
@@ -25,66 +25,67 @@ type Reader struct {
 }
 
 // Show the user some validation messages for their _redirects file
-func (reader *Reader) serveRedirectsStatus(w http.ResponseWriter, redirects *redirects.Redirects) error {
-	w.Header().Set("Content-Type", "text/plain; charset=utf-8")
-	w.Header().Set("X-Content-Type-Options", "nosniff")
-	w.WriteHeader(http.StatusOK)
-
-	_, err := fmt.Fprintln(w, redirects.Status())
+func (reader *Reader) serveRedirectsStatus(h serving.Handler, redirects *redirects.Redirects) error {
+	h.Writer.Header().Set("Content-Type", "text/plain; charset=utf-8")
+	h.Writer.Header().Set("X-Content-Type-Options", "nosniff")
+	h.Writer.WriteHeader(http.StatusOK)
+	_, err := fmt.Fprintln(h.Writer, redirects.Status())
 	return err
 }
 
-func (reader *Reader) tryRedirects(w http.ResponseWriter, r *http.Request, lookupPath *serving.LookupPath) error {
-	ctx := r.Context()
-	root, err := reader.vfs.Root(ctx, lookupPath.Path)
+func (reader *Reader) tryRedirects(h serving.Handler) error {
+	ctx := h.Request.Context()
+	root, err := reader.vfs.Root(ctx, h.LookupPath.Path)
 	if err != nil {
 		return err
 	}
 
-	redirs := redirects.ParseRedirects(ctx, root)
+	r := redirects.ParseRedirects(ctx, root)
 
-	rewrittenURL, status, err := redirs.Rewrite(r.URL)
+	rewrittenURL, status, err := r.Rewrite(h.Request.URL)
 	if err != nil {
 		return err
 	}
 
-	http.Redirect(w, r, rewrittenURL.Path, status)
+	http.Redirect(h.Writer, h.Request, rewrittenURL.Path, status)
 
 	return nil
 }
 
-func (reader *Reader) tryFile(w http.ResponseWriter, r *http.Request, lookupPath *serving.LookupPath) error {
-	ctx := r.Context()
+func (reader *Reader) tryFile(h serving.Handler) error {
+	ctx := h.Request.Context()
 
-	root, err := reader.vfs.Root(ctx, lookupPath.Path)
+	root, err := reader.vfs.Root(ctx, h.LookupPath.Path)
 	if err != nil {
 		return err
 	}
 
-	urlPath := r.URL.Path
-	fullPath, err := reader.resolvePath(ctx, root, lookupPath.SubPath)
+	fullPath, err := reader.resolvePath(ctx, root, h.SubPath)
+
+	request := h.Request
+	host := request.Host
+	urlPath := request.URL.Path
+
 	if locationError, _ := err.(*locationDirectoryError); locationError != nil {
 		if endsWithSlash(urlPath) {
-			fullPath, err = reader.resolvePath(ctx, root, lookupPath.SubPath,
-				"index.html")
+			fullPath, err = reader.resolvePath(ctx, root, h.SubPath, "index.html")
 		} else {
 			// TODO why are we doing that? In tests it redirects to HTTPS. This seems wrong,
 			// issue about this: https://gitlab.com/gitlab-org/gitlab-pages/issues/273
 
 			// Concat Host with URL.Path
-			redirectPath := "//" + r.Host + "/"
+			redirectPath := "//" + host + "/"
 			redirectPath += strings.TrimPrefix(urlPath, "/")
 
 			// Ensure that there's always "/" at end
 			redirectPath = strings.TrimSuffix(redirectPath, "/") + "/"
-			http.Redirect(w, r, redirectPath, 302)
+			http.Redirect(h.Writer, h.Request, redirectPath, 302)
 			return nil
 		}
 	}
 
 	if locationError, _ := err.(*locationFileNoExtensionError); locationError != nil {
-		fullPath, err = reader.resolvePath(ctx, root,
-			strings.TrimSuffix(lookupPath.SubPath, "/")+".html")
+		fullPath, err = reader.resolvePath(ctx, root, strings.TrimSuffix(h.SubPath, "/")+".html")
 	}
 
 	if err != nil {
@@ -96,20 +97,20 @@ func (reader *Reader) tryFile(w http.ResponseWriter, r *http.Request, lookupPath
 	if fullPath == redirects.ConfigFile {
 		if os.Getenv("FF_ENABLE_REDIRECTS") != "false" {
 			r := redirects.ParseRedirects(ctx, root)
-			return reader.serveRedirectsStatus(w, r)
+			return reader.serveRedirectsStatus(h, r)
 		}
 
-		w.WriteHeader(http.StatusForbidden)
+		h.Writer.WriteHeader(http.StatusForbidden)
 		return nil
 	}
 
-	return reader.serveFile(ctx, w, r, root, fullPath, lookupPath.HasAccessControl)
+	return reader.serveFile(ctx, h.Writer, h.Request, root, fullPath, h.LookupPath.HasAccessControl)
 }
 
-func (reader *Reader) tryNotFound(w http.ResponseWriter, r *http.Request, lookupPath *serving.LookupPath) error {
-	ctx := r.Context()
+func (reader *Reader) tryNotFound(h serving.Handler) error {
+	ctx := h.Request.Context()
 
-	root, err := reader.vfs.Root(ctx, lookupPath.Path)
+	root, err := reader.vfs.Root(ctx, h.LookupPath.Path)
 	if err != nil {
 		return err
 	}
@@ -119,8 +120,7 @@ func (reader *Reader) tryNotFound(w http.ResponseWriter, r *http.Request, lookup
 		return err
 	}
 
-	err = reader.serveCustomFile(ctx, w, r, http.StatusNotFound,
-		root, page404)
+	err = reader.serveCustomFile(ctx, h.Writer, h.Request, http.StatusNotFound, root, page404)
 	if err != nil {
 		return err
 	}
diff --git a/internal/serving/disk/serving.go b/internal/serving/disk/serving.go
index 34762491f..11b1689e3 100644
--- a/internal/serving/disk/serving.go
+++ b/internal/serving/disk/serving.go
@@ -1,7 +1,6 @@
 package disk
 
 import (
-	"net/http"
 	"os"
 
 	"gitlab.com/gitlab-org/gitlab-pages/internal/httperrors"
@@ -17,13 +16,13 @@ type Disk struct {
 
 // ServeFileHTTP serves a file from disk and returns true. It returns false
 // when a file could not been found.
-func (s *Disk) ServeFileHTTP(w http.ResponseWriter, r *http.Request, lookupPath *serving.LookupPath) bool {
-	if s.reader.tryFile(w, r, lookupPath) == nil {
+func (s *Disk) ServeFileHTTP(h serving.Handler) bool {
+	if s.reader.tryFile(h) == nil {
 		return true
 	}
 
 	if os.Getenv("FF_ENABLE_REDIRECTS") != "false" {
-		if s.reader.tryRedirects(w, r, lookupPath) == nil {
+		if s.reader.tryRedirects(h) == nil {
 			return true
 		}
 	}
@@ -32,13 +31,13 @@ func (s *Disk) ServeFileHTTP(w http.ResponseWriter, r *http.Request, lookupPath
 }
 
 // ServeNotFoundHTTP tries to read a custom 404 page
-func (s *Disk) ServeNotFoundHTTP(w http.ResponseWriter, r *http.Request, lookupPath *serving.LookupPath) {
-	if s.reader.tryNotFound(w, r, lookupPath) == nil {
+func (s *Disk) ServeNotFoundHTTP(h serving.Handler) {
+	if s.reader.tryNotFound(h) == nil {
 		return
 	}
 
 	// Generic 404
-	httperrors.Serve404(w)
+	httperrors.Serve404(h.Writer)
 }
 
 // New returns a serving instance that is capable of reading files
diff --git a/internal/serving/disk/zip/serving_test.go b/internal/serving/disk/zip/serving_test.go
index 134fe4dc3..14fdabdf6 100644
--- a/internal/serving/disk/zip/serving_test.go
+++ b/internal/serving/disk/zip/serving_test.go
@@ -19,14 +19,17 @@ func TestZip_ServeFileHTTP(t *testing.T) {
 	s := Instance()
 	w := httptest.NewRecorder()
 	r := httptest.NewRequest("GET", "http://zip.gitlab.io/zip/index.html", nil)
-
-	lookupPath := &serving.LookupPath{
-		Prefix:  "",
-		Path:    testServerURL + "/public.zip",
+	handler := serving.Handler{
+		Writer:  w,
+		Request: r,
+		LookupPath: &serving.LookupPath{
+			Prefix: "",
+			Path:   testServerURL + "/public.zip",
+		},
 		SubPath: "/index.html",
 	}
 
-	require.True(t, s.ServeFileHTTP(w, r, lookupPath))
+	require.True(t, s.ServeFileHTTP(handler))
 
 	resp := w.Result()
 	defer resp.Body.Close()
diff --git a/internal/serving/handler.go b/internal/serving/handler.go
new file mode 100644
index 000000000..a0d66ecbf
--- /dev/null
+++ b/internal/serving/handler.go
@@ -0,0 +1,12 @@
+package serving
+
+import "net/http"
+
+// Handler aggregates response/request and lookup path + subpath needed to
+// handle a request and response.
+type Handler struct {
+	Writer     http.ResponseWriter
+	Request    *http.Request
+	LookupPath *LookupPath
+	SubPath    string
+}
diff --git a/internal/serving/lookup_path.go b/internal/serving/lookup_path.go
index 1f0ec0f3a..1aefe1b85 100644
--- a/internal/serving/lookup_path.go
+++ b/internal/serving/lookup_path.go
@@ -9,5 +9,4 @@ type LookupPath struct {
 	IsHTTPSOnly        bool
 	HasAccessControl   bool
 	ProjectID          uint64
-	SubPath            string
 }
diff --git a/internal/serving/request.go b/internal/serving/request.go
index dac03f91e..694d0df4f 100644
--- a/internal/serving/request.go
+++ b/internal/serving/request.go
@@ -7,14 +7,29 @@ import "net/http"
 type Request struct {
 	Serving    Serving     // Serving chosen to serve this request
 	LookupPath *LookupPath // LookupPath contains pages project details
+	SubPath    string      // Subpath is a URL path subcomponent for this request
 }
 
 // ServeFileHTTP forwards serving request handler to the serving itself
 func (s *Request) ServeFileHTTP(w http.ResponseWriter, r *http.Request) bool {
-	return s.Serving.ServeFileHTTP(w, r, s.LookupPath)
+	handler := Handler{
+		Writer:     w,
+		Request:    r,
+		LookupPath: s.LookupPath,
+		SubPath:    s.SubPath,
+	}
+
+	return s.Serving.ServeFileHTTP(handler)
 }
 
 // ServeNotFoundHTTP forwards serving request handler to the serving itself
 func (s *Request) ServeNotFoundHTTP(w http.ResponseWriter, r *http.Request) {
-	s.Serving.ServeNotFoundHTTP(w, r, s.LookupPath)
+	handler := Handler{
+		Writer:     w,
+		Request:    r,
+		LookupPath: s.LookupPath,
+		SubPath:    s.SubPath,
+	}
+
+	s.Serving.ServeNotFoundHTTP(handler)
 }
diff --git a/internal/serving/serverless/serverless.go b/internal/serving/serverless/serverless.go
index c2f592de1..e1881362a 100644
--- a/internal/serving/serverless/serverless.go
+++ b/internal/serving/serverless/serverless.go
@@ -2,7 +2,6 @@ package serverless
 
 import (
 	"errors"
-	"net/http"
 	"net/http/httputil"
 
 	"gitlab.com/gitlab-org/gitlab-pages/internal/httperrors"
@@ -54,17 +53,15 @@ func New(service string, cluster Cluster) serving.Serving {
 }
 
 // ServeFileHTTP handle an incoming request and proxies it to Knative cluster
-func (s *Serverless) ServeFileHTTP(w http.ResponseWriter, r *http.Request,
-	_ *serving.LookupPath) bool {
+func (s *Serverless) ServeFileHTTP(h serving.Handler) bool {
 	metrics.ServerlessRequests.Inc()
 
-	s.proxy.ServeHTTP(w, r)
+	s.proxy.ServeHTTP(h.Writer, h.Request)
 
 	return true
 }
 
 // ServeNotFoundHTTP responds with 404
-func (s *Serverless) ServeNotFoundHTTP(w http.ResponseWriter,
-	_ *http.Request, _ *serving.LookupPath) {
-	httperrors.Serve404(w)
+func (s *Serverless) ServeNotFoundHTTP(h serving.Handler) {
+	httperrors.Serve404(h.Writer)
 }
diff --git a/internal/serving/serverless/serverless_test.go b/internal/serving/serverless/serverless_test.go
index 54cf23198..ebd143432 100644
--- a/internal/serving/serverless/serverless_test.go
+++ b/internal/serving/serverless/serverless_test.go
@@ -11,6 +11,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"gitlab.com/gitlab-org/gitlab-pages/internal/fixture"
+	"gitlab.com/gitlab-org/gitlab-pages/internal/serving"
 )
 
 func withTestCluster(t *testing.T, cert, key string, block func(*http.ServeMux, *url.URL, *Certs)) {
@@ -49,6 +50,7 @@ func TestServeFileHTTP(t *testing.T) {
 
 			writer := httptest.NewRecorder()
 			request := httptest.NewRequest("GET", "http://example.gitlab.com/", nil)
+			handler := serving.Handler{Writer: writer, Request: request}
 			request.Header.Set("X-Real-IP", "127.0.0.105")
 
 			mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
@@ -58,7 +60,7 @@ func TestServeFileHTTP(t *testing.T) {
 				require.Contains(t, r.Header.Get("X-Forwarded-For"), "127.0.0.105")
 			})
 
-			served := serverless.ServeFileHTTP(writer, request, nil)
+			served := serverless.ServeFileHTTP(handler)
 			result := writer.Result()
 
 			require.True(t, served)
@@ -80,12 +82,13 @@ func TestServeFileHTTP(t *testing.T) {
 
 			writer := httptest.NewRecorder()
 			request := httptest.NewRequest("GET", "http://example.gitlab.com/", nil)
+			handler := serving.Handler{Writer: writer, Request: request}
 
 			mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
 				w.WriteHeader(http.StatusBadRequest)
 			})
 
-			served := serverless.ServeFileHTTP(writer, request, nil)
+			served := serverless.ServeFileHTTP(handler)
 			result := writer.Result()
 			body, err := ioutil.ReadAll(writer.Body)
 			require.NoError(t, err)
@@ -110,13 +113,14 @@ func TestServeFileHTTP(t *testing.T) {
 
 			writer := httptest.NewRecorder()
 			request := httptest.NewRequest("GET", "http://example.gitlab.com/", nil)
+			handler := serving.Handler{Writer: writer, Request: request}
 
 			mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
 				w.WriteHeader(http.StatusServiceUnavailable)
 				w.Write([]byte("sorry, service unavailable"))
 			})
 
-			served := serverless.ServeFileHTTP(writer, request, nil)
+			served := serverless.ServeFileHTTP(handler)
 			result := writer.Result()
 			body, err := ioutil.ReadAll(writer.Body)
 			require.NoError(t, err)
@@ -141,13 +145,14 @@ func TestServeFileHTTP(t *testing.T) {
 
 			writer := httptest.NewRecorder()
 			request := httptest.NewRequest("GET", "http://example.gitlab.com/", nil)
+			handler := serving.Handler{Writer: writer, Request: request}
 
 			mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
 				w.WriteHeader(http.StatusOK)
 				w.Write([]byte("OK"))
 			})
 
-			served := serverless.ServeFileHTTP(writer, request, nil)
+			served := serverless.ServeFileHTTP(handler)
 			result := writer.Result()
 			body, err := ioutil.ReadAll(writer.Body)
 			require.NoError(t, err)
diff --git a/internal/serving/serving.go b/internal/serving/serving.go
index 1c89e8f4c..6fde82165 100644
--- a/internal/serving/serving.go
+++ b/internal/serving/serving.go
@@ -1,11 +1,7 @@
 package serving
 
-import "net/http"
-
 // Serving is an interface used to define a serving driver
 type Serving interface {
-	ServeFileHTTP(w http.ResponseWriter, r *http.Request,
-		lookupPath *LookupPath) bool
-	ServeNotFoundHTTP(w http.ResponseWriter, r *http.Request,
-		lookupPath *LookupPath)
+	ServeFileHTTP(Handler) bool
+	ServeNotFoundHTTP(Handler)
 }
diff --git a/internal/source/disk/custom.go b/internal/source/disk/custom.go
index 9e8bb4572..fb2aafcdc 100644
--- a/internal/source/disk/custom.go
+++ b/internal/source/disk/custom.go
@@ -22,11 +22,11 @@ func (p *customProjectResolver) Resolve(r *http.Request) (*serving.Request, erro
 		IsHTTPSOnly:        p.config.HTTPSOnly,
 		HasAccessControl:   p.config.AccessControl,
 		ProjectID:          p.config.ID,
-		SubPath:            r.URL.Path,
 	}
 
 	return &serving.Request{
 		Serving:    local.Instance(),
 		LookupPath: lookupPath,
+		SubPath:    r.URL.Path,
 	}, nil
 }
diff --git a/internal/source/disk/group.go b/internal/source/disk/group.go
index 8c54cdd4d..b12727aa6 100644
--- a/internal/source/disk/group.go
+++ b/internal/source/disk/group.go
@@ -95,11 +95,11 @@ func (g *Group) Resolve(r *http.Request) (*serving.Request, error) {
 		IsHTTPSOnly:        projectConfig.HTTPSOnly,
 		HasAccessControl:   projectConfig.AccessControl,
 		ProjectID:          projectConfig.ID,
-		SubPath:            subPath,
 	}
 
 	return &serving.Request{
 		Serving:    local.Instance(),
 		LookupPath: lookupPath,
+		SubPath:    subPath,
 	}, nil
 }
diff --git a/internal/source/gitlab/factory.go b/internal/source/gitlab/factory.go
index 0df2e8c60..41f7ea56b 100644
--- a/internal/source/gitlab/factory.go
+++ b/internal/source/gitlab/factory.go
@@ -13,16 +13,15 @@ import (
 // fabricateLookupPath fabricates a serving LookupPath based on the API LookupPath
 // `size` argument is DEPRECATED, see
 // https://gitlab.com/gitlab-org/gitlab-pages/issues/272
-func fabricateLookupPath(size int, lookup api.LookupPath, subPath string) *serving.LookupPath {
+func fabricateLookupPath(size int, lookup api.LookupPath) *serving.LookupPath {
 	return &serving.LookupPath{
 		ServingType:        lookup.Source.Type,
 		Path:               lookup.Source.Path,
 		Prefix:             lookup.Prefix,
-		IsNamespaceProject: lookup.Prefix == "/" && size > 1,
+		IsNamespaceProject: (lookup.Prefix == "/" && size > 1),
 		IsHTTPSOnly:        lookup.HTTPSOnly,
 		HasAccessControl:   lookup.AccessControl,
 		ProjectID:          uint64(lookup.ProjectID),
-		SubPath:            subPath,
 	}
 }
 
diff --git a/internal/source/gitlab/factory_test.go b/internal/source/gitlab/factory_test.go
index 804f5d020..2f3e19940 100644
--- a/internal/source/gitlab/factory_test.go
+++ b/internal/source/gitlab/factory_test.go
@@ -15,17 +15,16 @@ func TestFabricateLookupPath(t *testing.T) {
 	t.Run("when lookup path is not a namespace project", func(t *testing.T) {
 		lookup := api.LookupPath{Prefix: "/something"}
 
-		path := fabricateLookupPath(1, lookup, "/subpath")
+		path := fabricateLookupPath(1, lookup)
 
 		require.Equal(t, path.Prefix, "/something")
-		require.Equal(t, path.SubPath, "/subpath")
 		require.False(t, path.IsNamespaceProject)
 	})
 
 	t.Run("when lookup path is a namespace project", func(t *testing.T) {
 		lookup := api.LookupPath{Prefix: "/"}
 
-		path := fabricateLookupPath(2, lookup, "")
+		path := fabricateLookupPath(2, lookup)
 
 		require.Equal(t, path.Prefix, "/")
 		require.True(t, path.IsNamespaceProject)
diff --git a/internal/source/gitlab/gitlab.go b/internal/source/gitlab/gitlab.go
index eecc2f2f8..67c4eb6ba 100644
--- a/internal/source/gitlab/gitlab.go
+++ b/internal/source/gitlab/gitlab.go
@@ -96,8 +96,8 @@ func (g *Gitlab) Resolve(r *http.Request) (*serving.Request, error) {
 
 			return &serving.Request{
 				Serving:    fabricateServing(lookup),
-				LookupPath: fabricateLookupPath(size, lookup, subPath),
-			}, nil
+				LookupPath: fabricateLookupPath(size, lookup),
+				SubPath:    subPath}, nil
 		}
 	}
 
diff --git a/internal/source/gitlab/gitlab_test.go b/internal/source/gitlab/gitlab_test.go
index 5537a0339..e6f194eee 100644
--- a/internal/source/gitlab/gitlab_test.go
+++ b/internal/source/gitlab/gitlab_test.go
@@ -44,7 +44,7 @@ func TestResolve(t *testing.T) {
 
 		require.Equal(t, "/my/pages/project/", response.LookupPath.Prefix)
 		require.Equal(t, "some/path/to/project/", response.LookupPath.Path)
-		require.Equal(t, "", response.LookupPath.SubPath)
+		require.Equal(t, "", response.SubPath)
 		require.False(t, response.LookupPath.IsNamespaceProject)
 	})
 
@@ -57,7 +57,7 @@ func TestResolve(t *testing.T) {
 
 		require.Equal(t, "/my/pages/project/", response.LookupPath.Prefix)
 		require.Equal(t, "some/path/to/project/", response.LookupPath.Path)
-		require.Equal(t, "path/index.html", response.LookupPath.SubPath)
+		require.Equal(t, "path/index.html", response.SubPath)
 		require.False(t, response.LookupPath.IsNamespaceProject)
 	})
 
@@ -70,7 +70,7 @@ func TestResolve(t *testing.T) {
 
 		require.Equal(t, "/", response.LookupPath.Prefix)
 		require.Equal(t, "some/path/to/project-3/", response.LookupPath.Path)
-		require.Equal(t, "", response.LookupPath.SubPath)
+		require.Equal(t, "", response.SubPath)
 		require.True(t, response.LookupPath.IsNamespaceProject)
 	})
 
@@ -82,7 +82,7 @@ func TestResolve(t *testing.T) {
 		require.NoError(t, err)
 
 		require.Equal(t, "/", response.LookupPath.Prefix)
-		require.Equal(t, "path/to/index.html", response.LookupPath.SubPath)
+		require.Equal(t, "path/to/index.html", response.SubPath)
 		require.Equal(t, "some/path/to/project-3/", response.LookupPath.Path)
 		require.True(t, response.LookupPath.IsNamespaceProject)
 	})
@@ -95,6 +95,6 @@ func TestResolve(t *testing.T) {
 		require.NoError(t, err)
 
 		require.Equal(t, "/my/pages/project/", response.LookupPath.Prefix)
-		require.Equal(t, "index.html", response.LookupPath.SubPath)
+		require.Equal(t, "index.html", response.SubPath)
 	})
 }
-- 
GitLab